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

ci(perf): Set maxWorkers to match the number of cores available #6249

Merged
merged 1 commit into from
Aug 9, 2018

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Aug 9, 2018

Summary

AppVeyor VMs have 2 cores, yet we were telling Jest to use 3 workers. Changing it to 2 should reduce
overhead and speed up the test suite.

From the limited testing I've done on AppVeyor, the speed improvement is 10% at best, probably less. But every bit helps, especially when we're seeing tests fail from timeouts.

Test plan

I tested this by running AppVeyor builds. They completed successfully, and were marginally faster on average.

@Gudahtt Gudahtt changed the title test(perf): Set maxWorkers to match the number of cores available ci(perf): Set maxWorkers to match the number of cores available Aug 9, 2018
AppVeyor VMs have 2 cores, yet we were telling Jest to use 3 workers. Changing it to 2 should reduce
overhead and speed up the test suite.
@Gudahtt Gudahtt force-pushed the improve-appveyor-test-performance branch from 36db3bd to 944f278 Compare August 9, 2018 11:58
@Gudahtt
Copy link
Member Author

Gudahtt commented Aug 9, 2018

Interesting. The build succeeded both times.

Perhaps this setting was responsible for the flakiness we've seen in the Windows tests. Or maybe that's wishful thinking.

@arcanis
Copy link
Member

arcanis commented Aug 9, 2018

giphy 2

@arcanis arcanis merged commit 0155739 into yarnpkg:master Aug 9, 2018
@Gudahtt Gudahtt deleted the improve-appveyor-test-performance branch August 9, 2018 15:14
@Gudahtt
Copy link
Member Author

Gudahtt commented Aug 10, 2018

Hmm. It looks like it did help, but some tests are still timing out.

Ah well. At least it has improved.

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

Successfully merging this pull request may close these issues.

2 participants