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.spawm is slow? #89

Closed
ronag opened this issue May 29, 2023 · 3 comments
Closed

child_process.spawm is slow? #89

ronag opened this issue May 29, 2023 · 3 comments

Comments

@ronag
Copy link
Member

ronag commented May 29, 2023

Don't have any real numbers but ran some profiling on one of our services that makes heavy use of spawning cli tools to perform tasks and the spawn method is taking up significant time.

@santigimeno
Copy link
Member

This might be of interest: libuv/libuv#2133

@H4ad
Copy link
Member

H4ad commented Jun 18, 2023

To add more numbers to this issue, I did some exploration on execSync:

execSync calls spawnSync, which has a similar performance to spawn itself.

Benchmark

exec x 272 ops/sec ±5.89% (59 runs sampled)
execSync x 176 ops/sec ±4.25% (68 runs sampled)
Fastest is exec
benchmark.js
var Benchmark = require('benchmark');
var childProcess = require('child_process');

var suite = new Benchmark.Suite();
var command = 'echo "123"';

suite.add(`exec`, async function () {
  const r = await new Promise(resolve => {
    childProcess.exec(command, (err, out) => {
      resolve(out);
    });
  });
});

suite.add(`execSync`, function () {
  const r = childProcess.execSync(command, { encoding: 'utf8' });
});

suite
  // add listeners
  .on('cycle', function (event) {
    console.log(String(event.target));
  })
  .on('complete', function () {
    console.log('Fastest is ' + this.filter('fastest').map('name'));
  })
  .run({
    async: true,
  });

Adding some logs

Below, is the amount of time each part of the code takes when we call execSync:

First Execution

execSync normalizeExecArgs: 0.058121003210544586
execSync spawnSync > normalizeSpawnArguments: 0.6293980032205582
execSync spawnSync > validation: 0.06221800297498703
execSync spawnSync > sanitizeKillSignal: 0.03522700071334839
execSync spawnSync > stdio & input: 0.24450600147247314
execSync spawnSync > child_process.spawnSync: 2.081940993666649
execSync spawnSync: 3.4516379982233047
execSync inheritStderr: 0.002504996955394745
execSync checkExecSyncError: 0.08398999273777008
Total: 6.673679001629353ms

There is some lag in the first execution that I don't know what is causing, module compilation?

Second Execution

execSync normalizeExecArgs: 0.0116720050573349
execSync spawnSync > normalizeSpawnArguments: 0.2574800029397011
execSync spawnSync > validation: 0.004238002002239227
execSync spawnSync > sanitizeKillSignal: 0.001993998885154724
execSync spawnSync > stdio & input: 0.015389002859592438
execSync spawnSync > child_process.spawnSync: 1.9157060012221336
execSync spawnSync: 2.572445996105671
execSync inheritStderr: 0.0013229995965957642
execSync checkExecSyncError: 0.0015130043029785156
Total: 2.7211980000138283ms

Execute single-child-sync.js with the node compiled from h4ad-forks/node@856245f

single-child-sync.js
var command = 'echo "123"';
var childProcess = require('child_process');

var now = performance.now();
childProcess.execSync(command, { encoding: 'utf8' });
console.log(`Total: ${performance.now() - now}ms`);

now = performance.now();
childProcess.execSync(command, { encoding: 'utf8' });
console.log(`Total: ${performance.now() - now}ms`);

With these numbers, we could probably optimize the JS side a little bit to reach 5~10%, but I don't think we can extract much from the JS side, most likely the work will be on the C++ side.

kvakil added a commit to kvakil/node that referenced this issue Jun 22, 2023
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
kvakil added a commit to kvakil/node that referenced this issue Jun 22, 2023
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
kvakil added a commit to kvakil/node that referenced this issue Jun 22, 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
kvakil added a commit to kvakil/node that referenced this issue Jun 22, 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
nodejs-github-bot pushed a commit to nodejs/node that referenced this issue Jun 24, 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
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]>
nodejs-github-bot pushed a commit to nodejs/node that referenced this issue Jun 24, 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
Refs: v8/v8@1a782f6
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
RafaelGSS pushed a commit to nodejs/node that referenced this issue Jul 3, 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
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]>
RafaelGSS pushed a commit to nodejs/node that referenced this issue Jul 3, 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
Refs: v8/v8@1a782f6
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Debadree Chatterjee <[email protected]>
targos pushed a commit to targos/node that referenced this issue Jul 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]>
targos pushed a commit to targos/node that referenced this issue Jul 10, 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]>
targos pushed a commit to targos/node that referenced this issue Jul 10, 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]>
@H4ad
Copy link
Member

H4ad commented Jul 24, 2023

nodejs/node#48523 was merged and it improved by 53% and ~9% of spawn and spawnSync, should we close this issue?

targos pushed a commit to targos/node that referenced this issue Jul 31, 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]>
targos pushed a commit to targos/node that referenced this issue Jul 31, 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]>
@anonrig anonrig closed this as completed Aug 7, 2023
Ceres6 pushed a commit to Ceres6/node that referenced this issue 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 issue 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 issue 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 issue 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]>
aduh95 pushed a commit to aduh95/node that referenced this issue 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 issue 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]>
targos pushed a commit to nodejs/node that referenced this issue 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 to nodejs/node that referenced this issue 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]>
sercher added a commit to sercher/graaljs that referenced this issue 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 issue 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 issue 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 issue 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
None yet
Projects
None yet
Development

No branches or pull requests

4 participants