Address multi-platform issues with getrandom dependency

[aspect]

With regards to the closed #172

This is the best solution I have been able to come up with. I will give this example to cargo developers. In the meantime, the way to address this would be to migrate getrandom from dev-dependencies to regular dependencies. It will be eliminated away by the compiler and linker as it is not used anyways. In the meantime it allows one to use js feature.

Once again, to clarify, js features is needed for wasm but you can not specify sub-crate features for dev-dependencies as in release mode cargo errors out with no such dependency found for feature 'js'...

I've tried using js = ["getrandom?/js"] syntax, but optional dependencies (?) are available only on the nightly channel and that would require to set getrandom = { optional = true ... } and then use another feature like 'testing' to trigger its inclusion, which is complex and doesn't make much sense anyways. Relocating this crate from dev-dependencies is the cleanest and most elegant solution for the time being.

[tatsuya6502]

Hi. Thanks for opening this pull request.

getrandom is used by only one unit test. I am not sure what would be the best way to do this, but probably having "testing" feature or something will be better?

I think it would be solvable by adding "testing" feature and migrating dev-dependencies under [target.'cfg(feature = "testing").dependencies], but not sure.

or migrating it under [target.'cfg(feature = "testing").dev-dependencies]?

Let me do some research on it too.

Also, which cache are you trying to use? (moka::unsync, moka::sync or moka::future) I have not tried but I think only moka::unsync::Cache could work in wasm environment? Other caches will spawn background threads and wasm does not support multi-threads yet I think. They also heavily depend on std::sync::atomic::* types, and I am not sure if they are available in the wasm target.

[tatsuya6502]

As for getrandom, perhaps the followings would work?

[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]
getrandom = { version = "0.2" }

[target.'cfg(target_arch = "wasm32")'.dev-dependencies]
getrandom = { version = "0.2", features = ["js"] }

[aspect]

I am using unsync because wasm32-unknown-unknown (browser) doesn't have threads and sync just crashes in the builder, which is somewhat expected.

I actually ended up wrapping moka into our own interface that does #[cfg(target_arch = "...")] and based on that provides sync or unsync instances as the libraries we are working on are multi-target.

The funny thing is that because we are using futures in wasm32 (backed by wasm/js executor), rust requires thread safety, yet unsync requires &mut self in its calls, so I ended up having Arc<Mutex<unsync::Cache>>. :)

I haven't tried futures as it seems to have tokio as a dependency and I know this won't compile. async_std works well on wasm32, except there is no block_on type functionality as one can't block due to a single-threaded model. (Which reminds me, I haven't looked, but I am wondering how futures would fair in a single-threaded executor.). If futures doesn't have tokio, resides on top of async_std and doesn't use any thread blocking, I'd use that.

Back to the issue at hand:

Keep in mind that uuid/js needs to be triggered for wasm32 as well, so you do need to have a js feature for it to cascade to both, getrandom and uuid crates.

However, the core of the issue is that after adding js features to accommodate wasm32 support, I have hit another issue for a different target bpfel-unknown-unknown (a.k.a. bpf). (bpf target is used by Solana blockchain for on-chain programs (a.k.a) smart contracts, but this will apply to any other target_arch as well when built as a release i.e. without dev-dependencies).

What happens is that the release mode excludes all dev-dependencies during the cargo pre-processing (crate analysis) stage. Cargo then chokes with invalid feature as getrandom does not exist...

There is currently no way around that. I tried making dev-dependencies as optional in hope that getrandom?/js would work, but dev-dependencies can not be made optional (and that wouldn't address the core issue or dev-dependencies going missing in release).

To summarize: the js features is needed for getrandom/js and uuid/js but adding it causes breakage on other targets when building in release due to dev-dependencies being excluded and thus js feature becoming invalid, breaking the build.

So this currently ends up being that in all cases to add js feature, getrandom needs to be elevated from dev-dependencies into dependencies... I.e. basically, one currently can not have features that target dev-dependencies (rust-lang/cargo#1596).

The alternative solution to this issue would be to replace getrandom with rand (https://crates.io/crates/rand). Could be a more elegant solution as it will eliminate getrandom completely. This way js feature would remain only for uuid/js and uuid is fine as it is a regular dependency.

I can set up a small repo that demonstrates the issue if you want. Let me know.

[tatsuya6502]

Thanks for providing the details.

Keep in mind that uuid/js needs to be triggered for wasm32 as well, so you do need to have a js feature for it to cascade to both, getrandom and uuid crates.

I am OK for adding the js feature to moka to specify uuid/js.

But I believe we do not need to specify getrandom/js because uuid will take care of enabling the js feature of getrandom in your package's dependency tree. If I understand correctly, your package does not use getrandom under moka's dev-dependencies.

To confirm this, I created a modified version of moka and a package that depends on moka: https://gitlab.com/moka-labs/moka-gh-173-js-feature

This moka has a js feature in Cargo.toml, which only specifies uuid/js.
https://gitlab.com/moka-labs/moka-gh-173-js-feature/moka/-/compare/master...js-feature

[features]
# ...

js = ["uuid/js"]

And the package (called my-package1) depends on that moka. It has a js feature too to specify moka/js:
https://gitlab.com/moka-labs/moka-gh-173-js-feature/my-package1/-/blob/main/Cargo.toml

[features]
js = ["moka/js"]

[dependencies]
moka = { path = "../moka" }

Then I confirmed that I can build my-package1 for the default and wasm32-unknown-unknown targets (in both debug and release modes):

$ git clone https://gitlab.com/moka-labs/moka-gh-173-js-feature/moka.git
$ git clone https://gitlab.com/moka-labs/moka-gh-173-js-feature/my-package1.git

$ cd my-package1

## Build for the default target. (For my case, aarch64-apple-darwin)
$ cargo build
$ cargo build --release

## Build for wasm32-unknown-unknown target:
$ cargo build --target wasm32-unknown-unknown --features js
$ cargo build --target wasm32-unknown-unknown --features js --release

Can you please check if this works for your package? (I mean: only adding js = ["uuid/js"] to moka's Cargo.toml).

[tatsuya6502]

I am using unsync because wasm32-unknown-unknown (browser) doesn't have threads and sync just crashes in the builder, which is somewhat expected.

That is what I thought.

I actually ended up wrapping moka into our own interface that does #[cfg(target_arch = "...")] and based on that provides sync or unsync instances as the libraries we are working on are multi-target.

The funny thing is that because we are using futures in wasm32 (backed by wasm/js executor), rust requires thread safety, yet unsync requires &mut self in its calls, so I ended up having Arc<Mutex<unsync::Cache>>. :)

As for the threads part, you can make sync::Cache not to spawn background threads by doing all the followings:

• Call CacheBuilder's thread_pool_enabled method with false. (doc)
• Do not call CacheBuilder's support_invalidation_closures method.
  (doc)
  • This means you cannot use Cache's invalidate_entries_if method.
• If you want to set the eviction listener to the cache, use the Immediate delivery mode (which is the default mode). (doc)

Maybe it is worth to try. (but I do not know if there are any other problems)

I haven't tried futures as it seems to have tokio as a dependency and I know this won't compile. async_std works well on wasm32, except there is no block_on type functionality as one can't block due to a single-threaded model. (Which reminds me, I haven't looked, but I am wondering how futures would fair in a single-threaded executor.). If futures doesn't have tokio, resides on top of async_std and doesn't use any thread blocking, I'd use that.

future::Cache should not work for wasm32-unknown-unknown target. It currently (v0.9.x) does not use tokio but uses async-io and async-lock crates for sleep timers and locks respectively. And async-io spawns a background thread to manage timers. future::Cache also spawns the same background threads to sync::Cache, but unlike sync::Cache, you cannot disable them.

In the future, we will remove the background threads and async-io and async-lock crates from future::Cache, and make it to use user-provided async runtime. Like sqlx crate, you will specify the async runtime you use, such as async-std, tokio and actix-rt, via crate feature. If you tell me what async runtime you use in web browser, I will take a look at it too.

[tatsuya6502]

If you could create a tiny wasm project for me to play with moka on a web browser, that will be a great help. I will use it to figure out what needs to be done to make sync or future cache to work.

[tatsuya6502]

As for the threads part, you can make sync::Cache not to spawn background threads by doing all the followings:

• Call CacheBuilder's thread_pool_enabled method with false. (doc)
• Do not call CacheBuilder's support_invalidation_closures method. (doc)
  • This means you cannot use Cache's invalidate_entries_if method.
• If you want to set the eviction listener to the cache, use the Immediate delivery mode (which is the default mode). (doc)

I created a mini wasm32-unknown-unknown project and tried the above with moka having the js feature. I ran it on Firefox browser, and was able to create a sync::Cache instance (without panic) and do insert, get and invalidate.

I will do little further to check if some of the advanced functions such as time-to-live expiration and eviction listener are working in a sync::Cache instance.

[tatsuya6502]

I am going to merge this PR. But I will create another PR to revert the change on getrandom dependency because it broke one of the unit tests and I believe the change is unnecessary.

Thank you for contributing to Moka 😄 !