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

Speed up goimports usage #11896

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

SirGitsalot
Copy link
Member

Now that the go compiler is live I've started toying with bazel, and the extra time that goimports adds to each compile run is irking me. This PR speeds things up by running goimports once as a last compilation step rather than running it for each generated source file.

goimports performs two functions for us: removing imports that are declared but not used, and adding imports that are used but not declared. The former is fast, the latter is slow - like, a couple of orders of magnitude slower than the former. The difference exists because removing imports requires parsing only the go file that's being fixed, while adding imports potentially parses every go file in the project, the standard library, and in your GOPATH. goimports has an internal cache that avoids some repetitive parsing, and by using a single execution that runs over all of the generated files at once we're able to leverage that cache.

Here's some example timings on my M1 Macbook Pro. Before:

$ time make provider VERSION=beta OUTPUT_PATH="../terraform-provider-google-beta"
real	1m1.976s
user	1m33.130s
sys	2m37.034s

$ time make provider VERSION=ga OUTPUT_PATH="../terraform-provider-google"
real	1m0.521s
user	1m23.280s
sys	2m25.612s

After:

$ time make provider VERSION=beta OUTPUT_PATH="../terraform-provider-google-beta"
real	0m24.659s
user	0m38.855s
sys	0m18.785s

$ time make provider VERSION=ga OUTPUT_PATH="../terraform-provider-google"
real	0m22.059s
user	0m36.664s
sys	0m19.448s

YMMV of course, and full builds benefit a lot more than single product builds.

Ultimately the best way to speed up the compiler will be to include all imports that we're using, and the change to resource.go.tmpl picks the lowest hanging fruit (for context, 10 seconds of the speed up on my machine came from just those five lines). The new --show-import-diffs flag is useful for identifying the files that need import fixes.

@SirGitsalot SirGitsalot requested a review from c2thorn October 2, 2024 20:26
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR hasn't generated any diffs, but I'll let you know if a future commit does.

@c2thorn
Copy link
Member

c2thorn commented Oct 2, 2024

about to be OOO, can you take a look over this @zli82016 ?

@c2thorn c2thorn requested review from zli82016 and removed request for c2thorn October 2, 2024 21:02
mmv1/main.go Show resolved Hide resolved
Copy link
Member

@zli82016 zli82016 left a comment

Choose a reason for hiding this comment

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

LGTM. Amazing improvement.

@SirGitsalot SirGitsalot merged commit 73acb5d into GoogleCloudPlatform:main Oct 3, 2024
10 checks passed
@SirGitsalot SirGitsalot deleted the fasterimports branch October 3, 2024 19:11
karolgorc pushed a commit to karolgorc/magic-modules that referenced this pull request Oct 4, 2024
niharika-98 pushed a commit to niharika-98/magic-modules that referenced this pull request Oct 7, 2024
trodge pushed a commit to trodge/magic-modules that referenced this pull request Oct 10, 2024
karolgorc pushed a commit to karolgorc/magic-modules that referenced this pull request Oct 11, 2024
gontech pushed a commit to gontech/magic-modules that referenced this pull request Oct 16, 2024
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Oct 23, 2024
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Oct 24, 2024
BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request Nov 5, 2024
akshat-jindal-nit pushed a commit to akshat-jindal-nit/magic-modules that referenced this pull request Nov 18, 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.

4 participants