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

Run make in parallel instead of adding cpus to tests #2892

Closed
wants to merge 6 commits into from

Conversation

Madrigal
Copy link
Contributor

@Madrigal Madrigal commented Nov 8, 2024

Running tests locally, I noticed that setting "EACH_MODULE_CONCURRENCY" had little effect on how fast we run the tests. I suspect the likely reason is that tests are small, so trying to parallelize something that takes half a second to run achieves very little.

I did notice that running make with the -j flag (for --jobs) speeds this up quite a bit. Locally I run it with the output of nproc, but on GitHub runners I ran it with "4" since both Windows and Ubuntu runners have 4 cores https://docs.github.com/en/actions/using-github-hosted-runners/using-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories

Verified that locally this runs significantly faster, will use this last run as a reference to see how much faster (or slower) these tests run https://github.com/aws/aws-sdk-go-v2/actions/runs/11726345786

@Madrigal Madrigal requested a review from a team as a code owner November 8, 2024 19:22
@lucix-aws
Copy link
Contributor

So, CI seems basically the same speed. Unclear if it would be any different in a codebuild environment.

@Madrigal
Copy link
Contributor Author

Madrigal commented Nov 8, 2024

That was disappointing, based on local results I totally expected this to speed it up at least somehow, it seems to be exactly the same.

Will do some more research, but merging this just as is doesn't seem worth it if we just add more complexity with exactly 0 gains

@Madrigal
Copy link
Contributor Author

Well so this was a fiasco, besides multiple experiments the test time remained motly the same

Takeways

  1. go test uses all available CPUs (as defined by GOMAXPROCS). There's no need to specify cpus if we don't need to cap them
  2. EACH_MODULE_CONCURRENCY doesn't add more CPUs to a process, but rather defines how many commands to execute at a time. On eachmodule we list all modules and then run the commands passed to the process. The variable defines how many commands we execute at a time. Tinkering with it didn't seem to impact much how fast CI tests ran
  3. We could look into adding go workspaces, although the feature seems a bit controversial as discussed in proposal: ref/mod: mention whether go.work files should be checked into VCS golang/go#53502
  4. From other testing environments, we can tell that less cores slows this down and more cores speeds it up, so number of cores definitely impacts how fast we run our tests
  5. Not sure I can pin-point the difference in local testing, but one hypothesis is due to likely due to how fast you can do cache lookups when doing go test A B C vs go test A; go test B; go test C. However, this is pretty irrelevant when you're running all the tests, like on CI environments.

@Madrigal Madrigal closed this Nov 12, 2024
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.

3 participants