Performance issue with URL.searchParams.append for large numbers of params #51518
Description
Version
21.6.0
Platform
Darwin [snip] 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:53:18 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T6000 arm64
Subsystem
No response
What steps will reproduce the bug?
Using URL.searchParams.append:
time node -e "const url = new URL('https://example.com'); console.log(new Date()); Array.from('a'.repeat(50000)).map(a => url.searchParams.append('test', a)); console.log(new Date()); console.log(url.toString().length); console.log(new Date());"
2024-01-19T01:31:47.867Z
2024-01-19T01:34:12.696Z
350020
2024-01-19T01:34:12.697Z
node -e 140.94s user 14.34s system 106% cpu 2:25.17 total
vs. using URLSearchParams.append:
time node -e "const params = new URLSearchParams(); console.log(new Date()); Array.from('a'.repeat(50000)).map(a => params.append('test', a)); console.log(new Date()); console.log(('https://example.com/?' + params.toString()).length); console.log(new Date());"
2024-01-19T01:32:25.111Z
2024-01-19T01:32:25.120Z
350020
2024-01-19T01:32:25.127Z
node -e 0.04s user 0.01s system 79% cpu 0.066 total
How often does it reproduce? Is there a required condition?
No response
What is the expected behavior? Why is that the expected behavior?
No response
What do you see instead?
The URL.searchParams.append snippet with 50,000 params being append takes 2-3 minutes to run on an M1, which is a ridiculous amount of time. The equivalent snippet with URLSearchParams.append takes 50-100 milliseconds to run.
Additional information
I talked through this performance issue with Yagiz on Twitter and got to what appears to be the cause of the issue, every time URL.searchParams.append is called, the whole URL essentially gets stringified to update properties on URL itself: https://twitter.com/yagiznizipli/status/1748151781534650545
I believe it to be this call that is the cause:
Lines 478 to 480 in eb4432c
With #context being set here:
Lines 1023 to 1024 in eb4432c
And the #context.search setter being here:
Lines 1012 to 1017 in eb4432c
I'm unsure whether the performance issue is with the toString call in URLSearchParams, or whether it is within the logic of the #context.search setter. I've not worked with Node.js source before, so not sure where to start on profiling that, or submitting a fix (hence opening this issue to track it for anyone to grab).
I suspect that an "easy" fix either way is to update URLSearchParams such that it doesn't write back to URL, and instead update the relevant bits of URL (href, search, etc.) to essentially call the logic that is currently in the #context.search setter on-demand as getters when needed, if URLSearchParams has been created for the URL.