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/go: go list cmd/compile takes 40x longer than go list cmd/... #60455

Closed
findleyr opened this issue May 26, 2023 · 5 comments
Closed

cmd/go: go list cmd/compile takes 40x longer than go list cmd/... #60455

findleyr opened this issue May 26, 2023 · 5 comments
Assignees
@prattmic
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone
Go1.21

Comments

@findleyr
Copy link
Contributor

As in the title:

> time go list cmd/compile
cmd/compile

real	0m8.523s
user	0m8.610s
sys	0m0.068s

vs

> time go list cmd/...
...output, including cmd/compile

real	0m0.200s
user	0m0.492s
sys	0m0.185s

Here is my go version:

> go version
go version devel go1.21-e51b5f8384 Thu May 25 10:06:01 2023 -0700 linux/amd64

CC @bcmills @matloob @adonovan
@bcmills
Copy link
Member

bcmills commented May 26, 2023

This is likely another PGO issue, or another symptom of the same PGO issue. Probably closely related to #60428.
(attn @cherrymui @prattmic)

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go compiler/runtime Issues related to the Go compiler and/or runtime. labels May 26, 2023
@bcmills bcmills added this to the Go1.21 milestone May 26, 2023
@findleyr
Copy link
Contributor Author

I can confirm that go list -pgo=off cmd/compile is ~instantaneous.

@findleyr findleyr mentioned this issue May 26, 2023
Closed
@prattmic prattmic self-assigned this May 26, 2023
@prattmic
Copy link
Member

image

😬

@prattmic
Copy link
Member

The problem is this section: https://cs.opensource.google/go/go/+/refs/heads/master:src/cmd/go/internal/load/pkg.go;l=2976-2981;drc=01b5cce626c44f7fd1ca7e2c076d63c175193ccf

For the copying block above we have a map to avoid copying more than once, but the modification block I linked to runs (recursively!) every time we encounter a package. So if I import "io" and "bufio", we'll visit "io" and all of its dependencies, then we'll visit "bufio" and its dependencies. One of its dependencies is "io", so we'll visit that whole tree again.

I'll send a CL later today.

@prattmic prattmic mentioned this issue May 26, 2023
Closed
@gopherbot
Copy link

Change https://go.dev/cl/498735 mentions this issue: cmd/go: always track visited packages in setPGOProfilePath

Nasfame pushed a commit to golangFame/go that referenced this issue Jun 4, 2023
@prattmic @Nasfame
cmd/go: always track visited packages in setPGOProfilePath
24a1043 
Currently we only track visited (copied) packages when a copy is
required. When a copy is not required, we will rewalk each package's
entire dependency graph every time we see it, which is terribly
inefficient.

Pull the visited package check up a level so that we visit packages only
once regardless of how many times they are visited.

Fixes golang#60455.
Fixes golang#60428.

Change-Id: I4e9b31eeeaa170db650c461a5de2ca984b9aba0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/498735
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@prattmic prattmic

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
4 participants
@prattmic @bcmills @gopherbot @findleyr