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

async versions of slow libuv calls? #93

Open
ronag opened this issue Jun 2, 2023 · 5 comments
Open

async versions of slow libuv calls? #93

ronag opened this issue Jun 2, 2023 · 5 comments

Comments

@ronag
Copy link
Member

ronag commented Jun 2, 2023

e.g. child_process.spawn and os.cpus() can be very slow calls that cause lag on the main thread. Would it make sense to perform async versions of these calls that run on a worker thread?

@RafaelGSS
Copy link
Member

Shouldn't spawn already be the non-blocking operation? Considering child_process.spawnSync already exists.

@aduh95
Copy link

aduh95 commented Jun 2, 2023

If there's a way to make it more async, I'll be interested to hear that, I have attempted to come up with a promise API for child_process API, and couldn't find a design that made sense. If there's space for more asyncness, that'd be ideal for a new Promise API I think.

@ronag
Copy link
Member Author

ronag commented Jun 2, 2023

Shouldn't spawn already be the non-blocking operation? Considering child_process.spawnSync already exists.

It is async in terms of IO, but it's not async in terms of computation.

@kvakil
Copy link

kvakil commented Jun 8, 2023

see nodejs/node#14917 re async spawn

@metcoder95
Copy link
Member

If there's a way to make it more async, I'll be interested to hear that, I have attempted to come up with a promise API for child_process API, and couldn't find a design that made sense. If there's space for more asyncness, that'd be ideal for a new Promise API I think.

@aduh95 What were the design limitations or issues you faced?

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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <mlippautz@chromium.org>
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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 <yagiz@nizipli.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
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

5 participants