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

Replace parseInt with Number at get 6% boost #4289

Merged
merged 1 commit into from Sep 19, 2022

Conversation

anonrig
Copy link
Contributor

@anonrig anonrig commented Sep 19, 2022

Here is a small benchmark for Number(value) vs parseInt(value)

Result of micro-benchmark

Number: 67.073ms
parseInt: 206.781ms

Code

var { time, timeEnd } = console
var I = 1e8;
var y;

time("Number")
for (var i = 0; i < I; i++) y = Number(i)
timeEnd("Number")

time("parseInt")
for (var i = 0; i < I; i++) y = parseInt(i)
timeEnd("parseInt")

Before this pull-request:

➜  fastify git:(perf/reply) npx concurrently -k -s first "node ./examples/simple.js" "npx autocannon -c 100 -d 10 -p 10 localhost:3000/"
[1] Running 10s test @ http://localhost:3000/
[1] 100 connections with 10 pipelining factor
[1]
[1]
[1] ┌─────────┬──────┬──────┬───────┬───────┬─────────┬─────────┬────────┐
[1] │ Stat    │ 2.5% │ 50%  │ 97.5% │ 99%   │ Avg     │ Stdev   │ Max    │
[1] ├─────────┼──────┼──────┼───────┼───────┼─────────┼─────────┼────────┤
[1] │ Latency │ 4 ms │ 8 ms │ 12 ms │ 13 ms │ 7.54 ms │ 3.44 ms │ 136 ms │
[1] └─────────┴──────┴──────┴───────┴───────┴─────────┴─────────┴────────┘
[1] ┌───────────┬─────────┬─────────┬─────────┬─────────┬───────────┬─────────┬─────────┐
[1] │ Stat      │ 1%      │ 2.5%    │ 50%     │ 97.5%   │ Avg       │ Stdev   │ Min     │
[1] ├───────────┼─────────┼─────────┼─────────┼─────────┼───────────┼─────────┼─────────┤
[1] │ Req/Sec   │ 113663  │ 113663  │ 126911  │ 131199  │ 124666.19 │ 5335.45 │ 113662  │
[1] ├───────────┼─────────┼─────────┼─────────┼─────────┼───────────┼─────────┼─────────┤
[1] │ Bytes/Sec │ 21.4 MB │ 21.4 MB │ 23.9 MB │ 24.7 MB │ 23.4 MB   │ 999 kB  │ 21.4 MB │
[1] └───────────┴─────────┴─────────┴─────────┴─────────┴───────────┴─────────┴─────────┘
[1]
[1] Req/Bytes counts sampled once per second.
[1] # of samples: 11
[1]
[1] 1372k requests in 11.02s, 258 MB read
[1] npx autocannon -c 100 -d 10 -p 10 localhost:3000/ exited with code 0
--> Sending SIGTERM to other processes..
[0] node ./examples/simple.js exited with code SIGTERM

After this pull request

➜  fastify git:(perf/reply) ✗ npx concurrently -k -s first "node ./examples/simple.js" "npx autocannon -c 100 -d 10 -p 10 localhost:3000/"
[1] Running 10s test @ http://localhost:3000/
[1] 100 connections with 10 pipelining factor
[1]
[1]
[1] ┌─────────┬──────┬──────┬───────┬───────┬─────────┬─────────┬────────┐
[1] │ Stat    │ 2.5% │ 50%  │ 97.5% │ 99%   │ Avg     │ Stdev   │ Max    │
[1] ├─────────┼──────┼──────┼───────┼───────┼─────────┼─────────┼────────┤
[1] │ Latency │ 4 ms │ 8 ms │ 11 ms │ 12 ms │ 7.08 ms │ 3.22 ms │ 138 ms │
[1] └─────────┴──────┴──────┴───────┴───────┴─────────┴─────────┴────────┘
[1] ┌───────────┬─────────┬─────────┬─────────┬─────────┬───────────┬─────────┬─────────┐
[1] │ Stat      │ 1%      │ 2.5%    │ 50%     │ 97.5%   │ Avg       │ Stdev   │ Min     │
[1] ├───────────┼─────────┼─────────┼─────────┼─────────┼───────────┼─────────┼─────────┤
[1] │ Req/Sec   │ 123199  │ 123199  │ 134271  │ 136703  │ 132247.28 │ 4149.77 │ 123168  │
[1] ├───────────┼─────────┼─────────┼─────────┼─────────┼───────────┼─────────┼─────────┤
[1] │ Bytes/Sec │ 23.2 MB │ 23.2 MB │ 25.2 MB │ 25.7 MB │ 24.9 MB   │ 781 kB  │ 23.2 MB │
[1] └───────────┴─────────┴─────────┴─────────┴─────────┴───────────┴─────────┴─────────┘
[1]
[1] Req/Bytes counts sampled once per second.
[1] # of samples: 11
[1]
[1] 1456k requests in 11.01s, 273 MB read
[1] npx autocannon -c 100 -d 10 -p 10 localhost:3000/ exited with code 0
--> Sending SIGTERM to other processes..
[0] node ./examples/simple.js exited with code SIGTERM

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit 9d9d62c into fastify:main Sep 19, 2022
@anonrig anonrig deleted the perf/reply branch September 19, 2022 21:50
@Fdawgs
Copy link
Member

Fdawgs commented Sep 20, 2022

What version of Node was this tested on?

@anonrig
Copy link
Contributor Author

anonrig commented Sep 20, 2022

What version of Node was this tested on?

Latest version of Node 18

@jsumners
Copy link
Member

We should have included tests for the inputs we expect. Number does not parse the input, it converts the input.

@anonrig
Copy link
Contributor Author

anonrig commented Sep 20, 2022

I'll open a new pull request with the tests. Thanks for the input @jsumners

@Fdawgs
Copy link
Member

Fdawgs commented Sep 21, 2022

I'll open a new pull request with the tests.

Will need to test floats:

Number('1.5') // 1.5
parseInt('1.5', 10) // 1

@JoCat
Copy link

JoCat commented Oct 17, 2022

Offtop
I checked one thing just for fun

var { time, timeEnd } = console
var I = 1e8
var y

time("+num")
for (var i = 0; i < I; i++) y = +i
timeEnd("+num")

time("Number")
for (var i = 0; i < I; i++) y = Number(i)
timeEnd("Number")

time("parseInt")
for (var i = 0; i < I; i++) y = parseInt(i)
timeEnd("parseInt")

and got the following result

+num: 41.525ms
Number: 56.06ms
parseInt: 226.17ms

I use node.js v16.14.0

@JoCat
Copy link

JoCat commented Oct 17, 2022

For a more accurate comparison, I converted the numbers to strings before processing and got the following result:

var { time, timeEnd } = console
var I = 1e8
var y

time("+num")
for (let i = 0; i < I; i++) y = +`${i}`
timeEnd("+num")

time("Number")
for (let i = 0; i < I; i++) y = Number(`${i}`)
timeEnd("Number")

time("parseInt")
for (let i = 0; i < I; i++) y = parseInt(`${i}`, 10)
timeEnd("parseInt")
+num: 3.262s
Number: 3.200s
parseInt: 12.480s

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants