Repository navigation
performance regression against master #40
Description
Activity
@nodejs/v8
I downloaded an old canary build, I'm now on node-v10.0.0-v8-canary20180306870146ff63-darwin-x64.
master (not sure why but I can't replicate the numbers I had this morning):
benchThrough2*10000: 2667.087ms benchPassThrough*10000: 2539.030ms benchThrough*10000: 598.221ms benchSyncThrough*10000: 363.399ms benchThrough2*10000: 2672.474ms benchPassThrough*10000: 2672.423ms benchThrough*10000: 611.136ms benchSyncThrough*10000: 343.683mswith
--nountrusted_code_mitigations:$ ./node --nountrusted_code_mitigations ~/Repositories/syncthrough/benchmarks/basic.js benchThrough2*10000: 2876.624ms benchPassThrough*10000: 2734.464ms benchThrough*10000: 634.742ms benchSyncThrough*10000: 384.965ms benchThrough2*10000: 2714.740ms benchPassThrough*10000: 2733.384ms benchThrough*10000: 621.585ms benchSyncThrough*10000: 351.250mswithout
--nountrusted_code_mitigations:$ ./node ~/Repositories/syncthrough/benchmarks/basic.js benchThrough2*10000: 3067.916ms benchPassThrough*10000: 2926.186ms benchThrough*10000: 896.684ms benchSyncThrough*10000: 466.958ms benchThrough2*10000: 2969.392ms benchPassThrough*10000: 2955.923ms benchThrough*10000: 853.613ms benchSyncThrough*10000: 434.896msEssentially yes,
--nountrusted_code_mitigationsfixes it. What does that do?(With the new release I can't replicate the effect on http, which is good)
--nountrusted_code_mitigationsfixes it. What does that do?Spectre and Meltdown related mitigations, not needed for usual use of node.js AFAIK.
oh nice. Can we flip the switch to have
--untrusted_code_mitigationsinstead?
Or is this already happening on master?@mcollina you said earlier that the flag does not work for you. Did you find out why?
@hashseed I used an older binary, I wrongly guessed that the files were sorted, but they weren't.
Untrusted code mitigations are on by default in V8. Having them on regresses performance. --no-untrusted-code-mitigations disables them and fixes the regression.
What I mean is, can we flip this behavior in Node.js against V8? It seems they are not part of Node.js threat model, so we can live without them and enjoy better performance.
Yes. The best way to do this is to add GYP configs to pass
DISABLE_UNTRUSTED_CODE_MITIGATIONSas C++ define, to mirror this GN config.after we set DISABLE_UNTRUSTED_CODE_MITIGATIONS, can users still do something like
--enable-untrusted-code-mitigations@hashseed Would this be needed on V8 6.6? Should we do this change after nodejs/node#19201?
after we set DISABLE_UNTRUSTED_CODE_MITIGATIONS, can users still do something like --enable-untrusted-code-mitigations
Yes and no. It sets the default value for the flag, so you can still use
--untrusted-code-mitigationsto enable it. However, V8 compiles some of its builtin code at build time to include in the startup snapshot. Once included, the runtime flag won't be able to change it anymore. These builtin code are also affected by mitigations. Though I expect that the performance impact to be less significant. But the fact these builtins do not include mitigations may be a security risk if that's an issue.@hashseed Would this be needed on V8 6.6? Should we do this change after nodejs/node#19201?
Ideally as part of the PR to update V8 to 6.6, yes.
Yes, that sounds good.
You'll probably need this on 6.5 as well. I'm not so eager to have untrusted code mitigations build into the Node snapshot. I think it's better to consistently disable them via the GN/GYP flag. If someone needs a Node with these mitigations, he/she should build a proper binary. We're already stretching the test matrix quite a bit here...
This can be closed then
I got a performance regression from master to canary.
https://github.com/mcollina/syncthrough/blob/master/benchmarks/basic.js
In master:
On canary:
On node 9.7.0:
On node 8.10.0:
I can see a similar regression on http as well, if we run:
And then we load test with
npx autocannon -c 100 -d 5 -p 10 localhost:3000twice and get the second number: