-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
The performance gap between node16 and node21 changes as the n of the benchmark changes #50571
Comments
Probably is just the V8 JIT kick-in and the start of the optimization in that code, maybe the behavior had changed and now they didn't optimize aggressively initially and only started the optimization after more iterations than before. But this can have a lot of reasons, all of them are related to V8 than Node.js itself. But you found an interesting case that we can increase the number of iterations to be more precise, are you willing to create a PR to increase that number? Just enough that still be fast but avoid this behavior. |
Increase the number of iterations from `1e3` to `1e6` to avoid the test performance gap caused by inactive V8 optimization caused by too few iterations. Fixes: nodejs#50571
Thanks. I have submitted a PR #50585 to increase the number of iterations in I plan to test other benchmarks later to see if similar situations exist. If so, I will modify all and create a unified PR. I wonder if this is acceptable? |
@Septa2112 It would be highly appreciated if you did that! I suggest you create a commit for each benchmark change. |
BTW, I tried to analyze the cause of this phenomenon based on your tips. And I run the benchmark with I found that when iterations is default When n increases to 1e5 or larger, I can get the opt code of the two versions of node. Just like:
Does this mean that the performance gap between Please correct me if I say something wrong. |
@Septa2112 You are right, at least on I used the following code: let string;
const n = 1e3;
const str = 'abc';
const num = 123;
const start = performance.now();
for (let i = 0; i < n; i++)
string = ['...', str, ', ', num, ', ', str, ', ', num, '.'].join('');
const end = performance.now();
console.log(`Diff: ${end - start}ms`);
console.log(string); For
For `1e6:
Using
Is slower without optimizations but is faster faster the optimizations. |
@H4ad Thanks for your explanation! Now I find that some other cases may have similar problems. Should I open a new issue? Or update these cases directly in the current issue and create new PRs directly? |
Create new PRs and just reference this issue as the starting point to looking for those fixes. |
Increase the number of iterations from `1e3` to `1e6` to avoid the test performance gap caused by inactive V8 optimization caused by too few iterations. Fixes: #50571 PR-URL: #50585 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Increase the number of iterations from `1e3` to `1e6` to avoid the test performance gap caused by inactive V8 optimization caused by too few iterations. Fixes: #50571 PR-URL: #50585 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Increase the number of iterations from 1e5 to 5e6 to avoid the test performance gap caused by inactive V8 optimization caused by insufficient number of iterations Refs: nodejs#50571
Increase the number of iterations from `1e3` to `1e6` to avoid the test performance gap caused by inactive V8 optimization caused by too few iterations. Fixes: #50571 PR-URL: #50585 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
value 2500 to make the score accurate. Fixes: nodejs#50571
Increase the iteration number from 500 to 2500 to make time consumption of crypto operation dominate the benchmark running. Fixes: nodejs#50571
Increase the iteration number from 5000 to 500000 to make time consumption of crypto operation dominate the benchmark running. Fixes: nodejs#50571
Increase the 500 iteration to 50000 to reflect the real ciphers perf. The single iteration n=1 can not make the score reasonable. Fixes nodejs#50571
Increase the iteration number from 1000 to 10000 to get a reasonble score. The updated score has up to ~60% improvement. Fixes: nodejs#50571
Increase iteration value from 1000 to 10000 to reflect real performance of randomBytes generation. The randomInt has no need to apply this. Fixes:nodejs#50571
storage-getstore-nested-resources.js Fixes: nodejs#50571
Fixed: nodejs#50571 PR-URL: nodejs#51416 Fixes: nodejs#50571 Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Fixes: nodejs#50571 PR-URL: nodejs#51419 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Fixes: nodejs#50571 PR-URL: nodejs#51408 Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
storage-getstore-nested-resources.js Fixes: nodejs#50571 PR-URL: nodejs#51420 Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Version
node 16.x and node 21.1
Platform
Linux wpe-icx 6.2.0-26-generic #26~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Thu Jul 13 16:27:29 UTC 2 x86_64 x86_64 x86_64 GNU/Linux
Subsystem
No response
What steps will reproduce the bug?
I tested node-benchmark
es/string-concatenations.js
withnode 21.1
andnode 16.x
.When
n
is the default1e3
, the score difference between the two version reaches50%
.But when
n
is set to5000
or10000
or other larger numbers, there is no performance gap between the two versions.How often does it reproduce? Is there a required condition?
No response
What is the expected behavior? Why is that the expected behavior?
Expected
Reason
n
has not changed much, and the performance gap betweennode 16.x
andnode 21.1
should not change so much too.What do you see instead?
Additional information
I'm not sure whether this problem can be counted as an issue. Or is this a pre-existing problem?
The text was updated successfully, but these errors were encountered: