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

perf: 15 % better perf for number to string, avoid intermediate Math.abs, use implicit number to string coercion #28

Merged
merged 4 commits into from
Nov 2, 2022

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Nov 1, 2022

The reason why we do Math.abs is because 1e21.toString() returns "1e+21". + gets encoded but - not. So there is no good reason to do a Math.abs call. We only need to know if the number is bigger or equal than 1e21.

We could probably optimize also the >= 1e21 case because we only need to replace the +. But I guess that case is actually neglectable, because we are already bigger than Number.MAX_SAFE_INTEGER.

My benchmarks showed that doing

const tmp = value.toString(); 
const pos = tmp.indexOf('+'); 
return value.slice(0, pos) + '%2B' + value.slice(pos + 1);

could be faster than calling encodeString.

@codecov-commenter
Copy link

Codecov Report

Merging #28 (b3f12c6) into main (d46a36a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #28   +/-   ##
=======================================
  Coverage   98.70%   98.71%           
=======================================
  Files           4        4           
  Lines         310      311    +1     
  Branches       74       74           
=======================================
+ Hits          306      307    +1     
  Misses          4        4           
Impacted Files Coverage Δ
lib/stringify.js 100.00% <100.00%> (ø)

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

@anonrig
Copy link
Owner

anonrig commented Nov 1, 2022

Thanks! Nice catch. Can you provide the benchmark result for this @Uzlopak?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Nov 1, 2022

See #25
First add the fine grained benchmarks. Maybe also add more more cases. The we can bench more.

E.g. i think

"" + value 

Is faster than

 value.toString()

@anonrig
Copy link
Owner

anonrig commented Nov 1, 2022

@Uzlopak Can you update the branch and run a benchmark?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Nov 2, 2022

uzlopak@Battlestation:~/workspace/faster-querystring$ node benchmark/bench-cmp-branch.js 
Select the branch you want to compare (feature branch):
noAbs
Select the branch you want to compare with (main branch):
main
Checking out "noAbs"
Execute "npm run benchmark"

> fast-querystring@1.0.0 benchmark
> node benchmark/bench.js

encodeString: ""                                              x 303,114,918 ops/sec ±0.31% (192 runs sampled)
encodeString: "123"                                           x 118,990,503 ops/sec ±0.32% (191 runs sampled)
encodeString: "ä"                                             x 78,047,703 ops/sec ±0.17% (194 runs sampled)
encodeString: "𝌆"                                            x 33,737,820 ops/sec ±0.27% (192 runs sampled)
stringify: undefined                                       x 1,276,289,112 ops/sec ±0.08% (195 runs sampled)
stringify: null                                            x 1,276,734,422 ops/sec ±0.07% (195 runs sampled)
stringify: {}                                              x 142,636,134 ops/sec ±0.32% (192 runs sampled)
stringify: { id: true }                                    x 37,971,310 ops/sec ±0.45% (192 runs sampled)
stringify: { id: false }                                   x 37,561,548 ops/sec ±0.56% (195 runs sampled)
stringify: { id: 123 }                                     x 36,055,107 ops/sec ±0.47% (188 runs sampled)
stringify: { id: 1e+22 }                                   x 15,156,993 ops/sec ±0.43% (192 runs sampled)
stringify: { id: 123n }                                    x 20,941,835 ops/sec ±0.59% (192 runs sampled)
stringify: { id: Infinity }                                x 46,703,761 ops/sec ±0.54% (190 runs sampled)
stringify: { id: ["1", "3"] }                              x 17,871,067 ops/sec ±0.15% (194 runs sampled)
stringify: { id: "" }                                      x 38,830,704 ops/sec ±0.74% (187 runs sampled)
stringify: { id: "123" }                                   x 28,028,210 ops/sec ±0.44% (194 runs sampled)
stringify: { id: "ä" }                                     x 24,479,507 ops/sec ±0.43% (193 runs sampled)
stringify: { id: "𝌆" }                                    x 17,759,703 ops/sec ±0.64% (192 runs sampled)
stringify: { foo: ["1", "3"], bar: "2" }                   x 9,461,939 ops/sec ±0.17% (194 runs sampled)
parse:                                                 x 159,532,355 ops/sec ±0.25% (193 runs sampled)
parse: id=123                                          x 15,494,811 ops/sec ±0.29% (194 runs sampled)
parse: id=123&id=123                                   x 8,068,793 ops/sec ±0.43% (189 runs sampled)
parse: full%20name=Yagiz                               x 5,748,711 ops/sec ±0.18% (193 runs sampled)
parse: invalid%key=hello                               x 7,026,583 ops/sec ±0.23% (191 runs sampled)
parse: my+weird+field=q1%212%22%27w%245%267%2Fz8%29%3F x 1,368,378 ops/sec ±0.14% (193 runs sampled)

Checking out "main"
Execute "npm run benchmark"

> fast-querystring@1.0.0 benchmark
> node benchmark/bench.js

encodeString: ""                                              x 307,372,242 ops/sec ±0.39% (194 runs sampled)
encodeString: "123"                                           x 123,155,984 ops/sec ±0.29% (194 runs sampled)
encodeString: "ä"                                             x 80,387,483 ops/sec ±0.15% (194 runs sampled)
encodeString: "𝌆"                                            x 34,384,200 ops/sec ±0.14% (194 runs sampled)
stringify: undefined                                       x 1,277,945,259 ops/sec ±0.08% (195 runs sampled)
stringify: null                                            x 1,280,955,250 ops/sec ±0.07% (196 runs sampled)
stringify: {}                                              x 142,633,250 ops/sec ±0.29% (193 runs sampled)
stringify: { id: true }                                    x 36,402,126 ops/sec ±0.60% (187 runs sampled)
stringify: { id: false }                                   x 37,177,210 ops/sec ±0.59% (192 runs sampled)
stringify: { id: 123 }                                     x 30,838,305 ops/sec ±0.66% (190 runs sampled)
stringify: { id: 1e+22 }                                   x 13,042,202 ops/sec ±0.54% (192 runs sampled)
stringify: { id: 123n }                                    x 20,320,592 ops/sec ±1.15% (185 runs sampled)
stringify: { id: Infinity }                                x 44,892,389 ops/sec ±0.39% (188 runs sampled)
stringify: { id: ["1", "3"] }                              x 17,355,645 ops/sec ±0.42% (194 runs sampled)
stringify: { id: "" }                                      x 40,608,878 ops/sec ±0.56% (189 runs sampled)
stringify: { id: "123" }                                   x 30,560,278 ops/sec ±0.56% (195 runs sampled)
stringify: { id: "ä" }                                     x 23,331,829 ops/sec ±0.56% (191 runs sampled)
stringify: { id: "𝌆" }                                    x 18,610,251 ops/sec ±0.18% (194 runs sampled)
stringify: { foo: ["1", "3"], bar: "2" }                   x 9,089,680 ops/sec ±0.22% (191 runs sampled)
parse:                                                 x 161,492,686 ops/sec ±0.12% (195 runs sampled)
parse: id=123                                          x 16,699,358 ops/sec ±0.43% (192 runs sampled)
parse: id=123&id=123                                   x 7,836,626 ops/sec ±0.40% (189 runs sampled)
parse: full%20name=Yagiz                               x 5,990,112 ops/sec ±0.20% (192 runs sampled)
parse: invalid%key=hello                               x 6,842,480 ops/sec ±0.51% (189 runs sampled)
parse: my+weird+field=q1%212%22%27w%245%267%2Fz8%29%3F x 1,369,525 ops/sec ±0.18% (194 runs sampled)

encodeString: ""                                             .-1.39%
encodeString: "123"                                          .-3.38%
encodeString: "ä"                                            .-2.91%
encodeString: "𝌆"                                           .-1.88%
stringify: undefined                                      .-0.13%
stringify: null                                           .-0.33%
stringify: {}                                             .....0%
stringify: { id: true }                                   .+4.31%
stringify: { id: false }                                  .+1.03%
stringify: { id: 123 }                                    +16.92%
stringify: { id: 1e+22 }                                  +16.21%
stringify: { id: 123n }                                   .+3.06%
stringify: { id: Infinity }                               .+4.03%
stringify: { id: ["1", "3"] }                             .+2.97%
stringify: { id: "" }                                     .-4.38%
stringify: { id: "123" }                                  .-8.29%
stringify: { id: "ä" }                                    .+4.92%
stringify: { id: "𝌆" }                                   .-4.57%
stringify: { foo: ["1", "3"], bar: "2" }                  ..+4.1%
parse:                                                .-1.21%
parse: id=123                                         .-7.21%
parse: id=123&id=123                                  .+2.96%
parse: full%20name=Yagiz                              .-4.03%
parse: invalid%key=hello                              .+2.69%
parse: my+weird+field=q1%212%22%27w%245%267%2Fz8%29%3F.-0.08%
Back to noAbs 2b34ec3

@Uzlopak Uzlopak changed the title avoid intermediate Math.abs avoid intermediate Math.abs, use implicit number to string coercion Nov 2, 2022
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Nov 2, 2022

So it brings about 15 % more perf for number stringification

@Uzlopak Uzlopak changed the title avoid intermediate Math.abs, use implicit number to string coercion perf: 15 % better perf for number to string, avoid intermediate Math.abs, use implicit number to string coercion Nov 2, 2022
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Nov 2, 2022

I dont get why the test coverage is supposedly lower. Probably because it is now one line less?! Anyway, i created #30 to counter the coverage issue

@anonrig anonrig merged commit 79037c7 into anonrig:main Nov 2, 2022
Uzlopak added a commit to Uzlopak/fast-querystring that referenced this pull request Jan 2, 2023
…abs, use implicit number to string coercion (anonrig#28)

* avoid intermediate Math.abs

* format code

* use implicit conversion of numbers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants