url: improve URLSearchParams creation performance by anonrig · Pull Request #47190 · nodejs/node · GitHub

Summary

Validation: We didn't check for the length of the iterable objects passed to the URLSearchParams. This pull request adds that.
Performance: My local benchmarks show 45 to 80% faster URLSearchParams creation.

Footnote

There is one particular change made regarding to a comment written in 2017:

// Note: per spec we have to first exhaust the lists then process them

Upon investigating URL spec, I couldn't find any specific reasoning for this and removed it. The spec mentions that:

To initialize a [URLSearchParams](https://url.spec.whatwg.org/#urlsearchparams) object query with init, run these steps:

If init is a [sequence](https://webidl.spec.whatwg.org/#idl-sequence), then [for each](https://infra.spec.whatwg.org/#list-iterate) innerSequence of init:

If innerSequence’s [size](https://infra.spec.whatwg.org/#list-size) is not 2, then [throw](https://webidl.spec.whatwg.org/#dfn-throw) a [TypeError](https://webidl.spec.whatwg.org/#exceptiondef-typeerror).

[Append](https://infra.spec.whatwg.org/#list-append) (innerSequence[0], innerSequence[1]) to query’s [list](https://url.spec.whatwg.org/#concept-urlsearchparams-list).

Benchmark Result

                                                                                     confidence improvement accuracy (*)   (**)   (***)
url/url-searchparams-creation.js n=10000 inputType='iterable' type='array'                  ***     45.05 %       ±6.82% ±9.19% ±12.19%
url/url-searchparams-creation.js n=10000 inputType='iterable' type='encodelast'             ***     81.96 %       ±2.30% ±3.08%  ±4.06%
url/url-searchparams-creation.js n=10000 inputType='iterable' type='encodemany'             ***     83.22 %       ±1.73% ±2.32%  ±3.05%
url/url-searchparams-creation.js n=10000 inputType='iterable' type='multiprimitives'        ***     80.23 %       ±2.69% ±3.58%  ±4.66%
url/url-searchparams-creation.js n=10000 inputType='iterable' type='noencode'               ***     80.30 %       ±2.55% ±3.42%  ±4.49%
url/url-searchparams-creation.js n=10000 inputType='object' type='array'                             1.47 %       ±2.71% ±3.62%  ±4.74%
url/url-searchparams-creation.js n=10000 inputType='object' type='encodelast'                **      2.78 %       ±1.72% ±2.30%  ±3.00%
url/url-searchparams-creation.js n=10000 inputType='object' type='encodemany'                       -0.26 %       ±3.28% ±4.41%  ±5.82%
url/url-searchparams-creation.js n=10000 inputType='object' type='multiprimitives'          ***      2.76 %       ±1.21% ±1.62%  ±2.11%
url/url-searchparams-creation.js n=10000 inputType='object' type='noencode'                          1.39 %       ±1.59% ±2.12%  ±2.76%
url/url-searchparams-creation.js n=10000 inputType='string' type='array'                             2.38 %       ±5.54% ±7.46%  ±9.88%
url/url-searchparams-creation.js n=10000 inputType='string' type='encodelast'                        1.04 %       ±1.99% ±2.65%  ±3.47%
url/url-searchparams-creation.js n=10000 inputType='string' type='encodemany'                       -0.97 %       ±1.19% ±1.58%  ±2.06%
url/url-searchparams-creation.js n=10000 inputType='string' type='multiprimitives'                  -1.60 %       ±1.67% ±2.23%  ±2.90%
url/url-searchparams-creation.js n=10000 inputType='string' type='noencode'                          0.74 %       ±4.75% ±6.37%  ±8.40%

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

cc @nodejs/url

Review requested:

@nodejs/url

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

@mscdex afaik, benchmark ci is down...

@anonrig Wait for it to come back up or ask @nodejs/build about it?

@anonrig Wait for it to come back up or ask @nodejs/build about it?

Definitely, @mscdex. I created an issue: nodejs/build#3245

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

@nodejs/url please review

Can we move this branch to be with the catch-all else? You'll need to change the if clause below to

 if (pair == null ||
     typeof pair !== 'object' && typeof pair !== 'function' ||
     typeof pair[SymbolIterator] !== 'function')) {

but it reads a bit nicer.

@mscdex I started a benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1316/

@jasnell @nodejs/url Can you review?

url/url-searchparams-creation.js n=1000000 inputType='iterable' type='array'                                           ***     28.65 %       ±3.66%  ±4.87%  ±6.34%
url/url-searchparams-creation.js n=1000000 inputType='iterable' type='encodelast'                                      ***     76.70 %       ±5.39%  ±7.22%  ±9.49%
url/url-searchparams-creation.js n=1000000 inputType='iterable' type='encodemany'                                      ***     76.64 %       ±5.16%  ±6.92%  ±9.11%
url/url-searchparams-creation.js n=1000000 inputType='iterable' type='multiprimitives'                                 ***     72.97 %       ±5.19%  ±6.94%  ±9.10%
url/url-searchparams-creation.js n=1000000 inputType='iterable' type='noencode'                                        ***     82.60 %       ±5.46%  ±7.32%  ±9.64%
url/url-searchparams-creation.js n=1000000 inputType='object' type='array'                                                      1.02 %       ±3.15%  ±4.20%  ±5.46%
url/url-searchparams-creation.js n=1000000 inputType='object' type='encodelast'                                                -3.32 %       ±3.88%  ±5.16%  ±6.72%
url/url-searchparams-creation.js n=1000000 inputType='object' type='encodemany'                                                -0.23 %       ±2.83%  ±3.76%  ±4.89%
url/url-searchparams-creation.js n=1000000 inputType='object' type='multiprimitives'                                            0.63 %       ±3.51%  ±4.66%  ±6.07%
url/url-searchparams-creation.js n=1000000 inputType='object' type='noencode'                                                   1.00 %       ±3.57%  ±4.76%  ±6.21%
url/url-searchparams-creation.js n=1000000 inputType='string' type='array'                                                      1.00 %       ±4.28%  ±5.70%  ±7.41%
url/url-searchparams-creation.js n=1000000 inputType='string' type='encodelast'                                                -2.44 %       ±2.92%  ±3.89%  ±5.06%
url/url-searchparams-creation.js n=1000000 inputType='string' type='encodemany'                                                -2.24 %       ±2.74%  ±3.64%  ±4.74%
url/url-searchparams-creation.js n=1000000 inputType='string' type='multiprimitives'                                           -1.51 %       ±3.47%  ±4.61%  ±6.00%
url/url-searchparams-creation.js n=1000000 inputType='string' type='noencode'                                                  -0.19 %       ±3.33%  ±4.42%  ±5.76%

Landed in bf41f76

PR-URL: #47190 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

PR-URL: nodejs#47190 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

nodejs-github-bot added needs-ci PRs that need a full CI run. whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Mar 21, 2023

anonrig added the performance Issues and PRs related to the performance of Node.js. label Mar 21, 2023

anonrig force-pushed the url-search-params-improve branch from 7ed296b to 3bb3f8c Compare March 21, 2023 13:59

anonrig added url Issues and PRs related to the legacy built-in url module. request-ci Add this label to start a Jenkins CI on a PR. labels Mar 21, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 21, 2023

anonrig force-pushed the url-search-params-improve branch from 3bb3f8c to cfe7a49 Compare March 21, 2023 15:58

mscdex added the needs-benchmark-ci PR that need a benchmark CI run. label Mar 21, 2023

anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 21, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 21, 2023

anonrig added the review wanted PRs that need reviews. label Mar 22, 2023

anonrig requested a review from jasnell March 22, 2023 15:51

joyeecheung reviewed Mar 24, 2023

View reviewed changes

Comment thread lib/internal/url.js Outdated

watilde reviewed Mar 24, 2023

View reviewed changes

Comment thread lib/internal/url.js Outdated

watilde reviewed Mar 24, 2023

View reviewed changes

Comment thread lib/internal/url.js Outdated

url: improve URLSearchParams creation performance

b2cd992

anonrig force-pushed the url-search-params-improve branch from cfe7a49 to b2cd992 Compare March 24, 2023 13:55

TimothyGu approved these changes Mar 25, 2023

View reviewed changes

anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 3, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 3, 2023

nodejs-github-bot merged commit bf41f76 into nodejs:main Apr 3, 2023

RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023

url: improve URLSearchParams creation performance

e8818c6

PR-URL: #47190 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

RafaelGSS mentioned this pull request Apr 6, 2023

v19.9.0 proposal #47441

Merged

RafaelGSS pushed a commit that referenced this pull request Apr 6, 2023

url: improve URLSearchParams creation performance

7f538dc

PR-URL: #47190 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023

url: improve URLSearchParams creation performance

9458269

PR-URL: #47190 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

RafaelGSS pushed a commit that referenced this pull request Apr 8, 2023

url: improve URLSearchParams creation performance

d243115

PR-URL: #47190 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

danielleadams pushed a commit that referenced this pull request Jul 6, 2023

url: improve URLSearchParams creation performance

cb5d1cc

PR-URL: #47190 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023

url: improve URLSearchParams creation performance

4850ba4

PR-URL: nodejs#47190 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

danielleadams mentioned this pull request Jul 10, 2023

v18.17.0 release proposal #48694

Merged

-                        if ((typeof pair !== 'object' && typeof pair !== 'function') ||
-                            pair === null ||
-                            typeof pair[SymbolIterator] !== 'function') {
+                        if (pair == null) {

url: improve URLSearchParams creation performance by anonrig · Pull Request #47190 · nodejs/node · GitHub

Uh oh!

Uh oh!

url: improve URLSearchParams creation performance - #47190

url: improve URLSearchParams creation performance#47190
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
anonrig:url-search-params-improve

anonrig commented Mar 21, 2023 •
edited
Loading

Uh oh!

Uh oh!

nodejs-github-bot commented Mar 21, 2023

Uh oh!

nodejs-github-bot commented Mar 21, 2023

Uh oh!

anonrig commented Mar 21, 2023

Uh oh!

mscdex commented Mar 21, 2023

Uh oh!

anonrig commented Mar 21, 2023

Uh oh!

nodejs-github-bot commented Mar 21, 2023

Uh oh!

anonrig commented Mar 23, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TimothyGu Mar 25, 2023

Uh oh!

anonrig commented Apr 3, 2023 •
edited
Loading

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

url: improve URLSearchParams creation performance by anonrig · Pull Request #47190 · nodejs/node · GitHub

Uh oh!

Uh oh!

url: improve URLSearchParams creation performance - #47190

url: improve URLSearchParams creation performance#47190nodejs-github-bot merged 1 commit intonodejs:mainnodejs/node:mainfrom anonrig:url-search-params-improveanonrig/node:url-search-params-improveCopy head branch name to clipboard

Conversation

anonrig commented Mar 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Footnote

Benchmark Result

Uh oh!

nodejs-github-bot commented Mar 21, 2023

Uh oh!

nodejs-github-bot commented Mar 21, 2023

Uh oh!

anonrig commented Mar 21, 2023

Uh oh!

mscdex commented Mar 21, 2023

Uh oh!

anonrig commented Mar 21, 2023

Uh oh!

nodejs-github-bot commented Mar 21, 2023

Uh oh!

anonrig commented Mar 23, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TimothyGu Mar 25, 2023

Choose a reason for hiding this comment

Uh oh!

anonrig commented Apr 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

url: improve URLSearchParams creation performance#47190
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
anonrig:url-search-params-improve

anonrig commented Mar 21, 2023 •
edited
Loading

anonrig commented Apr 3, 2023 •
edited
Loading