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

Jump to bottom

util: improve text-decoder performance #45363

Closed
wants to merge 3 commits into from
Closed

util: improve text-decoder performance #45363

wants to merge 3 commits into from

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Nov 7, 2022 •
edited

This pull request fixes previous benchmarks, improves it, and later improves the performance of the input for SharedArrayBuffer and ArrayBuffer on TextDecoder.decode

Based on @jasnell's recommendation

cc @nodejs/performance

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1212/

                                                                                                 confidence improvement accuracy (*)    (**)   (***)
util/text-decoder.js type='ArrayBuffer' n=100 len=16384 ignoreBOM=0 encoding='iso-8859-3'                        7.47 %       ±9.99% ±13.30% ±17.31%
util/text-decoder.js type='ArrayBuffer' n=100 len=16384 ignoreBOM=0 encoding='latin1'                     *     10.43 %       ±8.50% ±11.30% ±14.71%
util/text-decoder.js type='ArrayBuffer' n=100 len=16384 ignoreBOM=0 encoding='utf-8'                             5.30 %       ±6.78%  ±9.04% ±11.81%
util/text-decoder.js type='ArrayBuffer' n=100 len=16384 ignoreBOM=1 encoding='iso-8859-3'                        0.11 %       ±9.96% ±13.25% ±17.24%
util/text-decoder.js type='ArrayBuffer' n=100 len=16384 ignoreBOM=1 encoding='latin1'                            2.40 %       ±8.54% ±11.36% ±14.78%
util/text-decoder.js type='ArrayBuffer' n=100 len=16384 ignoreBOM=1 encoding='utf-8'                             3.43 %       ±8.74% ±11.64% ±15.17%
util/text-decoder.js type='ArrayBuffer' n=100 len=256 ignoreBOM=0 encoding='iso-8859-3'                 ***     32.84 %      ±12.48% ±16.65% ±21.74%
util/text-decoder.js type='ArrayBuffer' n=100 len=256 ignoreBOM=0 encoding='latin1'                     ***     27.25 %      ±10.74% ±14.31% ±18.66%
util/text-decoder.js type='ArrayBuffer' n=100 len=256 ignoreBOM=0 encoding='utf-8'                      ***     36.39 %      ±13.64% ±18.18% ±23.73%
util/text-decoder.js type='ArrayBuffer' n=100 len=256 ignoreBOM=1 encoding='iso-8859-3'                 ***     29.60 %      ±11.42% ±15.21% ±19.81%
util/text-decoder.js type='ArrayBuffer' n=100 len=256 ignoreBOM=1 encoding='latin1'                     ***     26.58 %      ±11.86% ±15.78% ±20.54%
util/text-decoder.js type='ArrayBuffer' n=100 len=256 ignoreBOM=1 encoding='utf-8'                      ***     26.81 %      ±10.96% ±14.58% ±18.98%
util/text-decoder.js type='ArrayBuffer' n=100 len=524288 ignoreBOM=0 encoding='iso-8859-3'                      -3.16 %       ±6.15%  ±8.18% ±10.65%
util/text-decoder.js type='ArrayBuffer' n=100 len=524288 ignoreBOM=0 encoding='latin1'                           2.83 %       ±7.06%  ±9.40% ±12.24%
util/text-decoder.js type='ArrayBuffer' n=100 len=524288 ignoreBOM=0 encoding='utf-8'                           -3.08 %       ±6.04%  ±8.05% ±10.48%
util/text-decoder.js type='ArrayBuffer' n=100 len=524288 ignoreBOM=1 encoding='iso-8859-3'                      -2.60 %       ±7.61% ±10.13% ±13.19%
util/text-decoder.js type='ArrayBuffer' n=100 len=524288 ignoreBOM=1 encoding='latin1'                          -2.27 %       ±7.37%  ±9.80% ±12.76%
util/text-decoder.js type='ArrayBuffer' n=100 len=524288 ignoreBOM=1 encoding='utf-8'                            4.71 %       ±6.94%  ±9.24% ±12.03%
util/text-decoder.js type='Buffer' n=100 len=16384 ignoreBOM=0 encoding='iso-8859-3'                             6.20 %      ±11.38% ±15.15% ±19.73%
util/text-decoder.js type='Buffer' n=100 len=16384 ignoreBOM=0 encoding='latin1'                                 5.15 %       ±9.10% ±12.12% ±15.79%
util/text-decoder.js type='Buffer' n=100 len=16384 ignoreBOM=0 encoding='utf-8'                                  0.88 %      ±11.25% ±14.96% ±19.48%
util/text-decoder.js type='Buffer' n=100 len=16384 ignoreBOM=1 encoding='iso-8859-3'                            -5.50 %       ±9.90% ±13.18% ±17.15%
util/text-decoder.js type='Buffer' n=100 len=16384 ignoreBOM=1 encoding='latin1'                                 1.15 %       ±8.39% ±11.17% ±14.55%
util/text-decoder.js type='Buffer' n=100 len=16384 ignoreBOM=1 encoding='utf-8'                                  3.79 %      ±14.27% ±18.99% ±24.71%
util/text-decoder.js type='Buffer' n=100 len=256 ignoreBOM=0 encoding='iso-8859-3'                               9.18 %      ±11.30% ±15.04% ±19.58%
util/text-decoder.js type='Buffer' n=100 len=256 ignoreBOM=0 encoding='latin1'                            *     15.81 %      ±13.25% ±17.73% ±23.28%
util/text-decoder.js type='Buffer' n=100 len=256 ignoreBOM=0 encoding='utf-8'                                    2.84 %      ±10.10% ±13.45% ±17.51%
util/text-decoder.js type='Buffer' n=100 len=256 ignoreBOM=1 encoding='iso-8859-3'                              -0.79 %      ±11.28% ±15.02% ±19.58%
util/text-decoder.js type='Buffer' n=100 len=256 ignoreBOM=1 encoding='latin1'                                   3.46 %       ±9.30% ±12.38% ±16.13%
util/text-decoder.js type='Buffer' n=100 len=256 ignoreBOM=1 encoding='utf-8'                                    2.41 %       ±9.73% ±12.96% ±16.87%
util/text-decoder.js type='Buffer' n=100 len=524288 ignoreBOM=0 encoding='iso-8859-3'                            6.12 %       ±6.78%  ±9.02% ±11.74%
util/text-decoder.js type='Buffer' n=100 len=524288 ignoreBOM=0 encoding='latin1'                                3.18 %       ±6.18%  ±8.22% ±10.70%
util/text-decoder.js type='Buffer' n=100 len=524288 ignoreBOM=0 encoding='utf-8'                                 0.28 %       ±7.33%  ±9.77% ±12.73%
util/text-decoder.js type='Buffer' n=100 len=524288 ignoreBOM=1 encoding='iso-8859-3'                           -3.97 %       ±6.21%  ±8.27% ±10.76%
util/text-decoder.js type='Buffer' n=100 len=524288 ignoreBOM=1 encoding='latin1'                               -4.25 %       ±6.72%  ±8.94% ±11.63%
util/text-decoder.js type='Buffer' n=100 len=524288 ignoreBOM=1 encoding='utf-8'                                -0.13 %       ±4.92%  ±6.54%  ±8.51%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=16384 ignoreBOM=0 encoding='iso-8859-3'           *     12.96 %      ±10.21% ±13.59% ±17.71%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=16384 ignoreBOM=0 encoding='latin1'               *     15.27 %      ±13.12% ±17.54% ±23.00%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=16384 ignoreBOM=0 encoding='utf-8'                *      9.76 %       ±8.54% ±11.37% ±14.80%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=16384 ignoreBOM=1 encoding='iso-8859-3'                  7.90 %       ±8.94% ±11.92% ±15.55%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=16384 ignoreBOM=1 encoding='latin1'                      5.69 %      ±10.00% ±13.31% ±17.33%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=16384 ignoreBOM=1 encoding='utf-8'                       8.02 %       ±9.13% ±12.15% ±15.82%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=256 ignoreBOM=0 encoding='iso-8859-3'           ***     37.73 %      ±13.23% ±17.71% ±23.27%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=256 ignoreBOM=0 encoding='latin1'               ***     33.53 %      ±10.06% ±13.40% ±17.49%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=256 ignoreBOM=0 encoding='utf-8'                 **     18.71 %      ±10.85% ±14.45% ±18.82%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=256 ignoreBOM=1 encoding='iso-8859-3'           ***     36.62 %      ±10.20% ±13.58% ±17.71%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=256 ignoreBOM=1 encoding='latin1'               ***     29.98 %       ±6.95%  ±9.24% ±12.04%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=256 ignoreBOM=1 encoding='utf-8'                ***     29.35 %       ±9.48% ±12.62% ±16.43%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=524288 ignoreBOM=0 encoding='iso-8859-3'                -0.31 %       ±6.59%  ±8.77% ±11.42%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=524288 ignoreBOM=0 encoding='latin1'                    -0.23 %       ±7.59% ±10.11% ±13.16%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=524288 ignoreBOM=0 encoding='utf-8'                     -0.15 %       ±6.81%  ±9.06% ±11.79%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=524288 ignoreBOM=1 encoding='iso-8859-3'                 0.32 %       ±7.52% ±10.01% ±13.04%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=524288 ignoreBOM=1 encoding='latin1'                    -2.21 %       ±7.33%  ±9.76% ±12.71%
util/text-decoder.js type='SharedArrayBuffer' n=100 len=524288 ignoreBOM=1 encoding='utf-8'                     -1.12 %       ±7.53% ±10.02% ±13.05%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 54 comparisons, you can thus
expect the following amount of false-positive results:
  2.70 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.54 false positives, when considering a   1% risk acceptance (**, ***),
  0.05 false positives, when considering a 0.1% risk acceptance (***)

@anonrig anonrig added performance Issues and PRs related to the performance of Node.js. encoding Issues and PRs related to the TextEncoder and TextDecoder APIs. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Nov 7, 2022
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. i18n-api Issues and PRs related to the i18n implementation. needs-ci PRs that need a full CI run. labels Nov 7, 2022
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 7, 2022
@anonrig
Copy link
Member Author

anonrig commented Nov 7, 2022

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1212/

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 7, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@mscdex
Copy link
Contributor

mscdex commented Nov 7, 2022

FWIW it'd be better to post the results as text instead of a screenshot of the results.

@anonrig
Copy link
Member Author

anonrig commented Nov 7, 2022

FWIW it'd be better to post the results as text instead of a screenshot of the results.

I'll do it after the benchmark ci is completed.

@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 7, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 7, 2022
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47763/

@anonrig anonrig changed the title util: improve text-decoder performance up to 31% util: improve text-decoder performance up to 37% Nov 7, 2022
@tniessen
Copy link
Member

tniessen commented Nov 7, 2022

(Side note: while a good sales strategy, I guess, it is not very scientific to cherry-pick the highest percentage and put it into the title. A different set of benchmarks would likely give you an entirely different result.)

@anonrig anonrig changed the title util: improve text-decoder performance up to 37% util: improve text-decoder performance Nov 7, 2022
@anonrig
Copy link
Member Author

anonrig commented Nov 7, 2022 •
edited

(Side note: while a good sales strategy, I guess, it is not very scientific to cherry-pick the highest percentage and put it into the title. A different set of benchmarks would likely give you an entirely different result.)

I removed it. (This is why I added up to)

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47767/

addaleax
addaleax reviewed Nov 7, 2022
View reviewed changes
lib/internal/encoding.js Show resolved Hide resolved
src/node_i18n.cc Outdated Show resolved Hide resolved
src/util-inl.h Show resolved Hide resolved
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 7, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 7, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
addaleax
addaleax approved these changes Nov 7, 2022
View reviewed changes
src/util-inl.h Outdated Show resolved Hide resolved
src/util-inl.h Outdated Show resolved Hide resolved
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
@anonrig @RafaelGSS
benchmark: add parameters to text-decoder benchmark
c9cf399 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
@anonrig @RafaelGSS
util: improve text-decoder performance
89390a6 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Nov 10, 2022
Merged
This was referenced Nov 11, 2022
Open
Open
anonrig added a commit to anonrig/node that referenced this pull request Nov 27, 2022
@anonrig
util: improve text-decoder performance
c49f781 
PR-URL: nodejs#45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>

# Conflicts:
#	lib/internal/encoding.js
@anonrig anonrig mentioned this pull request Nov 27, 2022
Closed
anonrig added a commit to anonrig/node that referenced this pull request Nov 27, 2022
@anonrig
util: improve text-decoder performance
4b83232 
PR-URL: nodejs#45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
anonrig added a commit to anonrig/node that referenced this pull request Nov 28, 2022
@anonrig
benchmark: fix text-decoder benchmark
f5e3920 
PR-URL: nodejs#45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
anonrig added a commit to anonrig/node that referenced this pull request Nov 28, 2022
@anonrig
benchmark: add parameters to text-decoder benchmark
22e9f5b 
PR-URL: nodejs#45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
anonrig added a commit to anonrig/node that referenced this pull request Nov 28, 2022
@anonrig
util: improve text-decoder performance
26fbc98 
PR-URL: nodejs#45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@anonrig anonrig added lts-watch-v14.x lts-watch-v18.x PRs that may need to be released in v18.x. labels Nov 28, 2022
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@anonrig @danielleadams
benchmark: fix text-decoder benchmark
d8b1f6d 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@anonrig @danielleadams
benchmark: add parameters to text-decoder benchmark
ec2a14b 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@anonrig @danielleadams
util: improve text-decoder performance
c945b5d 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@danielleadams danielleadams mentioned this pull request Dec 30, 2022
Merged
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@anonrig @danielleadams
benchmark: fix text-decoder benchmark
ac457a4 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@anonrig @danielleadams
benchmark: add parameters to text-decoder benchmark
d86be20 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@anonrig @danielleadams
util: improve text-decoder performance
96c9525 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
@anonrig @danielleadams
benchmark: fix text-decoder benchmark
ab891ec 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
@anonrig @danielleadams
benchmark: add parameters to text-decoder benchmark
762d285 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
@anonrig @danielleadams
util: improve text-decoder performance
1dc91ab 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
@anonrig @danielleadams
benchmark: fix text-decoder benchmark
3857f39 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
@anonrig @danielleadams
benchmark: add parameters to text-decoder benchmark
cf21533 
PR-URL: #45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@targos targos added backported-to-v18.x PRs backported to the v18.x-staging branch. and removed lts-watch-v18.x PRs that may need to be released in v18.x. labels Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR left review comments

@Trott Trott Trott approved these changes

@addaleax addaleax addaleax approved these changes

Assignees
No one assigned
Labels
backported-to-v18.x PRs backported to the v18.x-staging branch. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. encoding Issues and PRs related to the TextEncoder and TextDecoder APIs. i18n-api Issues and PRs related to the i18n implementation. needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet