build: speed up compilation of some V8 files#52083
build: speed up compilation of some V8 files#52083targos wants to merge 3 commits intonodejs:mainfrom
Conversation
This introduces a special target to compile some of the 'v8_initializers' files with "-O1" instead of "-O3" to avoid huge compilation times with GCC versions <13. Closes: nodejs#52068
|
Review requested:
|
nodejs-github-bot
added
needs-ci
targos
added
the
commit-queue-squash
github-actions
bot
removed
the
request-ci
|
@targos You went with |
|
@lemire Yes. I tested |
|
@targos In the diff, I don't see GCC version detection... it says that the issue is fixed with GCC 13. Is there the equivalent of "if GCC >= 13 do this, else do that", or is that something that must be done manually later? |
|
@lemire I don't know, and I'm not interested in looking into it. |
Here is my proposal: we merge this PR but we add an issue where we saw that this workaround should be revisited once GCC >= 13 can be safely assumed. My concern here is that we might be trading off performance, but in a way that is non-obvious. |
targos
added
the
author ready
|
If -O2 doesn't make it faster, does -Os make a difference? It would be useful to measure if this affects wasm <-> JS interop performance. For example, a benchmark involving |
|
Actually I think there already is a benchmark for wasm/js interop via the cjs exports detction. Benchmark CI https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1496/ |
|
|
For posterity: Vs. a 200x speedup in compile times? Done, |
|
How much difference does -Os make in the binary size? I am not able to find where the object files are locally, the two generated .cc files appear to be around 400KB. While that’s not too big as far as generated file goes, not sure if the problematic path in GCC can lead to any surprising results here. I’d say if the binary size differences is >5m, then -Os would be better. (2m doesn’t seem that bad for V8). Otherwise -O1 is fine. |
lemire
left a comment
lemire
left a comment
There was a problem hiding this comment.
I think that this is fine as-is.
|
With With |
This introduces a special target to compile some of the 'v8_initializers' files with "-O1" instead of "-O3" to avoid huge compilation times with GCC versions <13. PR-URL: #52083 Fixes: #52068 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me>
|
Landed in 4e278f0 |
This introduces a special target to compile some of the 'v8_initializers' files with "-O1" instead of "-O3" to avoid huge compilation times with GCC versions <13. PR-URL: nodejs#52083 Fixes: nodejs#52068 Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me>
marco-ippolito
added
the
dont-land-on-v20.x
9 participants
This introduces a special target to compile some of the
'v8_initializers' files with "-O1" instead of "-O3" to avoid huge
compilation times with GCC versions <13.
Closes: #52068