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

cmd/compile: ~19% of kubernetes build time is Type.HasShape1 on dev.typeparams #47456

Closed
mdempsky opened this issue Jul 29, 2021 · 4 comments
Closed
Assignees
Milestone

Comments

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Jul 29, 2021

With kubernetes v1.21.1 (5e58841cce77d4bc13713ad2b91fa0d961e69192) and latest Go dev.typeparams (4a47e40), according to perf report we're now spending about 19% of total build time within cmd/compile/internal/types.(*Type).HasShape1.

Within a checkout of github.com/kubernetes/kubernetes, run:

perf record -g go build -a -mod=mod ./cmd/kubelet
perf report

(Nothing special about kubernetes or v1.12.1; just what I happened to have handy and was using for profiling unified IR when I noticed Type.HasShape1.)

I know the GC shape code is new and likely to undergo a lot of changes in the near future anyway; but since this is a build performance impact that already affects normal, non-generics-enabled builds, I thought I'd mention it.

/cc @randall77 @danscales

@mdempsky mdempsky added this to the Go1.18 milestone Jul 29, 2021
@randall77
Copy link
Contributor

@randall77 randall77 commented Jul 29, 2021

Yes, I'm not surprised. I think we'll need something similar to HasTParam where we set it when building the type so we don't have to recursively look through deep types.

Loading

@danscales
Copy link

@danscales danscales commented Jul 29, 2021

Yes, thanks for noting this @mdempsky , we will try to fix HasShape to be similar to HasTParam in the next week or two.

Loading

@danscales danscales self-assigned this Aug 2, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Aug 2, 2021

Change https://golang.org/cl/339029 mentions this issue: [dev.typeparams] cmd/compile: make HasShape() more efficient by implementing with a type flag

Loading

gopherbot pushed a commit that referenced this issue Aug 2, 2021
…menting with a type flag

Implement HasShape() similar to how HasTParam() is implemented.

Fixes #47456

Change-Id: Icbd538574237faad2c4cd8c8e187725a1df47637
Reviewed-on: https://go-review.googlesource.com/c/go/+/339029
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
@randall77
Copy link
Contributor

@randall77 randall77 commented Aug 4, 2021

Hm, @gopherbot didn't seem to close this.

Loading

@randall77 randall77 closed this Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants