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 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
@Trott

This comment has been minimized.

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

This comment has been minimized.

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
@Trott
Trott approved these changes Dec 21, 2019
@dnlup

This comment has been minimized.

Copy link
Contributor Author

dnlup commented Dec 21, 2019

Thanks @Trott .

BridgeAR added 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

This comment has been minimized.

Copy link
Member

BridgeAR commented Dec 25, 2019

Landed in 3885e15 🎉

@BridgeAR BridgeAR closed this Dec 25, 2019
BridgeAR added 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 dnlup:benchmark_child_process branch Jan 10, 2020
targos added 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.