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

benchmark: use let instead of var in child_process #31043

Closed
wants to merge 1 commit into from

Conversation

dnlup
Copy link
Contributor

@dnlup dnlup commented Dec 20, 2019

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

I am getting this error here when running benchmarks though:
Screenshot from 2019-12-20 21-05-44

I ran the benchmark with:

$ node benchmark/run.js child_process

@nodejs-github-bot nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. child_process Issues and PRs related to the child_process subsystem. labels Dec 20, 2019
@Trott
Copy link
Member

Trott commented Dec 21, 2019

You can work around the error in the benchmark by explicitly allowing zeroes with an environment variable:

NODEJS_BENCHMARK_ZERO_ALLOWED=1 node benchmark/run.js child_process

@Trott
Copy link
Member

Trott commented Dec 21, 2019

Also, if you don't actually care to run all the benchmarks and just want a quick test that your changes didn't break anything obvious:

node test/benchmark/test-benchmark-child-process.js

@dnlup
Copy link
Contributor Author

dnlup commented Dec 21, 2019

Thanks @Trott .

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 24, 2019
BridgeAR pushed a commit that referenced this pull request Dec 25, 2019
PR-URL: #31043
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BridgeAR
Copy link
Member

Landed in 3885e15 🎉

@BridgeAR BridgeAR closed this Dec 25, 2019
BridgeAR pushed a commit that referenced this pull request Jan 3, 2020
PR-URL: #31043
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BridgeAR BridgeAR mentioned this pull request Jan 7, 2020
@dnlup dnlup deleted the benchmark_child_process branch January 10, 2020 13:38
targos pushed a commit that referenced this pull request Jan 14, 2020
PR-URL: #31043
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
PR-URL: #31043
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
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. benchmark Issues and PRs related to the benchmark subsystem. child_process Issues and PRs related to the child_process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants