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

performance of Uint8Array.prototype.setFrom{Base64,Hex} #57

Closed
phoddie opened this issue Apr 12, 2024 · 3 comments
Closed

performance of Uint8Array.prototype.setFrom{Base64,Hex} #57

phoddie opened this issue Apr 12, 2024 · 3 comments

Comments

@phoddie
Copy link

phoddie commented Apr 12, 2024

The excellent notes from @anba on additional tests made me realize that XS does not implement these methods as expected by the spec text.

Uint8Array.prototype.setFrom{Base64,Hex} doesn't modify the typed array when the input string is invalid.

My recollection is that BYOB was proposed by @syg as a performance optimization. Requiring full validation of the input before modifying the output reduces the performance gain: either two full passes over the source data are necessary or a temporary buffer is needed to store the decoded data before updating the destination.

If leaving the destination unmodified for invalid input takes priority, that's fine. But, since performance was the motivator, I want to check before making changes. I couldn't find previous discussion about this tradeoff.

@bakkot
Copy link
Collaborator

bakkot commented Apr 15, 2024

I originally wrote it this way because it's more consistent with a general philosophy of "validate the input, then process it", but it does seem wasteful to require a second pass or temp buffer, especially in the common case of valid input. I don't think there was any discussion of this.

I'll put an item on the agenda for the next meeting.

@bakkot
Copy link
Collaborator

bakkot commented Apr 15, 2024

PR: #58

@bakkot
Copy link
Collaborator

bakkot commented Aug 8, 2024

Done in #58, tests in tc39/test262#4133.

@bakkot bakkot closed this as completed Aug 8, 2024
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

No branches or pull requests

2 participants