Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

parallelize many cmd/dep tests #608

Merged
merged 2 commits into from
May 23, 2017
Merged

parallelize many cmd/dep tests #608

merged 2 commits into from
May 23, 2017

Conversation

jmank88
Copy link
Collaborator

@jmank88 jmank88 commented May 19, 2017

This change parallelizes many of the cmd/dep tests, and reduces the running time for go test ./cmd/dep/... on my machine from ~21s to ~18s, with room for more improvement with more parallelism (beyond GOMAXPROCS). With go test -parallel 16 ./cmd/dep/..., I can get it down to ~8s. In fact, most of the -parallel 16 speedup comes from pre-existing parallelism, and runs about ~10s on master.

So this PR offers around ~10% speedup, and it may be worth considering a CI test script change to run with more parallelism for an even better payoff.

@jmank88
Copy link
Collaborator Author

jmank88 commented May 22, 2017

The googlebot appears to be hung. Maybe this comment will trigger it...

@sdboyer
Copy link
Member

sdboyer commented May 23, 2017

Seems fine - I love how parallelism works in subtests, but the same approach is so much harder to keep track of in normal tests. Still, an improvement.

I'm not sure what kind of parallelism is really even possible in our build environments. Do you want to take a crack at taking advantage of some GOMAXPROCS changes in this PR, or defer that to a later one?

@jmank88
Copy link
Collaborator Author

jmank88 commented May 23, 2017

I'm not sure what kind of parallelism is really even possible in our build environments.

Yeah I have no idea, though I suspect the payoff here is from the integration tests being low CPU and mostly waiting on the network or whatever else, so even some extra concurrency on a single processor could be effective. On the other hand it may set us back on some of the other tests/packages.

Do you want to take a crack at taking advantage of some GOMAXPROCS changes in this PR, or defer that to a later one?

Let's defer. It might take some experimentation. I'm not sure what the best formula would be for CI, but my gut is to try some factor * number of processors (or GOMAXPROCS, if explicitly set for us). I also wonder if only the integration tests should have their parallelism turned up. If so, then it might be more cleanly done after #609 , if we can target whole packages rather than certain tests with regexps.

@sdboyer
Copy link
Member

sdboyer commented May 23, 2017

SGTM!

@sdboyer sdboyer merged commit 3f59fc9 into golang:master May 23, 2017
@jmank88 jmank88 deleted the par_cmd_tests branch June 7, 2017 23:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants