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

Add copy_to_uninit() to all TypedArrays #4340

Merged
merged 1 commit into from
Dec 8, 2024

Conversation

daxpedda
Copy link
Collaborator

@daxpedda daxpedda commented Dec 8, 2024

Currently copying from a TypedArray, e.g. Uint8Array, to a [MaybeUninit<T>] is not possible without going through raw_copy_to_ptr(), making it unsafe.

This PR adds a method copy_to_uninit() to all TypedArrays, making it safe to copy to a [MaybeUninit<T>] and also returns a [T] because all its values are guaranteed to be initialized now.

Copy link
Contributor

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice feature!

@daxpedda daxpedda merged commit c60e807 into rustwasm:main Dec 8, 2024
55 checks passed
newpavlov pushed a commit to rust-random/getrandom that referenced this pull request Dec 9, 2024
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
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

Successfully merging this pull request may close these issues.

2 participants