chore: enable v8_enable_private_mapping_fork_optimization by default

[deepak1556]

Description of Change

Benchmark test https://gist.github.com/deepak1556/4a585f987c634feae28232b63a5eb0e8

There is not much impact for the chromium child processes since they get forked from zygote process which does not get impacted by the V8 heap in the main process, so most of the gain with this change are for processes forked with child_process api which see an 8-9x improvement in the above test for a 600MB heap.

You can also disable zygote for comparison purpose --no-zygote --no-sandbox and the above test shows a 7x difference with the change. But this is not the most common case in real world as zygote is not disabled unless user decides to disable sandbox for the renderer process.

Release Notes

Notes: Improve fork/execve performance for child_process api on linux

[bpasero]

I think this also requires the change from https://chromium-review.googlesource.com/c/v8/v8/+/4602858 to be backported right? Or will that come in automatically in the Electron betas?

[deepak1556]

The V8 change landed in Chromium 117 and Electron main branch is currently tracking Chromium 118.

[zcbenz]

Can it be set in build/args/all.gn instead of adding a patch?

[deepak1556]

@zcbenz is there a way to use OS specific conditions in all.gn ? The variables get defined in https://source.chromium.org/chromium/chromium/src/+/main:build/config/BUILDCONFIG.gn which gets loaded after all.gn and if I do something like v8_enable_private_mapping_fork_optimization = host_os == "linux" then I get

ERROR at build arg file (use "gn args <out_dir>" to edit):2:12: Undefined identifier host_os
v8_enable_private_mapping_fork_optimization = host_os == "linux"

[zcbenz]

There is no way to do so, I think we can:

1. Always set the flag to true.
2. Patch V8 so the flag is ignored on non-linux.

While there is no set rule on this, but for most build flags setting them do not break builds on unsuppoerted platforms. I think this is something we can upstream.

[jkleinsc]

Can't this just be enabled at build time/in CI like we do for other flags, eg:
https://github.com/electron/electron/blob/main/.circleci/config/base.yml#L130 and https://github.com/electron/electron/blob/main/.circleci/config/base.yml#L591

[deepak1556]

Always set the flag to true.

I think we can go this route, on macOS we use posix_spawn both in Node.js and Chromium to spawn child process and it already ignores copying the MAP_JIT locations, so this change should be a no-op.

@jkleinsc this is more of a feature flag change so would be better to keep it in build/config, IMO build time changes should be restricted to compiler related settings and not hide feature changes.

[zcbenz]

Build fails on mac.

[deepak1556]

There is no MADV_DONTFORK defined in <sdk>/usr/include/sys/mman.h, will fix the failure in upstream.

[deepak1556]

Upstream CL https://chromium-review.googlesource.com/c/v8/v8/+/4780212

[deepak1556]

@codebytere fyi, parallel/test-debugger-random-port-with-inspect-port seems flaky in run https://app.circleci.com/pipelines/github/electron/electron/73591/workflows/36ac8714-2925-4570-b478-7aba60a53d63/jobs/1587948/tests

[deepak1556]

Friendly ping for review @electron/wg-api

[zcbenz]

API LGTM

[jkleinsc]

API LGTM

[release-clerk]

Release Notes Persisted

Improved fork() and execve() performance for child_process API on Linux.