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

Jump to bottom

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

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

mdempsky opened this issue Jul 29, 2021 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.18

Comments

@mdempsky
Copy link
Member

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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 29, 2021
@mdempsky mdempsky added this to the Go1.18 milestone Jul 29, 2021
@randall77
Copy link
Contributor

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.

@danscales
Copy link
Contributor

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

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

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

gopherbot pushed a commit that referenced this issue Aug 2, 2021
@danscales
[dev.typeparams] cmd/compile: make HasShape() more efficient by imple…
156eeb4 
…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

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

@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
4 participants
@mdempsky @danscales @randall77 @gopherbot