perf: replace querystring with fast-querystring

[anonrig]

I wanted to see the performance difference between legacy and fast-querystring benchmarks in undici.

[anonrig]

I wonder if this is related to the cost of loading querystring module vs loading fast-querystring. I'll investigate on this more.

[anonrig]

Here's the benchmark of just importing the packages. Since node:querystring ships with node itself, the requiring is a lot faster (cached), but for others it's not... I wonder how we can improve the benchmarks to reflect real-world usage, instead of including require times.

I'll also update the fast-querystring repository with the benchmarks for this.

➜  fast-querystring git:(main) ✗ node benchmark/import.mjs
╔═════════════════════════════╤═════════╤══════════════════╤═══════════╗
║ Slower tests                │ Samples │           Result │ Tolerance ║
╟─────────────────────────────┼─────────┼──────────────────┼───────────╢
║ @aws-sdk/querystring-parser │    1000 │  10154.56 op/sec │  ± 0.43 % ║
║ querystringparser           │    1000 │  10624.80 op/sec │  ± 0.53 % ║
║ query-string                │    1000 │  11004.50 op/sec │  ± 0.47 % ║
║ qs                          │    1000 │  11077.09 op/sec │  ± 0.37 % ║
║ querystringify              │    1000 │  11147.54 op/sec │  ± 0.51 % ║
║ fast-querystring            │    1000 │  38184.61 op/sec │  ± 0.84 % ║
╟─────────────────────────────┼─────────┼──────────────────┼───────────╢
║ Fastest test                │ Samples │           Result │ Tolerance ║
╟─────────────────────────────┼─────────┼──────────────────┼───────────╢
║ node:querystring            │   10000 │ 224756.38 op/sec │  ± 1.14 % ║
╚═════════════════════════════╧═════════╧══════════════════╧═══════════╝

[codecov-commenter]

Codecov Report

Base: 90.28% // Head: 90.30% // Increases project coverage by +0.01% :tada:

Coverage data is based on head (aef2434) compared to base (93fd794). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1679      +/-   ##
==========================================
+ Coverage   90.28%   90.30%   +0.01%     
==========================================
  Files          58       58              
  Lines        5188     5188              
==========================================
+ Hits         4684     4685       +1     
+ Misses        504      503       -1     

Impacted Files	Coverage Δ
lib/core/util.js	97.93% <100.00%> (ø)
lib/fetch/file.js	90.80% <0.00%> (+1.14%)	:arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[Kikobeats]

maybe better to just use a native API? https://github.com/nodejs/undici/pull/1682

[anonrig]

maybe better to just use a native API? https://github.com/nodejs/undici/pull/1682

Native API is slower. Benchmarks are available at github.com/anonrig/fast-querystring

[silverwind]

maybe better to just use a native API? #1682

Native API is slower. Benchmarks are available at github.com/anonrig/fast-querystring

But is it compatible with the standard URLSearchParams interface? Imho, speed does not matter if it comes at the cost of compatibilty. The tests look rather bleak to me.

Thought I do recall URLSearchParams does for example deal oddly with falsy values which may often not be desirable, so maybe 100% compatibilty to URLSearchParams is not a goal.

[mcollina]

Thought I do recall URLSearchParams does for example deal oddly with falsy values which may often not be desirable, so maybe 100% compatibilty to URLSearchParams is not a goal.

Compatiblity with URLSearchParams is not a goal. Essentially URLSearchParams should be avoided as it's significantly slower than the old querystring module.

[ronag]

@mcollina @KhafraDev

[anonrig]

Benchmarks does not reflect the truth because requiring node:querystring versus requiring fast-querystring is not the same, due to snapshotting. If you compare the functionality, it is 2x faster.