url: improve URLSearchParams creation performance #47190

anonrig · 2023-03-21T13:46:10Z

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

nodejs-github-bot · 2023-03-21T13:46:15Z

Review requested:

@nodejs/url

nodejs-github-bot · 2023-03-21T14:41:29Z

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

anonrig · 2023-03-21T17:04:31Z

@mscdex afaik, benchmark ci is down...

mscdex · 2023-03-21T17:46:39Z

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

anonrig · 2023-03-21T18:03:46Z

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

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

nodejs-github-bot · 2023-03-21T19:22:00Z

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

anonrig · 2023-03-23T14:07:57Z

@nodejs/url please review

lib/internal/url.js

TimothyGu · 2023-03-25T03:08:41Z

lib/internal/url.js

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


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.

anonrig · 2023-04-03T02:19:34Z

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

nodejs-github-bot · 2023-04-03T15:59:48Z

Landed in bf41f76

PR-URL: #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

lib/internal/url.js Show resolved Hide resolved

watilde reviewed Mar 24, 2023

View reviewed changes

lib/internal/url.js Show resolved Hide resolved

watilde reviewed Mar 24, 2023

View reviewed changes

lib/internal/url.js Show resolved Hide resolved

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
24 checks passed

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

url: improve URLSearchParams creation performance

7d496b8

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

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>

Apr	MAY	Jun
	08
2022	2023	2024

url: improve URLSearchParams creation performance #47190

url: improve URLSearchParams creation performance #47190

anonrig commented Mar 21, 2023 •

edited

nodejs-github-bot commented Mar 21, 2023

nodejs-github-bot commented Mar 21, 2023

anonrig commented Mar 21, 2023

mscdex commented Mar 21, 2023

anonrig commented Mar 21, 2023

nodejs-github-bot commented Mar 21, 2023

anonrig commented Mar 23, 2023

TimothyGu Mar 25, 2023

anonrig commented Apr 3, 2023 •

edited

nodejs-github-bot commented Apr 3, 2023

url: improve URLSearchParams creation performance #47190

url: improve URLSearchParams creation performance #47190

Conversation

anonrig commented Mar 21, 2023 • edited

Summary

Footnote

Benchmark Result

nodejs-github-bot commented Mar 21, 2023

nodejs-github-bot commented Mar 21, 2023

anonrig commented Mar 21, 2023

mscdex commented Mar 21, 2023

anonrig commented Mar 21, 2023

nodejs-github-bot commented Mar 21, 2023

anonrig commented Mar 23, 2023

TimothyGu Mar 25, 2023

Choose a reason for hiding this comment

anonrig commented Apr 3, 2023 • edited

nodejs-github-bot commented Apr 3, 2023

anonrig commented Mar 21, 2023 •

edited

anonrig commented Apr 3, 2023 •

edited