test: minimize time for child_process benchmark #12518

Trott · 2017-04-19T17:49:32Z

test-benchmark-child-process sometimes times out on Windows in CI.
Minimize the number of configurations that run so it will (hopefully)
not time out anymore.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

Affected core subsystem(s)

test child_process benchmark

test-benchmark-child-process sometimes times out on Windows in CI. Minimize the number of configurations that run so it will (hopefully) not time out anymore.

Trott · 2017-04-19T17:49:39Z

cc @joyeecheung

Trott · 2017-04-19T17:50:45Z

I'd like to get this landed sooner than 48 hours because Windows CI is failing a lot because of this right now. @nodejs/testing @nodejs/build

Trott · 2017-04-19T17:53:29Z

CI: https://ci.nodejs.org/job/node-test-pull-request/7525/

Trott · 2017-04-19T18:35:11Z

Hmmm....CI still timed out on one node, but also had another odd timeout failure.

Took only 2 seconds to run on a machine where it passed... wonder if we just need to clean up some hosts first...maybe something is just odd with test-azure_msft-win2016-x64-4 or something like that...

Running CI again: https://ci.nodejs.org/job/node-test-pull-request/7527/

Trott · 2017-04-19T18:41:03Z

Trying with execSync rather than exec to see if that helps at all.

CI: https://ci.nodejs.org/job/node-test-pull-request/7528/

Trott · 2017-04-19T18:42:59Z

If the above CI jobs don't run cleanly, the thing to do might be to move the test to disabled until we figure out how to make it reliable on Windows. I would have to suspect a bug in the actual benchmark code and not in the test but ¯\(ツ)/¯.

Anyway, hopefully one or both of the above CI runs comes out green and we can at least try this.

vsemozhetbyt · 2017-04-19T19:01:57Z

@Trott Are we sure we have yes.exe in this machine in PATH? Or can we test its functionality aside?

Trott · 2017-04-19T19:47:26Z

@Trott Are we sure we have yes.exe in this machine in PATH?

No idea. Guessing not. @nodejs/build?

Guess that may be the problem. Now wondering if this landed without a clean CI run in the first place...

Trott · 2017-04-19T19:49:58Z

@vsemozhetbyt Took your comment and ran with it: #12326 (comment)

Trott · 2017-04-19T19:55:39Z

Although I don't think it would hang if that's the case...it would exit with ENOENT, I think.

Trott · 2017-04-19T19:59:20Z

No real reason to think this will make a huge difference, but set dur=0 because we can. In the likely event this doesn't solve the problem, I'll move the test to disabled or revert the commit that added it.

CI: https://ci.nodejs.org/job/node-test-pull-request/7529/

Trott · 2017-04-19T20:28:19Z

sigh Windows build failure. Let's try again.

CI: https://ci.nodejs.org/job/node-test-pull-request/7530/

vsemozhetbyt · 2017-04-19T20:55:20Z

What if we --filter these 5 benchmarks one by one here to detect the source of the problem?

Trott · 2017-04-19T21:20:49Z

Set dur=0 seems to have fixed it, although a stress test is probably in order.

Also, dur=0.1 really shouldn't break, so doing what @vsemozhetbyt and trying to figure out which benchmark is blowing up is good too. We can do it in CI, but if someone on @nodejs/platform-windows or @nodejs/build happens to have an accessible Windows 2016 machine and figure out which ones are hanging, that would be more awesome and efficient.

Trott · 2017-04-19T21:43:05Z

Stress test for this PR: https://ci.nodejs.org/job/node-stress-single-test/1156/nodes=win2016/console

Stress test for master (should show failures): https://ci.nodejs.org/job/node-stress-single-test/1157/nodes=win2016/console

Hopefully I didn't make any typos when filling out the Jenkins form...

Trott · 2017-04-19T21:57:54Z

Also, when it's done waiting for one of the other stress jobs to finish, this should figure out which of the five benchmarks are the issue: ~~https://ci.nodejs.org/job/node-stress-single-test/1159/nodes=win2016/~~ whoops I had an error in the branch, will try again...

Trott · 2017-04-19T22:52:18Z

Stress tests are behaving as expected. This branch has run more than 40 times without a problem. Will let it go a bit longer. Master has timed out twice without a successful run so far.

Trott · 2017-04-19T22:54:08Z

Stress test to identify which of the benchmarks is causing problems: https://ci.nodejs.org/job/node-stress-single-test/1161/nodes=win2016/console

Trott · 2017-04-19T22:54:20Z

Meantime: Landing to fix CI.

test-benchmark-child-process sometimes times out on Windows in CI. Minimize the number of configurations that run so it will (hopefully) not time out anymore. Set dur=0. PR-URL: nodejs#12518 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>

Trott · 2017-04-19T22:57:00Z

Landed in 32f77ff. CI should be back to green now.

Trott · 2017-04-20T00:14:13Z

Interestingly, looking at the results on Windows right now for the split out tests, it seems like it may have been a genuine timeout and not anything hanging. Splitting the test into five pieces, they all pass, but take about 30 seconds each to run. Yikes! That seems...odd... https://ci.nodejs.org/job/node-stress-single-test/1161/nodes=win2016/console

Gotta run right now, but maybe if I get some time later, I'll spin up a similar job on another Windows machine and maybe a Linux machine too and compare the run times.

joyeecheung · 2017-04-21T06:51:24Z

Sorry for being a bit late. I think we can probably make two tests for each set of benchmarks, one setting numeric configurations to 0 (mostly for testing if there are any syntax errors) and one setting those to 1 or a value as small as possible (see if they actually run properly). If we ever have to disable the second one, at least we will have the first one in place...

test-benchmark-child-process sometimes times out on Windows in CI. Minimize the number of configurations that run so it will (hopefully) not time out anymore. Set dur=0. PR-URL: #12518 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>

gibfahn · 2017-05-16T12:51:55Z

Should land with #12326.

test: minimize time for child_process benchmark

9adad1c

test-benchmark-child-process sometimes times out on Windows in CI. Minimize the number of configurations that run so it will (hopefully) not time out anymore.

Trott added benchmark Issues and PRs related to the benchmark subsystem. child_process Issues and PRs related to the child_process subsystem. test Issues and PRs related to the tests. labels Apr 19, 2017

cjihrig approved these changes Apr 19, 2017

View reviewed changes

santigimeno approved these changes Apr 19, 2017

View reviewed changes

squash: try execSync instead of sync for more Windows reliability?

28a4412

This was referenced Apr 19, 2017

benchmark: fix CLI arguments check in common.js #12429

Closed

benchmark: reduce string concatenations #12455

Closed

squash: dur=0

87dd4f0

Trott closed this Apr 19, 2017

evanlucas mentioned this pull request May 1, 2017

v7.10.0 proposal #12775

Merged

gibfahn added baking-for-lts PRs that need to wait before landing in a LTS release. lts-watch-v6.x labels May 16, 2017

gibfahn mentioned this pull request May 16, 2017

test: add test for child_process benchmark #12326

Closed

3 tasks

MylesBorins removed the baking-for-lts PRs that need to wait before landing in a LTS release. label Aug 17, 2018

Trott deleted the cp-bench-test branch January 13, 2022 22:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: minimize time for child_process benchmark #12518

test: minimize time for child_process benchmark #12518

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017 •

edited

Loading

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

vsemozhetbyt commented Apr 19, 2017 •

edited

Loading

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

vsemozhetbyt commented Apr 19, 2017 •

edited

Loading

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017 •

edited

Loading

Trott commented Apr 19, 2017 •

edited

Loading

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 20, 2017

joyeecheung commented Apr 21, 2017 •

edited

Loading

gibfahn commented May 16, 2017

test: minimize time for child_process benchmark #12518

test: minimize time for child_process benchmark #12518

Conversation

Trott commented Apr 19, 2017

Checklist

Affected core subsystem(s)

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017 • edited Loading

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

vsemozhetbyt commented Apr 19, 2017 • edited Loading

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

vsemozhetbyt commented Apr 19, 2017 • edited Loading

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017 • edited Loading

Trott commented Apr 19, 2017 • edited Loading

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 19, 2017

Trott commented Apr 20, 2017

joyeecheung commented Apr 21, 2017 • edited Loading

gibfahn commented May 16, 2017

Trott commented Apr 19, 2017 •

edited

Loading

vsemozhetbyt commented Apr 19, 2017 •

edited

Loading

vsemozhetbyt commented Apr 19, 2017 •

edited

Loading

Trott commented Apr 19, 2017 •

edited

Loading

Trott commented Apr 19, 2017 •

edited

Loading

joyeecheung commented Apr 21, 2017 •

edited

Loading