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 From impl for TypedArrays #1620

Merged
merged 5 commits into from
Jun 26, 2019
Merged

Conversation

Pauan
Copy link
Contributor

@Pauan Pauan commented Jun 24, 2019

There's been quite a few issues (1, 2, 3) about TypedArrays and Rust slices.

Right now the only way to convert a Rust slice into a TypedArray is to use the unsafe view function. But it's common for people to make mistakes when using it, and those mistakes are quite bad (since it's unsafe).

So this PR adds in safe From impls for all of the TypedArrays, allowing for easy, safe, and fast conversion from Rust slices.

@alexcrichton
Copy link
Contributor

Seems reasonable to me! Did you want to measure some of the TODO before merging though?

@Pauan
Copy link
Contributor Author

Pauan commented Jun 25, 2019

I ran some benchmarks (sending a 2,560,000 length slice into JS), and the differences are negligible:

Chrome:
  new           309.59499999880790ms  (100%)
  slice len     326.13338709568546ms  (105.34194640191212%)
  slice length  328.74645161322286ms  (106.18597372547494%)
  raw js        330.33241935434840ms  (106.698245848423%)
  slice         331.26806451248063ms  (107.00046191439563%)

Firefox:
  raw js        343.23333333333335ms  (100%)
  slice         344.03333333333336ms  (100.23307780432312%)
  slice len     346.96551724137930ms  (101.08736153827171%)
  slice length  355.10344827586210ms  (103.4583254098343%)
  new           361.75000000000000ms  (105.3947800125618%)

So I went ahead and removed the TODOs.

@alexcrichton alexcrichton merged commit 250e84f into rustwasm:master Jun 26, 2019
@alexcrichton
Copy link
Contributor

👍

@Pauan Pauan deleted the add-typed-array-into branch June 26, 2019 06:46
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.

None yet

2 participants