-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
TextDecoder is slow. #39879
Comments
Yeah, it was implemented to be spec compliant and not really to be performant. I'm sure there is lots of room for improvement. I'd love to see someone take on that work. |
What I found in the mentioned issue is that the time is mostly spent in this function: Line 432 in e2148d7
|
In my machine, the test code ran in Chrome results in: 49 and 834ms. But Node.js results in 113 and 1730ms. When I add two lines in args.GetReturnValue().Set(Buffer::New(env, 0).ToLocalChecked());
return; That means DO NOT do the left logic after decoding via |
btw, it seems |
All I/O optimizations are multiplied by zero, when decoding text, this main cases for clients - Redis, PostgreSQL, etc... |
It seems #42695 fixes this issue. node
chrome
|
Here's the output before after 2 pull requests I opened on TextDecoder: Before (v18.11.0):
After (Main):
|
Bun:
Deno:
|
@srl295 do you have any suggestions for optimizations on large values? |
@srl295 ... as a reminder for context... this is making use of the |
Nodejs text encoding/decoding is slow: nodejs/node#39879 It's faster to encode in JS. Browsers don't show much difference with this change.
Nodejs text encoding/decoding is slow: nodejs/node#39879 It's faster to encode in JS. Browsers don't show much difference with this change.
in #39301 we talked about marking string_decoder as legacy but we also brougth up the perf of String_decoder vs TextDecoder, but we didn't talk much about if there was something wrong with TextDecoder and how it performed agains other environments and why it's slower. or if it can be approved upon.
I conducted a test that runs 10k times in different environment and tested to see how fast it is able to do it's job and nodejs didn't do a good job...
here is the results i got
(Lower is better)
buffer.toString('utf8')
andstring_decoder
is considerable much faster, maybe you can steel something from it and apply it onto the TextDecoder? or do exactly what safari is doing :)And the test i run:
The text was updated successfully, but these errors were encountered: