Skip to content

build: enable V8 builtins PGO#50416

Merged
jkleinsc merged 2 commits into
mainfrom
sattard/enable-v8-builtins-pgo
Mar 23, 2026
Merged

build: enable V8 builtins PGO#50416
jkleinsc merged 2 commits into
mainfrom
sattard/enable-v8-builtins-pgo

Conversation

@MarshallOfSound
Copy link
Copy Markdown
Member

@MarshallOfSound MarshallOfSound commented Mar 22, 2026

Description of Change

Re-enables V8 builtins PGO, which was disabled in #38252 due to profile references breaking the shipped mksnapshot_args. The CI-side mitigation for that has existed since the GHA migration (sed stripping in build-electron/action.yml), but the build-side disable was never reverted.

What this does:

  • Removes v8_builtins_profiling_log_file = "" from all.gn so V8's default auto-selection kicks in (picks tools/builtins-pgo/profiles/x64.profile when chrome_pgo_phase == 2, i.e. release builds only)
  • Adds a small V8 patch changing --abort-on-bad-builtin-profile-data to --warn-about-builtin-profile-data in mksnapshot invocation
  • Extends CI's mksnapshot_args stripping to also remove the orphaned --reorder-builtins / --warn-about-builtin-profile-data flags (which crash/no-op without profile input)

Why the warn flag is safe:

Electron sets v8_enable_javascript_promise_hooks = true, which adds #ifdef branches in builtins-microtask-queue-gen.cc and promise-misc.tq. This changes the control-flow graph hash of 11 Promise/async builtins, so they mismatch V8's pre-generated profile (built with Chrome defaults). With --abort, this fails the build; with --warn, per pipeline.cc:ValidateProfileData, the builtin's profile data is set to nullptr and it compiles without PGO — identical to today's behavior for that builtin. No correctness risk.

The 11 skipped builtins: RunMicrotasks, AsyncFunctionEnter/Await, AsyncGeneratorYieldWithAwait/PrototypeNext/Await, FulfillPromise, NewPromiseCapability, PromiseConstructor/Resolve/PrototypeThen. The remaining ~490 builtins (Array, String, RegExp, Map/Set, TypedArray, iterators, etc.) all receive PGO.

Validated: Release build succeeds, mksnapshot.zip extracted to an isolated directory runs standalone and generates snapshot_blob.bin after CI stripping is applied.

Future work: For full coverage of the 11 Promise/async builtins, we could generate Electron-specific profiles by running v8/tools/builtins-pgo/generate.py with our V8 gn args against JetStream2 — roughly a 15-20 min CI job per arch that could trigger on Chromium rolls.

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes
  • PR release notes describe the change in a way relevant to app developers

Release Notes

Notes: Enabled profile-guided optimization for V8 builtins in release builds, improving JavaScript builtin performance (Array, String, RegExp, etc.).

@MarshallOfSound MarshallOfSound requested review from a team as code owners March 22, 2026 11:06
@MarshallOfSound
Copy link
Copy Markdown
Member Author

MarshallOfSound commented Mar 22, 2026

We should backport this to supported release lines as a perf win once it's validated in a successful nightly build.

deepak1556
deepak1556 approved these changes Mar 22, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failures on linux are unrelated to this change, refs #48149 (comment)

Comment thread patches/v8/build_warn_instead_of_abort_on_builtin_pgo_profile_mismatch.patch Outdated
@MarshallOfSound
build: enable V8 builtins PGO
0fd9798 
Removes the gn arg that disabled V8 builtins profile-guided optimization
and adds a V8 patch to warn instead of abort when the builtin PGO profile
data does not match. Also strips the PGO-related flags from the generated
mksnapshot_args so they are not passed through to downstream mksnapshot
invocations.
@MarshallOfSound
docs: clarify Node.js async_hooks as reason for promise_hooks flag
93902d1 
Addresses review feedback: the v8_enable_javascript_promise_hooks flag
is set to support Node.js async_hooks, not used directly by Electron.
@MarshallOfSound MarshallOfSound force-pushed the sattard/enable-v8-builtins-pgo branch from 80029ef to 93902d1 Compare March 23, 2026 05:54
@jkleinsc jkleinsc merged commit 29750dd into main Mar 23, 2026
67 checks passed
@jkleinsc jkleinsc deleted the sattard/enable-v8-builtins-pgo branch March 23, 2026 15:54
@release-clerk
Copy link
Copy Markdown

release-clerk Bot commented Mar 23, 2026

Release Notes Persisted

Enabled profile-guided optimization for V8 builtins in release builds, improving JavaScript builtin performance (Array, String, RegExp, etc.).

@MarshallOfSound
Copy link
Copy Markdown
Member Author

MarshallOfSound commented Mar 30, 2026

/trop run backport-to 42-x-y, 41-x-y, 40-x-y

@trop
Copy link
Copy Markdown
Contributor

trop Bot commented Mar 30, 2026

The backport process for this PR has been manually initiated - sending your PR to 42-x-y!

@trop
Copy link
Copy Markdown
Contributor

trop Bot commented Mar 30, 2026

The backport process for this PR has been manually initiated - sending your PR to 41-x-y!

@trop
Copy link
Copy Markdown
Contributor

trop Bot commented Mar 30, 2026

The backport process for this PR has been manually initiated - sending your PR to 40-x-y!

@trop trop Bot mentioned this pull request Mar 30, 2026
Merged
@trop
Copy link
Copy Markdown
Contributor

trop Bot commented Mar 30, 2026

I have automatically backported this PR to "42-x-y", please check out #50573

This was referenced Mar 30, 2026
Merged
Merged
@trop
Copy link
Copy Markdown
Contributor

trop Bot commented Mar 30, 2026

I have automatically backported this PR to "41-x-y", please check out #50574

@trop
Copy link
Copy Markdown
Contributor

trop Bot commented Mar 30, 2026

I have automatically backported this PR to "40-x-y", please check out #50575

@trop trop Bot added in-flight/40-x-y merged/42-x-y PR was merged to the "42-x-y" branch. and removed in-flight/42-x-y labels Mar 30, 2026
@trop trop Bot added merged/41-x-y PR was merged to the "41-x-y" branch. merged/40-x-y PR was merged to the "40-x-y" branch. and removed in-flight/41-x-y in-flight/40-x-y labels Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkleinsc jkleinsc jkleinsc approved these changes

@deepak1556 deepak1556 deepak1556 approved these changes

@codebytere codebytere codebytere approved these changes

Assignees

No one assigned

Labels

merged/40-x-y PR was merged to the "40-x-y" branch. merged/41-x-y PR was merged to the "41-x-y" branch. merged/42-x-y PR was merged to the "42-x-y" branch. semver/none

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MarshallOfSound @jkleinsc @deepak1556 @codebytere