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

perf(web): fast path for utf8 decoding - part 1 #15910

Closed
wants to merge 1 commit into from

Conversation

littledivy
Copy link
Member

  • Avoid copying buffers.

https://encoding.spec.whatwg.org/#dom-textdecoder-decode

Implementations are strongly encouraged to use an implementation strategy that avoids this copy. When doing so they will have to make sure that changes to input do not affect future calls to decode().

  • Special op to avoid string label deserialization and parsing. (Ideally we should map labels to integers in JS)
  • Avoid webidl Object.assign when options is undefined.

Upto ~1.8x improvement on small data.

# This patch
time 1893 ms rate 5282620
time 2067 ms rate 4837929
# main
time 3284 ms rate 3045066
time 3252 ms rate 3075030

Comment on lines +97 to +103
if (options !== undefined) {
options = webidl.converters.TextDecodeOptions(options, {
prefix,
context: "Argument 2",
});
stream = options.stream;
}
Copy link
Member

Choose a reason for hiding this comment

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

This change should be made into a more general change in the WebIDL dictionary converter. That way it automatically applies to all WebIDL dictionary converters, not just this one.

)
) {
// We clone the data into a non-shared ArrayBuffer so we can pass it
// to Rust.
// `input` is now a Uint8Array, and calling the TypedArray constructor
// with a TypedArray argument copies the data.
input = new Uint8Array(input);
input = new Uint8Array(input.buffer ? input : new Uint8Array(input));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Double new Uint8Array(newUint8Array(input))?

Copy link

@ghost ghost Sep 15, 2022

Choose a reason for hiding this comment

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

Which is a copy... but arguably not too obvious?

Wait, mb, double copy - that's odd.

I'd think it's expecting input: SharedArrayBuffer, so .buffer is false, then constructs an TA in the second branch, then copies it.

@littledivy
Copy link
Member Author

Revived by #16593

@littledivy littledivy closed this Nov 11, 2022
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.

3 participants