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

large performance regressions since f51c152 #47328

Closed
mscdex opened this issue Mar 31, 2023 · 2 comments · Fixed by #47340 or #47342
Closed

large performance regressions since f51c152 #47328

mscdex opened this issue Mar 31, 2023 · 2 comments · Fixed by #47340 or #47342
Assignees
Labels
performance Issues and PRs related to the performance of Node.js. regression Issues related to regressions. whatwg-url Issues and PRs related to the WHATWG URL implementation.

Comments

@mscdex
Copy link
Contributor

mscdex commented Mar 31, 2023

Since the related PR was landed before the CI benchmark machine was restarted, these regressions were not caught before it was merged.

I'm not sure if there are other built-in modules affected, but the URL module is one such user of toUSVString().

https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1306/consoleFull

url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='altspaces'                       ***    -17.43 %       ±1.75%  ±2.33%  ±3.03%
url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='encodefake'                      ***    -25.28 %       ±2.79%  ±3.72%  ±4.86%
url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='encodelast'                      ***    -20.54 %       ±2.20%  ±2.93%  ±3.82%
url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='encodemany'                      ***    -16.30 %       ±2.05%  ±2.74%  ±3.57%
url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='manyblankpairs'                  ***    -38.49 %       ±4.55%  ±6.11%  ±8.09%
url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='manypairs'                       ***    -16.03 %       ±2.22%  ±2.96%  ±3.86%
url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='multicharsep'                    ***    -27.57 %       ±2.15%  ±2.87%  ±3.76%
url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='multivalue'                      ***    -23.27 %       ±1.78%  ±2.37%  ±3.08%
url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='multivaluemany'                  ***    -16.69 %       ±1.17%  ±1.56%  ±2.05%
url/legacy-vs-whatwg-url-searchparams-parse.js n=1000000 method='whatwg' searchParam='noencode'                        ***    -26.78 %       ±2.35%  ±3.13%  ±4.09%
url/url-searchparams-creation.js n=1000000 inputType='iterable' type='array'                                           ***    -25.56 %       ±0.71%  ±0.95%  ±1.24%
url/url-searchparams-creation.js n=1000000 inputType='iterable' type='encodelast'                                      ***    -40.80 %       ±1.59%  ±2.14%  ±2.81%
url/url-searchparams-creation.js n=1000000 inputType='iterable' type='encodemany'                                      ***    -41.81 %       ±2.19%  ±2.91%  ±3.79%
url/url-searchparams-creation.js n=1000000 inputType='iterable' type='multiprimitives'                                 ***    -42.60 %       ±1.83%  ±2.46%  ±3.23%
url/url-searchparams-creation.js n=1000000 inputType='iterable' type='noencode'                                        ***    -39.99 %       ±1.57%  ±2.11%  ±2.77%
url/url-searchparams-creation.js n=1000000 inputType='object' type='array'                                             ***    -29.03 %       ±1.94%  ±2.58%  ±3.36%
url/url-searchparams-creation.js n=1000000 inputType='object' type='encodelast'                                        ***    -34.75 %       ±1.69%  ±2.27%  ±2.99%
url/url-searchparams-creation.js n=1000000 inputType='object' type='encodemany'                                        ***    -35.23 %       ±1.30%  ±1.74%  ±2.28%
url/url-searchparams-creation.js n=1000000 inputType='object' type='multiprimitives'                                   ***    -35.42 %       ±2.37%  ±3.17%  ±4.17%
url/url-searchparams-creation.js n=1000000 inputType='object' type='noencode'                                          ***    -35.35 %       ±1.82%  ±2.43%  ±3.19%
url/url-searchparams-creation.js n=1000000 inputType='string' type='array'                                             ***    -23.30 %       ±2.24%  ±2.98%  ±3.88%
url/url-searchparams-creation.js n=1000000 inputType='string' type='encodelast'                                        ***    -26.13 %       ±2.25%  ±3.02%  ±3.98%
url/url-searchparams-creation.js n=1000000 inputType='string' type='encodemany'                                        ***    -23.93 %       ±1.70%  ±2.27%  ±2.95%
url/url-searchparams-creation.js n=1000000 inputType='string' type='multiprimitives'                                   ***    -24.05 %       ±1.58%  ±2.10%  ±2.74%
url/url-searchparams-creation.js n=1000000 inputType='string' type='noencode'                                          ***    -23.97 %       ±1.77%  ±2.35%  ±3.06%
url/url-searchparams-read.js n=20000000 param='nonexistent' accessMethod='get'                                         ***    -60.10 %       ±1.35%  ±1.80%  ±2.34%
url/url-searchparams-read.js n=20000000 param='nonexistent' accessMethod='getAll'                                      ***    -57.29 %       ±2.38%  ±3.20%  ±4.22%
url/url-searchparams-read.js n=20000000 param='nonexistent' accessMethod='has'                                         ***    -58.05 %       ±2.19%  ±2.94%  ±3.87%
url/url-searchparams-read.js n=20000000 param='one' accessMethod='get'                                                 ***    -60.58 %       ±1.58%  ±2.11%  ±2.75%
url/url-searchparams-read.js n=20000000 param='one' accessMethod='getAll'                                              ***    -39.72 %       ±2.91%  ±3.87%  ±5.04%
url/url-searchparams-read.js n=20000000 param='one' accessMethod='has'                                                 ***    -60.04 %       ±2.66%  ±3.57%  ±4.71%
url/url-searchparams-read.js n=20000000 param='three' accessMethod='get'                                               ***    -57.71 %       ±1.36%  ±1.81%  ±2.36%
url/url-searchparams-read.js n=20000000 param='three' accessMethod='getAll'                                            ***    -37.37 %       ±2.63%  ±3.51%  ±4.60%
url/url-searchparams-read.js n=20000000 param='three' accessMethod='has'                                               ***    -58.09 %       ±2.38%  ±3.19%  ±4.20%
url/url-searchparams-read.js n=20000000 param='two' accessMethod='get'                                                 ***    -58.71 %       ±1.70%  ±2.27%  ±2.95%
url/url-searchparams-read.js n=20000000 param='two' accessMethod='getAll'                                              ***    -37.93 %       ±2.61%  ±3.47%  ±4.52%
url/url-searchparams-read.js n=20000000 param='two' accessMethod='has'                                                 ***    -57.99 %       ±2.55%  ±3.40%  ±4.44%
url/usvstring.js n=50000000 input='nonstring'                                                                          ***    -45.54 %       ±3.05%  ±4.08%  ±5.35%
url/usvstring.js n=50000000 input='valid'                                                                              ***    -73.28 %       ±2.34%  ±3.14%  ±4.15%
url/usvstring.js n=50000000 input='validsurr'                                                                          ***    -14.33 %       ±2.56%  ±3.42%  ±4.48%

/cc @anonrig

@mscdex mscdex added performance Issues and PRs related to the performance of Node.js. whatwg-url Issues and PRs related to the WHATWG URL implementation. regression Issues related to regressions. labels Mar 31, 2023
@mscdex mscdex changed the title large performance regressions since f51c152 large performance regressions since [f51c152](https://github.com/nodejs/node/commit/f51c152) Mar 31, 2023
@mscdex mscdex changed the title large performance regressions since [f51c152](https://github.com/nodejs/node/commit/f51c152) large performance regressions since f51c152 Mar 31, 2023
@anonrig anonrig self-assigned this Mar 31, 2023
@anonrig
Copy link
Member

anonrig commented Mar 31, 2023

I'll revert the changes but this function will be obsolete when v8 11.3 is merged, since V8 has toWellFormed already implemented in JS.

@targos
Copy link
Member

targos commented Mar 31, 2023

I marked the PR as "dont-land", since the V8 update won't be backported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Issues and PRs related to the performance of Node.js. regression Issues related to regressions. whatwg-url Issues and PRs related to the WHATWG URL implementation.
Projects
None yet
3 participants