Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better concurrency support #390

Open
ambyjkl opened this issue Aug 28, 2024 · 1 comment
Open

Better concurrency support #390

ambyjkl opened this issue Aug 28, 2024 · 1 comment

Comments

@ambyjkl
Copy link
Contributor

ambyjkl commented Aug 28, 2024

Hello,
I want to use adblock-rust in a setup where concurrency would be beneficial. Although disabling object-pooling and unsync-regex-caching is enough to make Engine Send + Sync, there is still work to be done to actually make concurrent usage efficient. Hence, I want to contribute towards this goal, here is what I'm planing to do:

  • Use a thread-local variable for the request_tokens vector
    • I believe this solution might even outperform the object pooling implementation, and hence eliminate the need for the lifeguard dependency
  • Use a concurrent cache rather than a mutexed hashmap
    • While I have not used any concurrent cache other than redis (dashmap doesn't count as it doesn't have eviction support) in previous projects, after a bit of research, mini-moka (https://crates.io/crates/mini-moka) in particular caught my attention, since it has all the features we need while also providing a non-thread-safe implementation for the single-threaded case, so there can be high code reuse, while also being the sibling of moka which is battle-tested in production on crates.io itself

Please let me know your thoughts

@ambyjkl
Copy link
Contributor Author

ambyjkl commented Aug 29, 2024

On second thought, time-based eviction is probably not the best idea; we don't lose much by leaving the compiled regex in the cache, since recompiling can be expensive. Also, Instant is not available on wasm32-unknown-unknown. Considering this, I think quick_cache (https://crates.io/crates/quick_cache/0.6.5) is a better fit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant