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

util: improve text-decoder performance #45363

Closed
wants to merge 3 commits into from

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Nov 7, 2022

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

@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.

@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

@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

(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

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.

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
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
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
anonrig added a commit to anonrig/node that referenced this pull request Nov 27, 2022
PR-URL: nodejs#45363
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>

# Conflicts:
#	lib/internal/encoding.js
anonrig added a commit to anonrig/node that referenced this pull request Nov 27, 2022
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
Development

Successfully merging this pull request may close these issues.