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(ext/url): use DOMString webidl instead of USVString #11775

Merged
merged 1 commit into from
Aug 19, 2021

Conversation

AaronO
Copy link
Contributor

@AaronO AaronO commented Aug 19, 2021

Improves url parsing by ~20%:

  • before: ~2450ns/parse
  • after: ~1950ns/parse

This is safe since all these string values are immediately sent to the op-layer that uses v8::String::to_rust_string_lossy() with REPLACE_INVALID_UTF8 effectively doing the same transform as our USVString webidl converter

… for URL parsing

A 20% decrease in url parsing:
- before: `~2450ns/parse`
- after: `~1950ns/parse`
@lucacasonato
Copy link
Member

The same probably applies to URLSearchParams too

@AaronO
Copy link
Contributor Author

AaronO commented Aug 19, 2021

The same probably applies to URLSearchParams too

Only partially, since URLSearchParams has pure JS getters (see its get()/has()), so if we changed the converters but left the code otherwise as is, there would be a change in behaviour since it's possible to have a !== b yet USVString(a) === USVString(b)

So I decided to leave it as is, since we'll want to make other tweaks beyond just switching the webidl converter as for URL

@lucacasonato
Copy link
Member

Makes sense. We could do it in the constructor though (that goes right into a parse op).

@AaronO AaronO merged commit 9105104 into denoland:main Aug 19, 2021
@AaronO AaronO deleted the perf/ext-url-usvstring branch August 19, 2021 15:36
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