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

Possibility of using async runtime tasks instead of thread pools #288

Closed
stepantubanov opened this issue Jul 23, 2023 · 10 comments
Closed
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@stepantubanov
Copy link

stepantubanov commented Jul 23, 2023

While investigating high thread count and occasional CPU throttling I noticed that moka is creating it's own thread pool. Together with Tokio runtime it results in number of threads being 2x greater than "available parallelism". I was wondering if it would be possible to convert those threads into async runtime tasks? What would be main challenges there and is it a lot of work? Main reasoning for this is to have better control over CPU resources and reduce throttling.

Related issues:

@tatsuya6502
Copy link
Member

tatsuya6502 commented Jul 23, 2023

Hi, thank you for the suggestions.

if it would be possible to convert those threads into async runtime tasks?

Yes, I believe it is possible. However it will make future::Cache to depend on a specific async runtime because each runtime provides their own spawn and timer implementations. We could do something like SQLx does by providing different crate features for different async runtime (e.g. Tokio, async-std, actix-rt). But future::Cache will be only compatible with these async runtimes and incompatible with others.

Instead of doing that, I was thinking to remove these background threads completely, so that future::Cache does not have to depend on any async runtime. The work that the background threads were doing will be done by callers of cache's methods (insert, get, etc). sync::Cache has such an option since v0.9.3 (doc).

To do that for future::Cache, we will need to copy the majority of methods for the background threads, and convert them from regular fn into async fn. I did some experiments in February as a part of #228 and the diff is here.

I found removing the background threads would cause a major breaking changes on the API:

#228

  • The get method will become an async fn.
  • The blocking().insert(..) and blocking().invalidate(..) methods will be removed.
    • We cannot provide them without relying on a specific async runtime (Tokio, async-std, or actix-rt, etc).
  • The return type of the event listener closure will be changed from () to ListenerFuture. which is Pin<Box<dyn Future<Output = ()> + Send>>:
    • So, the new signature of the event listener closure will be:
      • impl Fn(Arc<K>, V, RemovalCause) -> ListenerFuture + Send + Sync + 'static
  • invalidate_entries_if is temporary removed.

Let me review and remember what I did there, and then I will do it again for v0.12.0 release. Since it will be a major breaking change, we will need to write a migration guide.

@tatsuya6502 tatsuya6502 added the enhancement New feature or request label Jul 23, 2023
@tatsuya6502 tatsuya6502 self-assigned this Jul 23, 2023
@tatsuya6502
Copy link
Member

tatsuya6502 commented Aug 1, 2023

I started to work on this for v0.12.0 release:

See the MIGRATION-GUIDE.md for details of the change.

Since this will be a major breaking change on future::Cache, we will create some beta releases before v0.12.0.

@tatsuya6502 tatsuya6502 added this to the v0.12.0 milestone Aug 1, 2023
@ben-manes
Copy link

Somewhat related, a neat trick with async support is that should be able to combine it with other libraries very easily to perform more advanced operations. I wrote a few simple examples in Java that I think would work very well in moka.

  1. Using a reactive streams library (e.g. rxRust), coalesce cache loads over a time and size window to perform a single batch load.
  2. The reverse of (1) is a write-behind cache, where the aggregated updates are written back to the data store
  3. Enrich the async operation with a retry, timeout, and fallback policy.

I'm not familiar enough with Rust's async runtimes, but I bet you could write similar examples once this is complete to demonstrate the flexibility of your library.

@tatsuya6502
Copy link
Member

I have merged the huge pull request #294 into the main branch, but did not cut a beta release. I will spend few more evenings on the following pull request, and if it goes well, I will cut the beta release.

You will be affected by the above if you use async_eviction_listener and do async cancellation. For example, some sever frameworks such as Axum do async cancellation for you; request handling will be cancelled when the client connection is dropped.

If you are not affected, you can try the thread-pool-less future::Cache now by adding the following to your Cargo.toml:

[dependencies.moka]
git = "https://github.com/moka-rs/moka"
branch = "main"
default-features = false                     # Necessary for now to turn off `sync::Cache`.
features = ["future", "atomic64", "quanta"]  # atomic64 and quanta are in the default features.

Also, now the MIGRATION-GUIDE.md is available on the main branch.

@tatsuya6502
Copy link
Member

tatsuya6502 commented Aug 27, 2023

@stepantubanov @DDtKey @tobz1000

I published moka v0.12.0-beta.1 to crates.io.
https://crates.io/crates/moka/0.12.0-beta.1

Its future::Cache does not use background threads at all.

[dependencies]
# `default-features = false` is necessary for now to turn off `sync::Cache`.
# atomic64 and quanta are in the default features.
moka = { version = "0.12.0-beta.1", default-features = false, features = ["future", "atomic64", "quanta"] }

You may/may not need to change your codes to use v0.12.0-beta.1. Please see the MIGRATION-GUIDE.md for details.

@tatsuya6502
Copy link
Member

I published moka v0.12.0-beta.2 to crates.io.
https://crates.io/crates/moka/0.12.0-beta.2

Its future::Cache has no change since v0.12.0-beta.1, but its sync caches do not use background threads anymore. Also a crate feature sync was removed from the default features, so you do not need to put default-features = false in your dependency anymore.

[dependencies]
moka = { version = "0.12.0-beta.2", features = ["future"] }

I also updated the MIGRATION-GUIDE.md.

@tatsuya6502
Copy link
Member

Ah, I forgot to enable sync feature for doc.rs, so the doc for sync module was not generated. I will fix it in next beta release.

https://docs.rs/moka/0.12.0-beta.2/moka/

doc rs-moka-v0 12 0-beta 2

@tatsuya6502
Copy link
Member

I forgot to enable sync feature for doc.rs,

Fixed via #318.

Since #294 was a big change, I am a bit worried if I introduced any issues to future::Cache. I will spend next couple of weeks for writing and running more end-to-end tests.

@tatsuya6502
Copy link
Member

I published moka v0.12.0-beta.3 to crates.io.
https://crates.io/crates/moka/0.12.0-beta.3

[dependencies]
moka = { version = "0.12.0-beta.3", features = ["future"] }

I am hoping this is going to be the last beta. If everything goes well, I will release v0.12.0 in a week or so.

This beta.3 has no API change. It has minor improvements on internal implementations:

@tatsuya6502
Copy link
Member

Closing this as completed.

I published moka v0.12.0 to crates.io. It no longer creates its own thread pool.
https://crates.io/crates/moka/0.12.0

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

No branches or pull requests

3 participants