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

Optimize child_process performance #138

Open
mehulkar opened this issue Nov 25, 2023 · 4 comments
Open

Optimize child_process performance #138

mehulkar opened this issue Nov 25, 2023 · 4 comments

Comments

@mehulkar
Copy link

mehulkar commented Nov 25, 2023

child_process takes ~20ms to run a script with execSync. If this can be optimized further, that would be great.

Use case

I have ported some bash scripts to Node.js to make them more easily cross-platform. But sometimes that means using child_process to call out to other programs (e.g. git). These calls can add up and make me think twice about the performance tradeoff.

Benchmark

Here's a benchmark: https://github.com/mehulkar/bench-childprocess

Ref

@H4ad
Copy link
Member

H4ad commented Nov 25, 2023

We already tried to optimize it once: #89

@anonrig
Copy link
Member

anonrig commented Nov 25, 2023

We already tried to optimize it once: #89

There is still room for optimizations probably

isker added a commit to isker/node that referenced this issue 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
@isker
Copy link

isker commented Jan 15, 2024

child_process takes ~20ms to run a script with execSync

I don't think your benchmark is really measuring child_process spawn performance, just the overhead of launching node compared to bash. Comment out every line in the node script and see how little the timing changes. For me on macOS, the hyperfine-measured time goes from ~38ms to ~34ms.

@isker
Copy link

isker commented Jan 15, 2024

4ms is still a lot, but a lot less crazy than 20ms or 38ms 🌞.

nodejs-github-bot pushed a commit to nodejs/node that referenced this issue 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 <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Medhansh404 pushed a commit to Medhansh404/node that referenced this issue 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 <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this issue 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 <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this issue 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 <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit to nodejs/node that referenced this issue 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 <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this issue 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 <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
richardlau pushed a commit to nodejs/node that referenced this issue 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 <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
richardlau pushed a commit to nodejs/node that referenced this issue 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 <kvakil@sylph.kvakil.me>
Reviewed-By: Luigi Pinca <luigipinca@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

4 participants