Add `wasm32v1-none` support #541

daxpedda · 2024-11-21T10:36:55Z

Changes:

Add default-features = false to the js-sys and wasm-bindgen-test dependencies, enabling no_std support.
The crate feature std now enabled std crate features of wasm-bindgen, js-sys and wasm-bindgen-test.
Use any(target_os = "unknown", target_os = "none") instead of target_os = "unknown".
thread_local! is only supported with std, so a fallback to static with once_cell::unsync::Lazy is added. This is safe because Wasm doesn't support threads without target_feature = "atomics".
When detecting target_feature = "atomics" without feature = "std", use #[thread_local].
Now wasm32-unknown-unknown is properly supported without pulling in std.

I was unable to add actual no_std tests without some ugly modifications to it. Especially because of the whole harness = false thing. Let me know how you want me to proceed.

~~This depends on rustwasm/wasm-bindgen#4277 and a new wasm-bindgen release.~~
Was released.

daxpedda · 2024-11-21T12:08:47Z

Current CI failure is unrelated.

newpavlov · 2024-11-21T15:46:14Z

The CI failures are fixed in #542.

daxpedda · 2024-11-25T07:55:46Z

Hm, CI still has an unrelated failure.

newpavlov · 2024-11-25T09:40:51Z

Yes, see rust-lang/rust#133401. We may disable the GNU/Hurd job since it's not the first time the std breaks this target.

UPD: Fixed in #543.

daxpedda · 2024-11-29T18:16:51Z

Rebased after #551 was merged.

newpavlov · 2024-12-02T14:27:12Z

How expensive is the getrandom_init call in practice? Maybe we should just skip the thread-local dance and obtain fresh RngSource on each call? As a small optimization we could cache the source kind in an atomic static.

Also, is it common to use one (non-trivial) WASM file in both Web and Node environments simultaneously, i.e. maybe we should split wasm_js into wasm_web_js and wasm_node_js?

daxpedda · 2024-12-04T16:46:26Z

How expensive is the getrandom_init call in practice? Maybe we should just skip the thread-local dance and obtain fresh RngSource on each call? As a small optimization we could cache the source kind in an atomic static.

For Web that seems reasonable. NodeJS seems to be quite involved and I would argue that a thread-local is justified.

EDIT: I will do some tests and get back to you.

Also, is it common to use one (non-trivial) WASM file in both Web and Node environments simultaneously, i.e. maybe we should split wasm_js into wasm_web_js and wasm_node_js?

AFAIK it is common to do it together. I'm not NodeJS expert, but whats strange to me is that the implementation is different to begin with.

newpavlov · 2024-12-04T18:02:42Z

I've opened #557 to explore how such simplification could look like. Could you measure whether there is a measurable performance impact or not?

.github/workflows/tests.yml

daxpedda · 2024-12-04T20:01:17Z

I've opened #557 to explore how such simplification could look like. Could you measure whether there is a measurable performance impact or not?

I did some local benchmarking with Firefox and Chrome and I end up with around 40ns. Its only one FFI call after all, js_sys::global() is cached with a thread_local internally.

NodeJS is around 3 calls (I replaced the "crypto" string with a #[wasm_bindgen(static_string)]). However its generally a bit slower then more up-to-date browsers and ends up around 340ns.

However, I didn't dig into the specifics on why the NodeJS implementation is so special, the API should be the same. My local testing shows that NodeJS works just fine with the browser API.

Browser: 40ns
NodeJS: 340ns
NodeJS with Browser API: 45ns

So all in all I think this is acceptable. I will add the implementation to that effect to this PR tonight.

I also had a couple of optimizations in mind:

Replace the "crypto" string in the NodeJS implementation with a #[wasm_bindgen(static_string)] cached JsString.
Get rid of the Uint8Array when not using shared memory.
Cache the Crypto object via a #[wasm_bindgen(thread_local)]. This should basically put us back to the custom thread-local implementation before falling back to the current implementation, which I'm still unsure who it is for actually.

newpavlov · 2024-12-04T20:46:44Z

I didn't dig into the specifics on why the NodeJS implementation is so special, the API should be the same.

It's probably because a full Web Crypto support was added only in Node.js 19 (2022-10-18) and getRandomValues() was techincally supported since 17.4 (2022-01-18).

We had releases with WASM support a fair time before that, so we just kept the code as-is without considering whether the fallback is needed or not. I guess we can just drop the alternative Node.js path completely for the v0.3 release.

I will add the implementation to that effect to this PR tonight.

We also can continue with #557. I will leave the choice to you.

Get rid of the Uint8Array when not using shared memory.

Can you demonstrate how this detection could look like?

daxpedda · 2024-12-04T21:46:04Z

I will add the implementation to that effect to this PR tonight.

We also can continue with #557. I will leave the choice to you.

Sure, I'm a fan of splitting things up.

Get rid of the Uint8Array when not using shared memory.

Can you demonstrate how this detection could look like?

We can simply use wasm_bindgen::memory() to determine if the WebAssembly.Memory.buffer is an ArrayBuffer or a SharedArrayBuffer. Afterwards we can easily cache it in a static atomic.

daxpedda · 2024-12-04T22:37:57Z

Closing in favor of #557.

In #557 we forgot to port the `no_std` support from #541. This also adds running Clippy for `wasm32v1-none`.

As discussed in #541. This PR adds the following improvements: - Caching of the global `Crypto` object. - Detecting if our Wasm memory is based on a `SharedArrayBuffer`. If not, we can copy bytes directly into our memory instead of having to go through JS. This saves allocating the buffer in JS and copying the bytes into Wasm memory. This is also the most common path. `SharedArrayBuffer` requires `target_feature = "atomics"`, which is unstable and requires Rust nightly. See #559 (comment) for full context. - The atomic path only creates a sub-array when necessary, potentially saving another FFI call. - The atomic path will now allocate an `Uint8Array` with the minimum amount of bytes necessary instead of a fixed size. - The maximum chunk size for the non-atomic path and the maximum `Uint8Array` size for the atomic paths have been increased to 65536 bytes: the maximum allowed buffer size for `Crypto.getRandomValues()`. All in all this should give a performance improvement of ~5% to ~500% depending on the amount of requested bytes and which path is taken. See #559 (comment) for some benchmark results. This spawned a bunch of improvements and fixes in `wasm-bindgen` that are being used here: - rustwasm/wasm-bindgen#4315 - rustwasm/wasm-bindgen#4316 - rustwasm/wasm-bindgen#4318 - rustwasm/wasm-bindgen#4319 - rustwasm/wasm-bindgen#4340

daxpedda force-pushed the wasm32v1-none branch 4 times, most recently from 6002c05 to 48d38ce Compare November 21, 2024 11:56

daxpedda force-pushed the wasm32v1-none branch 4 times, most recently from f18f3b3 to ff7a200 Compare November 25, 2024 07:48

daxpedda mentioned this pull request Nov 28, 2024

Add no_std support daxpedda/web-time#39

Merged

daxpedda force-pushed the wasm32v1-none branch 2 times, most recently from 84af741 to 604082a Compare November 29, 2024 17:35

daxpedda marked this pull request as ready for review November 29, 2024 17:43

daxpedda mentioned this pull request Nov 29, 2024

Remove faulty support for wasm64-unknown-unknown #551

Merged

daxpedda force-pushed the wasm32v1-none branch from 604082a to a48e020 Compare November 29, 2024 18:16

daxpedda force-pushed the wasm32v1-none branch from a48e020 to 56bd92a Compare November 29, 2024 18:18

daxpedda added 2 commits December 4, 2024 17:31

Implement no_std support for Wasm

c732ef0

Add wasm32v1-none support

011d707

daxpedda force-pushed the wasm32v1-none branch from 56bd92a to 011d707 Compare December 4, 2024 16:44

newpavlov reviewed Dec 4, 2024

View reviewed changes

.github/workflows/tests.yml Show resolved Hide resolved

newpavlov mentioned this pull request Dec 4, 2024

wasm_js: remove the separate codepath for Node.js and TLS caching #557

Merged

daxpedda closed this Dec 4, 2024

This was referenced Dec 5, 2024

Web optimizations #559

Merged

Add no_std support for Web #560

Merged

newpavlov pushed a commit that referenced this pull request Dec 5, 2024

Add no_std support for Web (#560)

ae0c807

In #557 we forgot to port the `no_std` support from #541. This also adds running Clippy for `wasm32v1-none`.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `wasm32v1-none` support #541

Add `wasm32v1-none` support #541

daxpedda commented Nov 21, 2024 •

edited

Loading

daxpedda commented Nov 21, 2024

newpavlov commented Nov 21, 2024

daxpedda commented Nov 25, 2024

newpavlov commented Nov 25, 2024 •

edited

Loading

daxpedda commented Nov 29, 2024

newpavlov commented Dec 2, 2024 •

edited

Loading

daxpedda commented Dec 4, 2024 •

edited

Loading

newpavlov commented Dec 4, 2024 •

edited

Loading

daxpedda commented Dec 4, 2024

newpavlov commented Dec 4, 2024 •

edited

Loading

daxpedda commented Dec 4, 2024

daxpedda commented Dec 4, 2024

Add wasm32v1-none support #541

Add wasm32v1-none support #541

Conversation

daxpedda commented Nov 21, 2024 • edited Loading

daxpedda commented Nov 21, 2024

newpavlov commented Nov 21, 2024

daxpedda commented Nov 25, 2024

newpavlov commented Nov 25, 2024 • edited Loading

daxpedda commented Nov 29, 2024

newpavlov commented Dec 2, 2024 • edited Loading

daxpedda commented Dec 4, 2024 • edited Loading

newpavlov commented Dec 4, 2024 • edited Loading

daxpedda commented Dec 4, 2024

newpavlov commented Dec 4, 2024 • edited Loading

daxpedda commented Dec 4, 2024

daxpedda commented Dec 4, 2024

Add `wasm32v1-none` support #541

Add `wasm32v1-none` support #541

daxpedda commented Nov 21, 2024 •

edited

Loading

newpavlov commented Nov 25, 2024 •

edited

Loading

newpavlov commented Dec 2, 2024 •

edited

Loading

daxpedda commented Dec 4, 2024 •

edited

Loading

newpavlov commented Dec 4, 2024 •

edited

Loading

newpavlov commented Dec 4, 2024 •

edited

Loading