Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

child_process: improve spawn performance on Linux #48523

Closed
wants to merge 2 commits into from

Conversation

kvakil
Copy link
Contributor

@kvakil kvakil commented Jun 22, 2023

deps: V8: cherry-pick 1a782f6543ae

Original commit message:

[base] add build flag to use MADV_DONTFORK

Embedders like Node.js and Electron expose fork(2)/execve(2) to their
users. Unfortunately when the V8 heap is very large, these APIs become
rather slow on Linux, due to the kernel needing to do all the
bookkeeping for the forked process (in clone's dup_mmap and execve's
exec_mmap). Of course, this is useless because the forked child thread
will never actually need to access the V8 heap.

Add a new build flag v8_enable_private_mapping_fork_optimization which
marks all pages allocated by OS::Allocate as MADV_DONTFORK. This
improves the performance of Node.js's fork/execve combination by 10x on
a 600 MB heap.

Fixed: v8:7381
Change-Id: Ib649f774d4a932b41886313ce89acc369923699d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4602858
Commit-Queue: Michael Lippautz <[email protected]>
Reviewed-by: Michael Lippautz <[email protected]>
Cr-Commit-Position: refs/heads/main@{#88447}

Refs: v8/v8@1a782f6

child_process: improve spawn performance on Linux

Speed up child_process.spawn by enabling the new V8 build flag which
makes fork/exec faster.

Here are the results of running the existing benchmark. Note that this
optimization helps more for applications with larger heaps, so this is
somewhat of an underestimate of the real world performance benefits.

$ ./node benchmark/compare.js --runs 15 \
        --new ./node \
        --old ~/node-v20/out/Release/node \
        --filter params child_process > cpr
$ node-benchmark-compare cpr
                                 confidence improvement  (***)
methodName='exec' n=1000                ***     60.84 % ±5.43%
methodName='execFile' n=1000            ***     53.72 % ±3.33%
methodName='execFileSync' n=1000        ***      9.10 % ±0.84%
methodName='execSync' n=1000            ***     10.44 % ±0.97%
methodName='spawn' n=1000               ***     53.10 % ±2.90%
methodName='spawnSync' n=1000           ***      8.64 % ±1.22%

  0.01 false positives, when considering a 0.1% risk acceptance (***)

Fixes: #25382
Fixes: #14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89

Original commit message:

    [base] add build flag to use MADV_DONTFORK

    Embedders like Node.js and Electron expose fork(2)/execve(2) to their
    users. Unfortunately when the V8 heap is very large, these APIs become
    rather slow on Linux, due to the kernel needing to do all the
    bookkeeping for the forked process (in clone's dup_mmap and execve's
    exec_mmap). Of course, this is useless because the forked child thread
    will never actually need to access the V8 heap.

    Add a new build flag v8_enable_private_mapping_fork_optimization which
    marks all pages allocated by OS::Allocate as MADV_DONTFORK. This
    improves the performance of Node.js's fork/execve combination by 10x on
    a 600 MB heap.

    Fixed: v8:7381
    Change-Id: Ib649f774d4a932b41886313ce89acc369923699d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4602858
    Commit-Queue: Michael Lippautz <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#88447}

Refs: v8/v8@1a782f6
@kvakil kvakil added child_process Issues and PRs related to the child_process subsystem. v8 engine Issues and PRs related to the V8 dependency. performance Issues and PRs related to the performance of Node.js. labels Jun 22, 2023
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Jun 22, 2023
@kvakil kvakil added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 22, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 22, 2023
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

Speed up child_process.spawn by enabling the new V8 build flag which
makes fork/exec faster.

Here are the results of running the existing benchmark. Note that this
optimization helps more for applications with larger heaps, so this is
somewhat of an underestimate of the real world performance benefits.

```console
$ ./node benchmark/compare.js --runs 15 \
        --new ./node \
        --old ~/node-v20/out/Release/node \
        --filter params child_process > cpr
$ node-benchmark-compare cpr
                                 confidence improvement  (***)
methodName='exec' n=1000                ***     60.84 % ±5.43%
methodName='execFile' n=1000            ***     53.72 % ±3.33%
methodName='execFileSync' n=1000        ***      9.10 % ±0.84%
methodName='execSync' n=1000            ***     10.44 % ±0.97%
methodName='spawn' n=1000               ***     53.10 % ±2.90%
methodName='spawnSync' n=1000           ***      8.64 % ±1.22%

  0.01 false positives, when considering a 0.1% risk acceptance (***)
```

Fixes: nodejs#25382
Fixes: nodejs#14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89
@nodejs-github-bot
Copy link
Collaborator

@kvakil kvakil added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 22, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 22, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@kvakil kvakil added the review wanted PRs that need reviews. label Jun 23, 2023
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
Speed up child_process.spawn by enabling the new V8 build flag which
makes fork/exec faster.

Here are the results of running the existing benchmark. Note that this
optimization helps more for applications with larger heaps, so this is
somewhat of an underestimate of the real world performance benefits.

```console
$ ./node benchmark/compare.js --runs 15 \
        --new ./node \
        --old ~/node-v20/out/Release/node \
        --filter params child_process > cpr
$ node-benchmark-compare cpr
                                 confidence improvement  (***)
methodName='exec' n=1000                ***     60.84 % ±5.43%
methodName='execFile' n=1000            ***     53.72 % ±3.33%
methodName='execFileSync' n=1000        ***      9.10 % ±0.84%
methodName='execSync' n=1000            ***     10.44 % ±0.97%
methodName='spawn' n=1000               ***     53.10 % ±2.90%
methodName='spawnSync' n=1000           ***      8.64 % ±1.22%

  0.01 false positives, when considering a 0.1% risk acceptance (***)
```

Fixes: nodejs#25382
Fixes: nodejs#14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89
PR-URL: nodejs#48523
Refs: v8/v8@1a782f6
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
Original commit message:

    [base] add build flag to use MADV_DONTFORK

    Embedders like Node.js and Electron expose fork(2)/execve(2) to their
    users. Unfortunately when the V8 heap is very large, these APIs become
    rather slow on Linux, due to the kernel needing to do all the
    bookkeeping for the forked process (in clone's dup_mmap and execve's
    exec_mmap). Of course, this is useless because the forked child thread
    will never actually need to access the V8 heap.

    Add a new build flag v8_enable_private_mapping_fork_optimization which
    marks all pages allocated by OS::Allocate as MADV_DONTFORK. This
    improves the performance of Node.js's fork/execve combination by 10x on
    a 600 MB heap.

    Fixed: v8:7381
    Change-Id: Ib649f774d4a932b41886313ce89acc369923699d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4602858
    Commit-Queue: Michael Lippautz <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#88447}

Refs: v8/v8@1a782f6
PR-URL: nodejs#48523
Fixes: nodejs#25382
Fixes: nodejs#14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
Speed up child_process.spawn by enabling the new V8 build flag which
makes fork/exec faster.

Here are the results of running the existing benchmark. Note that this
optimization helps more for applications with larger heaps, so this is
somewhat of an underestimate of the real world performance benefits.

```console
$ ./node benchmark/compare.js --runs 15 \
        --new ./node \
        --old ~/node-v20/out/Release/node \
        --filter params child_process > cpr
$ node-benchmark-compare cpr
                                 confidence improvement  (***)
methodName='exec' n=1000                ***     60.84 % ±5.43%
methodName='execFile' n=1000            ***     53.72 % ±3.33%
methodName='execFileSync' n=1000        ***      9.10 % ±0.84%
methodName='execSync' n=1000            ***     10.44 % ±0.97%
methodName='spawn' n=1000               ***     53.10 % ±2.90%
methodName='spawnSync' n=1000           ***      8.64 % ±1.22%

  0.01 false positives, when considering a 0.1% risk acceptance (***)
```

Fixes: nodejs#25382
Fixes: nodejs#14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89
PR-URL: nodejs#48523
Refs: v8/v8@1a782f6
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
@ruyadorno
Copy link
Member

This commit does not land cleanly on v18.x-staging and will need manual backport in case we want it in v18.

@ruyadorno ruyadorno added the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label Sep 10, 2023
aduh95 pushed a commit to aduh95/node that referenced this pull request Oct 9, 2023
Original commit message:

    [base] add build flag to use MADV_DONTFORK

    Embedders like Node.js and Electron expose fork(2)/execve(2) to their
    users. Unfortunately when the V8 heap is very large, these APIs become
    rather slow on Linux, due to the kernel needing to do all the
    bookkeeping for the forked process (in clone's dup_mmap and execve's
    exec_mmap). Of course, this is useless because the forked child thread
    will never actually need to access the V8 heap.

    Add a new build flag v8_enable_private_mapping_fork_optimization which
    marks all pages allocated by OS::Allocate as MADV_DONTFORK. This
    improves the performance of Node.js's fork/execve combination by 10x on
    a 600 MB heap.

    Fixed: v8:7381
    Change-Id: Ib649f774d4a932b41886313ce89acc369923699d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4602858
    Commit-Queue: Michael Lippautz <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#88447}

Refs: v8/v8@1a782f6
PR-URL: nodejs#48523
Fixes: nodejs#25382
Fixes: nodejs#14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
aduh95 pushed a commit to aduh95/node that referenced this pull request Oct 9, 2023
Speed up child_process.spawn by enabling the new V8 build flag which
makes fork/exec faster.

Here are the results of running the existing benchmark. Note that this
optimization helps more for applications with larger heaps, so this is
somewhat of an underestimate of the real world performance benefits.

```console
$ ./node benchmark/compare.js --runs 15 \
        --new ./node \
        --old ~/node-v20/out/Release/node \
        --filter params child_process > cpr
$ node-benchmark-compare cpr
                                 confidence improvement  (***)
methodName='exec' n=1000                ***     60.84 % ±5.43%
methodName='execFile' n=1000            ***     53.72 % ±3.33%
methodName='execFileSync' n=1000        ***      9.10 % ±0.84%
methodName='execSync' n=1000            ***     10.44 % ±0.97%
methodName='spawn' n=1000               ***     53.10 % ±2.90%
methodName='spawnSync' n=1000           ***      8.64 % ±1.22%

  0.01 false positives, when considering a 0.1% risk acceptance (***)
```

Fixes: nodejs#25382
Fixes: nodejs#14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89
PR-URL: nodejs#48523
Refs: v8/v8@1a782f6
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
@aduh95 aduh95 added backport-open-v18.x Indicate that the PR has an open backport. and removed backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. labels Oct 9, 2023
targos pushed a commit that referenced this pull request Nov 26, 2023
Original commit message:

    [base] add build flag to use MADV_DONTFORK

    Embedders like Node.js and Electron expose fork(2)/execve(2) to their
    users. Unfortunately when the V8 heap is very large, these APIs become
    rather slow on Linux, due to the kernel needing to do all the
    bookkeeping for the forked process (in clone's dup_mmap and execve's
    exec_mmap). Of course, this is useless because the forked child thread
    will never actually need to access the V8 heap.

    Add a new build flag v8_enable_private_mapping_fork_optimization which
    marks all pages allocated by OS::Allocate as MADV_DONTFORK. This
    improves the performance of Node.js's fork/execve combination by 10x on
    a 600 MB heap.

    Fixed: v8:7381
    Change-Id: Ib649f774d4a932b41886313ce89acc369923699d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4602858
    Commit-Queue: Michael Lippautz <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#88447}

Refs: v8/v8@1a782f6
PR-URL: #48523
Backport-PR-URL: #50098
Fixes: #25382
Fixes: #14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
targos pushed a commit that referenced this pull request Nov 26, 2023
Speed up child_process.spawn by enabling the new V8 build flag which
makes fork/exec faster.

Here are the results of running the existing benchmark. Note that this
optimization helps more for applications with larger heaps, so this is
somewhat of an underestimate of the real world performance benefits.

```console
$ ./node benchmark/compare.js --runs 15 \
        --new ./node \
        --old ~/node-v20/out/Release/node \
        --filter params child_process > cpr
$ node-benchmark-compare cpr
                                 confidence improvement  (***)
methodName='exec' n=1000                ***     60.84 % ±5.43%
methodName='execFile' n=1000            ***     53.72 % ±3.33%
methodName='execFileSync' n=1000        ***      9.10 % ±0.84%
methodName='execSync' n=1000            ***     10.44 % ±0.97%
methodName='spawn' n=1000               ***     53.10 % ±2.90%
methodName='spawnSync' n=1000           ***      8.64 % ±1.22%

  0.01 false positives, when considering a 0.1% risk acceptance (***)
```

Fixes: #25382
Fixes: #14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89
PR-URL: #48523
Backport-PR-URL: #50098
Refs: v8/v8@1a782f6
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
@targos targos added backported-to-v18.x PRs backported to the v18.x-staging branch. and removed backport-open-v18.x Indicate that the PR has an open backport. labels Nov 26, 2023
isker added a commit to isker/node that referenced this pull request Jan 15, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: nodejs#48523
Refs: nodejs/performance#138
nodejs-github-bot pushed a commit that referenced this pull request Jan 18, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: #48523
Refs: nodejs/performance#138
PR-URL: #51467
Reviewed-By: Keyhan Vakil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Medhansh404 pushed a commit to Medhansh404/node that referenced this pull request Jan 19, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: nodejs#48523
Refs: nodejs/performance#138
PR-URL: nodejs#51467
Reviewed-By: Keyhan Vakil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Jan 22, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: nodejs#48523
Refs: nodejs/performance#138
PR-URL: nodejs#51467
Reviewed-By: Keyhan Vakil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Feb 2, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: nodejs#48523
Refs: nodejs/performance#138
PR-URL: nodejs#51467
Reviewed-By: Keyhan Vakil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this pull request Feb 15, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: #48523
Refs: nodejs/performance#138
PR-URL: #51467
Reviewed-By: Keyhan Vakil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Feb 19, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: nodejs#48523
Refs: nodejs/performance#138
PR-URL: nodejs#51467
Reviewed-By: Keyhan Vakil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: #48523
Refs: nodejs/performance#138
PR-URL: #51467
Reviewed-By: Keyhan Vakil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
The linked v8 issue is closed. The NodeJS half of that issue has also
been resolved. While there still may be issues related to
child_process spawn performance, they are not related to this v8 issue.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=7381
Refs: #48523
Refs: nodejs/performance#138
PR-URL: #51467
Reviewed-By: Keyhan Vakil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
Original commit message:

    [base] add build flag to use MADV_DONTFORK

    Embedders like Node.js and Electron expose fork(2)/execve(2) to their
    users. Unfortunately when the V8 heap is very large, these APIs become
    rather slow on Linux, due to the kernel needing to do all the
    bookkeeping for the forked process (in clone's dup_mmap and execve's
    exec_mmap). Of course, this is useless because the forked child thread
    will never actually need to access the V8 heap.

    Add a new build flag v8_enable_private_mapping_fork_optimization which
    marks all pages allocated by OS::Allocate as MADV_DONTFORK. This
    improves the performance of Node.js's fork/execve combination by 10x on
    a 600 MB heap.

    Fixed: v8:7381
    Change-Id: Ib649f774d4a932b41886313ce89acc369923699d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4602858
    Commit-Queue: Michael Lippautz <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#88447}

Refs: v8/v8@1a782f6
PR-URL: nodejs/node#48523
Backport-PR-URL: nodejs/node#50098
Fixes: nodejs/node#25382
Fixes: nodejs/node#14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
Speed up child_process.spawn by enabling the new V8 build flag which
makes fork/exec faster.

Here are the results of running the existing benchmark. Note that this
optimization helps more for applications with larger heaps, so this is
somewhat of an underestimate of the real world performance benefits.

```console
$ ./node benchmark/compare.js --runs 15 \
        --new ./node \
        --old ~/node-v20/out/Release/node \
        --filter params child_process > cpr
$ node-benchmark-compare cpr
                                 confidence improvement  (***)
methodName='exec' n=1000                ***     60.84 % ±5.43%
methodName='execFile' n=1000            ***     53.72 % ±3.33%
methodName='execFileSync' n=1000        ***      9.10 % ±0.84%
methodName='execSync' n=1000            ***     10.44 % ±0.97%
methodName='spawn' n=1000               ***     53.10 % ±2.90%
methodName='spawnSync' n=1000           ***      8.64 % ±1.22%

  0.01 false positives, when considering a 0.1% risk acceptance (***)
```

Fixes: nodejs/node#25382
Fixes: nodejs/node#14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89
PR-URL: nodejs/node#48523
Backport-PR-URL: nodejs/node#50098
Refs: v8/v8@1a782f6
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
Original commit message:

    [base] add build flag to use MADV_DONTFORK

    Embedders like Node.js and Electron expose fork(2)/execve(2) to their
    users. Unfortunately when the V8 heap is very large, these APIs become
    rather slow on Linux, due to the kernel needing to do all the
    bookkeeping for the forked process (in clone's dup_mmap and execve's
    exec_mmap). Of course, this is useless because the forked child thread
    will never actually need to access the V8 heap.

    Add a new build flag v8_enable_private_mapping_fork_optimization which
    marks all pages allocated by OS::Allocate as MADV_DONTFORK. This
    improves the performance of Node.js's fork/execve combination by 10x on
    a 600 MB heap.

    Fixed: v8:7381
    Change-Id: Ib649f774d4a932b41886313ce89acc369923699d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4602858
    Commit-Queue: Michael Lippautz <[email protected]>
    Reviewed-by: Michael Lippautz <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#88447}

Refs: v8/v8@1a782f6
PR-URL: nodejs/node#48523
Backport-PR-URL: nodejs/node#50098
Fixes: nodejs/node#25382
Fixes: nodejs/node#14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
Speed up child_process.spawn by enabling the new V8 build flag which
makes fork/exec faster.

Here are the results of running the existing benchmark. Note that this
optimization helps more for applications with larger heaps, so this is
somewhat of an underestimate of the real world performance benefits.

```console
$ ./node benchmark/compare.js --runs 15 \
        --new ./node \
        --old ~/node-v20/out/Release/node \
        --filter params child_process > cpr
$ node-benchmark-compare cpr
                                 confidence improvement  (***)
methodName='exec' n=1000                ***     60.84 % ±5.43%
methodName='execFile' n=1000            ***     53.72 % ±3.33%
methodName='execFileSync' n=1000        ***      9.10 % ±0.84%
methodName='execSync' n=1000            ***     10.44 % ±0.97%
methodName='spawn' n=1000               ***     53.10 % ±2.90%
methodName='spawnSync' n=1000           ***      8.64 % ±1.22%

  0.01 false positives, when considering a 0.1% risk acceptance (***)
```

Fixes: nodejs/node#25382
Fixes: nodejs/node#14917
Refs: nodejs/performance#93
Refs: nodejs/performance#89
PR-URL: nodejs/node#48523
Backport-PR-URL: nodejs/node#50098
Refs: v8/v8@1a782f6
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backported-to-v18.x PRs backported to the v18.x-staging branch. build Issues and PRs related to build files or the CI. child_process Issues and PRs related to the child_process subsystem. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js. review wanted PRs that need reviews. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
8 participants