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.

Sign up for GitHub

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

Jump to bottom

test: reduce test-benchmark-net run duration #14183

Closed
wants to merge 2 commits into from
Closed

test: reduce test-benchmark-net run duration #14183

wants to merge 2 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Jul 12, 2017

Set configuration option to reduce combinations of benchmark settings
tried in test, reducing execution time by about 50%.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@Trott
test: reduce test-benchmark-net run duration
6143a60 
Set configuration option to reduce combinations of benchmark settings
tried in test, reducing execution time by about 50%.
@Trott Trott added flaky-test Issues and PRs related to the tests with unstable failures on the CI. test Issues and PRs related to the tests. labels Jul 12, 2017
@Trott
Copy link
Member Author

Trott commented Jul 12, 2017

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

@mscdex mscdex added benchmark Issues and PRs related to the benchmark subsystem. net Issues and PRs related to the net subsystem. labels Jul 12, 2017
@Trott
Copy link
Member Author

Trott commented Jul 12, 2017

Just from results on Pi 3, this fixes (or at least mitigates) the timeout issues with the test we've been seeing in CI since the release several hours ago. (The security fix in the release affects performance, so the connection there makes sense. Subsequent release will re-implement the improved performance without the security issue.)

@Trott
Copy link
Member Author

Trott commented Jul 12, 2017

Expedited landing, anyone?

@nodejs/testing

@Trott
Copy link
Member Author

Trott commented Jul 12, 2017

Alas, this was enough to get it to finish running on Pi 3, but not Pi 2, and I suspect Pi 1 will continue to have difficulties. Additional improvements coming in a little bit...

@Trott
Copy link
Member Author

Trott commented Jul 12, 2017

Additional config should cut run time by more like 75%. Let's try again.

Ci: https://ci.nodejs.org/job/node-test-pull-request/9081/

@Trott
Copy link
Member Author

Trott commented Jul 12, 2017

Hooray, that seemed to fix it on Pi 2. Only question mark now is Pi 1, because there was a build/infra fail there. Will try again:

CI for Pi devices again (hope I did it right): https://ci.nodejs.org/job/node-test-binary-arm/9170/

@gibfahn
Copy link
Member

gibfahn commented Jul 12, 2017

cc/ @nodejs/performance @nodejs/benchmarking

refack
refack approved these changes Jul 12, 2017
View reviewed changes
Copy link
Contributor

@refack refack left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
👍 on expedite

@Trott
Copy link
Member Author

Trott commented Jul 12, 2017

Landed in 82aa34e

@Trott Trott closed this Jul 12, 2017
Trott added a commit that referenced this pull request Jul 12, 2017
@Trott
test: reduce test-benchmark-net run duration
82aa34e 
Set configuration option to reduce combinations of benchmark settings
tried in test, reducing execution time by about 50%.

PR-URL: #14183
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
addaleax pushed a commit that referenced this pull request Jul 12, 2017
@Trott @addaleax
test: reduce test-benchmark-net run duration
8d1f3da 
Set configuration option to reduce combinations of benchmark settings
tried in test, reducing execution time by about 50%.

PR-URL: #14183
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@addaleax addaleax mentioned this pull request Jul 18, 2017
Merged
addaleax pushed a commit that referenced this pull request Jul 18, 2017
@Trott @addaleax
test: reduce test-benchmark-net run duration
cd9eba9 
Set configuration option to reduce combinations of benchmark settings
tried in test, reducing execution time by about 50%.

PR-URL: #14183
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Jul 19, 2017
@Trott @Fishrock123
test: reduce test-benchmark-net run duration
6c23e0a 
Set configuration option to reduce combinations of benchmark settings
tried in test, reducing execution time by about 50%.

PR-URL: #14183
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@Trott Trott deleted the shorter-net-bench-test branch January 13, 2022 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gibfahn gibfahn gibfahn approved these changes

@gareth-ellis gareth-ellis gareth-ellis approved these changes

@cjihrig cjihrig cjihrig approved these changes

@benjamingr benjamingr benjamingr approved these changes

@refack refack refack approved these changes

Assignees
No one assigned
Labels
benchmark Issues and PRs related to the benchmark subsystem. flaky-test Issues and PRs related to the tests with unstable failures on the CI. net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@Trott @gibfahn @refack @benjamingr @cjihrig @gareth-ellis @mscdex @MylesBorins