build: add configure option --v8-enable-short-builtin-calls#42109
Conversation
|
Review requested:
|
|
Is this a breaking change for native addons? |
No, this is not a breaking change. |
|
For the failed check "First commit message adheres to guidelines at https://goo.gl/p2fr5Q /...", I have modified the title format of this commit, but don't know how to rerun this failed check. |
|
Should we be able to measure the performance improvement with our micro benchmarks? |
We have tested the performance with some internal node.js application benchmarks, and short-builtin-calls(together with sparkplug) can bring +6% performance increase. |
|
@nodejs/build @nodejs/v8 |
|
Benchmark run on the Results |
|
I have modified the commit message to fix the lint error and now all checks have passed. |
|
For CI tests, this commit passed all of them about 1 weeks ago. |
This comment was marked as outdated.
This comment was marked as outdated.
|
Is it possible that the consistent failures on fedora-latest-x64 in CI are due to the change in this PR? I'm not seeing it anywhere else, and it's consistent here.... |
Thx for the info. I was using retry button. Looks I click the wrong button.
Old build works fine, not sure why. https://ci.nodejs.org/job/node-test-pull-request/43380/ Looks like something wrong with Jenkins cc @richardlau @nodejs/build |
I rebooted test-digitalocean-fedora34-x64-1 and the CI now passes on it 🤷. |
|
Landed in aa52873 |
|
@qdaoming-intel Thx for the contribution. (sorry the commit-queue mess and take so long to merge) |
|
@gengjiawen Thank you and all other people's support. |
Add configure option --v8-enable-short-builtin-calls and enable it by default on x86_64 platform. PR-URL: nodejs#42109 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
|
This commit breaks build/compiling C++ in v16.x, is says something like: |
Please don't merge this commit to v16.x, because this commit depends on some new feature in V8, and v16.x doesn't have such feature. |
Add configure option --v8-enable-short-builtin-calls
and enable it by default on x86_64 platform.