Closed mcollina closed 6 years ago
@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.683ms
with --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.250ms
without --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.896ms
Essentially yes, --nountrusted_code_mitigations
fixes it. What does that do?
(With the new release I can't replicate the effect on http, which is good)
--nountrusted_code_mitigations
fixes 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_mitigations
instead?
Or is this already happening on master?
Untrusted code mitigations are on by default in V8. Having them on regresses performance. --no-untrusted-code-mitigations
disables them and fixes the regression.
@mcollina you said earlier that the flag does not work for you. Did you find out why?
@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_MITIGATIONS
as 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 https://github.com/nodejs/node/pull/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-mitigations
to 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.
@hashseed knowing what you just said, should we instead compile with untrusted code mitigations and disable them by default when Node starts (with v8.setFlagsFromString
)?
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:3000
twice and get the second number: