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/go: go list does not scale with GOMAXPROCS #63136

Closed
goto1134 opened this issue Sep 21, 2023 · 8 comments
Closed

cmd/go: go list does not scale with GOMAXPROCS #63136

goto1134 opened this issue Sep 21, 2023 · 8 comments
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. ToolSpeed
Milestone

Comments

@goto1134
Copy link
Contributor

goto1134 commented Sep 21, 2023

What version of Go are you using (go version)?

master ace1494

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH='arm64'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOOS='darwin'

What did you do?

  1. Clone Kubernetes
  2. Run GOMAXPROCS=10 go list -json -m -u -mod=readonly -debug-trace=trace-default.txt all
  3. Run GOMAXPROCS=20 go list -json -m -u -mod=readonly -debug-trace=trace-maxproc.txt all

The second command takes the same amount of time as the first one.
Here are the collected traces:

What did you expect to see?

The second command runs faster and scales, depending on the GOMAXPROCS value.

What did you see instead?

The second comment took the same amount of time as the first one.

Here are the collected traces:
trace-default.txt
trace-maxproc20.txt

From the traces, it's evident that the bottleneck is in getting the modules in a loop here:
https://github.com/golang/go/blob/ace1494d9235be94f1325ab6e45105a446b3224c/src/cmd/go/internal/modload/list.go#L277C1-L286C4

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/530037 mentions this issue: cmd/go: scale go list with GOMAXPROCS

@bcmills bcmills added ToolSpeed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Sep 21, 2023
@bcmills bcmills added this to the Backlog milestone Sep 21, 2023
@goto1134 goto1134 changed the title cmd/go: go list does not scale with GOMAXPROC cmd/go: go list does not scale with GOMAXPROCS Sep 21, 2023
goto1134 added a commit to goto1134/go that referenced this issue Sep 21, 2023
@dany74q
Copy link

dany74q commented Oct 17, 2023

Any update on this one ? It makes life much better for large multi-module mono-repos

@goto1134
Copy link
Contributor Author

@dany74q , the next step is to provide benchmarks. I want to do it sometime later. Probably in November.

I hope this change will be accepted for the February release of Go.

goto1134 added a commit to goto1134/go that referenced this issue Jan 9, 2024
goos: darwin
goarch: arm64
pkg: cmd/go/internal/modload/list_test
                     │   old.txt   │               new.txt                │
                     │   sec/op    │    sec/op     vs base                │
ListModules/Empty-10   98.71µ ± 5%   101.54µ ± 2%        ~ (p=0.143 n=10)
ListModules/Cmd-10     273.8m ± 1%    109.7m ± 1%  -59.93% (p=0.000 n=10)
ListModules/K8S-10     12.199 ± 3%     5.416 ± 3%  -55.60% (p=0.000 n=10)
geomean                69.08m         39.22m       -43.23%

Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Jan 9, 2024
goto1134 added a commit to goto1134/go that referenced this issue Jan 9, 2024
goto1134 added a commit to goto1134/go that referenced this issue Jan 9, 2024
goos: darwin
goarch: arm64
pkg: cmd/go/internal/modload/list_test
                     │   old.txt   │               new.txt                │
                     │   sec/op    │    sec/op     vs base                │
ListModules/Empty-10   98.71µ ± 5%   101.54µ ± 2%        ~ (p=0.143 n=10)
ListModules/Cmd-10     273.8m ± 1%    109.7m ± 1%  -59.93% (p=0.000 n=10)
ListModules/K8S-10     12.199 ± 3%     5.416 ± 3%  -55.60% (p=0.000 n=10)
geomean                69.08m         39.22m       -43.23%

Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Jan 9, 2024
goos: darwin
goarch: arm64
pkg: cmd/go/internal/modload/list_test
                     │   old.txt   │               new.txt               │
                     │   sec/op    │   sec/op     vs base                │
ListModules/Empty-10   124.0µ ± 2%   125.7µ ± 1%        ~ (p=0.063 n=10)
ListModules/Cmd-10     403.3m ± 0%   109.7m ± 1%  -72.80% (p=0.000 n=10)
ListModules/K8S-10     18.261 ± 0%    5.406 ± 2%  -70.39% (p=0.000 n=10)
geomean                97.01m        42.09m       -56.61%

Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Jan 10, 2024
goos: darwin
goarch: arm64
pkg: cmd/go/internal/modload/list_test
                     │   old.txt   │               new.txt               │
                     │   sec/op    │   sec/op     vs base                │
ListModules/Empty-10   124.0µ ± 2%   125.7µ ± 1%        ~ (p=0.063 n=10)
ListModules/Cmd-10     403.3m ± 0%   109.7m ± 1%  -72.80% (p=0.000 n=10)
ListModules/K8S-10     18.261 ± 0%    5.406 ± 2%  -70.39% (p=0.000 n=10)
geomean                97.01m        42.09m       -56.61%

Fixes golang#63136
@AdallomRoy
Copy link

@goto1134 any updates on this?

@goto1134
Copy link
Contributor Author

@AdallomRoy, the PR is still being reviewed and waiting for me to revisit the performance tests after the latest review comments. It's my next task on the list 🙂

goto1134 added a commit to goto1134/go that referenced this issue Apr 30, 2024
goto1134 added a commit to goto1134/go that referenced this issue Apr 30, 2024
goto1134 added a commit to goto1134/go that referenced this issue Apr 30, 2024
goos: darwin
goarch: arm64
pkg: cmd/go
cpu: Apple M1 Max
                     │   old.txt   │               new.txt               │
                     │   sec/op    │   sec/op     vs base                │
ListModules/Empty-10   6.414m ± 0%   6.154m ± 3%   -4.06% (p=0.000 n=20)
ListModules/Cmd-10     823.8m ± 3%   296.6m ± 1%  -63.99% (p=0.000 n=20)
ListModules/K8S-10     37.075 ± 3%    9.003 ± 3%  -75.72% (p=0.000 n=20)
geomean                580.8m        254.2m       -56.22%

                     │   old.txt   │               new.txt               │
                     │ sys-sec/op  │ sys-sec/op   vs base                │
ListModules/Empty-10   2.065m ± 1%   1.985m ± 3%   -3.86% (p=0.000 n=20)
ListModules/Cmd-10     43.48m ± 5%   33.32m ± 6%  -23.37% (p=0.000 n=20)
ListModules/K8S-10      2.106 ± 9%    1.798 ± 5%  -14.64% (p=0.000 n=20)
geomean                57.40m        49.18m       -14.33%

                     │    old.txt    │               new.txt               │
                     │  user-sec/op  │ user-sec/op  vs base                │
ListModules/Empty-10    2.459m ±  0%   2.419m ± 2%   -1.63% (p=0.000 n=20)
ListModules/Cmd-10      33.55m ±  3%   26.73m ± 8%  -20.30% (p=0.000 n=20)
ListModules/K8S-10     1106.9m ± 11%   834.4m ± 7%  -24.62% (p=0.000 n=20)
geomean                 45.03m         37.79m       -16.08%

Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Apr 30, 2024
goos: darwin
goarch: arm64
pkg: cmd/go
cpu: Apple M1 Max
                     │   old.txt   │               new.txt               │
                     │   sec/op    │   sec/op     vs base                │
ListModules/Empty-10   6.414m ± 0%   6.154m ± 3%   -4.06% (p=0.000 n=20)
ListModules/Cmd-10     823.8m ± 3%   296.6m ± 1%  -63.99% (p=0.000 n=20)
ListModules/K8S-10     37.075 ± 3%    9.003 ± 3%  -75.72% (p=0.000 n=20)
geomean                580.8m        254.2m       -56.22%

                     │   old.txt   │               new.txt               │
                     │ sys-sec/op  │ sys-sec/op   vs base                │
ListModules/Empty-10   2.065m ± 1%   1.985m ± 3%   -3.86% (p=0.000 n=20)
ListModules/Cmd-10     43.48m ± 5%   33.32m ± 6%  -23.37% (p=0.000 n=20)
ListModules/K8S-10      2.106 ± 9%    1.798 ± 5%  -14.64% (p=0.000 n=20)
geomean                57.40m        49.18m       -14.33%

                     │    old.txt    │               new.txt               │
                     │  user-sec/op  │ user-sec/op  vs base                │
ListModules/Empty-10    2.459m ±  0%   2.419m ± 2%   -1.63% (p=0.000 n=20)
ListModules/Cmd-10      33.55m ±  3%   26.73m ± 8%  -20.30% (p=0.000 n=20)
ListModules/K8S-10     1106.9m ± 11%   834.4m ± 7%  -24.62% (p=0.000 n=20)
geomean                 45.03m         37.79m       -16.08%

Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Apr 30, 2024
goos: darwin
goarch: arm64
pkg: cmd/go
cpu: Apple M1 Max
                     │   old.txt   │               new.txt               │
                     │   sec/op    │   sec/op     vs base                │
ListModules/Empty-10   6.414m ± 0%   6.154m ± 3%   -4.06% (p=0.000 n=20)
ListModules/Cmd-10     823.8m ± 3%   296.6m ± 1%  -63.99% (p=0.000 n=20)
ListModules/K8S-10     37.075 ± 3%    9.003 ± 3%  -75.72% (p=0.000 n=20)
geomean                580.8m        254.2m       -56.22%

                     │   old.txt   │               new.txt               │
                     │ sys-sec/op  │ sys-sec/op   vs base                │
ListModules/Empty-10   2.065m ± 1%   1.985m ± 3%   -3.86% (p=0.000 n=20)
ListModules/Cmd-10     43.48m ± 5%   33.32m ± 6%  -23.37% (p=0.000 n=20)
ListModules/K8S-10      2.106 ± 9%    1.798 ± 5%  -14.64% (p=0.000 n=20)
geomean                57.40m        49.18m       -14.33%

                     │    old.txt    │               new.txt               │
                     │  user-sec/op  │ user-sec/op  vs base                │
ListModules/Empty-10    2.459m ±  0%   2.419m ± 2%   -1.63% (p=0.000 n=20)
ListModules/Cmd-10      33.55m ±  3%   26.73m ± 8%  -20.30% (p=0.000 n=20)
ListModules/K8S-10     1106.9m ± 11%   834.4m ± 7%  -24.62% (p=0.000 n=20)
geomean                 45.03m         37.79m       -16.08%

Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Apr 30, 2024
goos: darwin
goarch: arm64
pkg: cmd/go
cpu: Apple M1 Max
                     │   old.txt   │               new.txt               │
                     │   sec/op    │   sec/op     vs base                │
ListModules/Empty-10   6.414m ± 0%   6.154m ± 3%   -4.06% (p=0.000 n=20)
ListModules/Cmd-10     823.8m ± 3%   296.6m ± 1%  -63.99% (p=0.000 n=20)
ListModules/K8S-10     37.075 ± 3%    9.003 ± 3%  -75.72% (p=0.000 n=20)
geomean                580.8m        254.2m       -56.22%

                     │   old.txt   │               new.txt               │
                     │ sys-sec/op  │ sys-sec/op   vs base                │
ListModules/Empty-10   2.065m ± 1%   1.985m ± 3%   -3.86% (p=0.000 n=20)
ListModules/Cmd-10     43.48m ± 5%   33.32m ± 6%  -23.37% (p=0.000 n=20)
ListModules/K8S-10      2.106 ± 9%    1.798 ± 5%  -14.64% (p=0.000 n=20)
geomean                57.40m        49.18m       -14.33%

                     │    old.txt    │               new.txt               │
                     │  user-sec/op  │ user-sec/op  vs base                │
ListModules/Empty-10    2.459m ±  0%   2.419m ± 2%   -1.63% (p=0.000 n=20)
ListModules/Cmd-10      33.55m ±  3%   26.73m ± 8%  -20.30% (p=0.000 n=20)
ListModules/K8S-10     1106.9m ± 11%   834.4m ± 7%  -24.62% (p=0.000 n=20)
geomean                 45.03m         37.79m       -16.08%

Fixes golang#63136
@AdallomRoy
Copy link

@goto1134 any updates on this? it's been another month and performance is still abysmal.

@goto1134
Copy link
Contributor Author

goto1134 commented May 21, 2024

@goto1134 any updates on this? it's been another month and performance is still abysmal.

Hey, @AdallomRoy ! As you might see, I provided all the changes requested by reviewers. Could you clarify how I can help you?

goto1134 added a commit to goto1134/go that referenced this issue Aug 16, 2024
goto1134 added a commit to goto1134/go that referenced this issue Aug 16, 2024
goto1134 added a commit to goto1134/go that referenced this issue Aug 16, 2024
goos: darwin
goarch: arm64
pkg: cmd/go
cpu: Apple M1 Max
                     │   old.txt    │               new.txt               │
                     │    sec/op    │   sec/op     vs base                │
ListModules/Empty-10   7.695m ± 10%   7.562m ± 9%        ~ (p=0.678 n=20)
ListModules/Cmd-10     339.7m ±  3%   112.9m ± 2%  -66.76% (p=0.000 n=20)
ListModules/K8S-10     13.657 ± 10%    3.511 ± 3%  -74.29% (p=0.000 n=20)
geomean                329.3m         144.2m       -56.21%

                     │   old.txt    │               new.txt                │
                     │  sys-sec/op  │  sys-sec/op   vs base                │
ListModules/Empty-10   2.523m ± 16%   2.297m ± 12%        ~ (p=0.904 n=20)
ListModules/Cmd-10     70.64m ± 20%   46.33m ±  9%  -34.41% (p=0.000 n=20)
ListModules/K8S-10      2.552 ±  6%    2.541 ±  9%        ~ (p=0.883 n=20)
geomean                76.90m         64.66m        -15.91%

                     │   old.txt    │               new.txt               │
                     │ user-sec/op  │ user-sec/op  vs base                │
ListModules/Empty-10   3.068m ±  7%   2.967m ± 4%        ~ (p=1.000 n=20)
ListModules/Cmd-10     21.72m ± 12%   14.92m ± 2%  -31.31% (p=0.000 n=20)
ListModules/K8S-10     604.7m ±  9%   523.1m ± 8%  -13.49% (p=0.000 n=20)
geomean                34.28m         28.50m       -16.87%

Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Aug 17, 2024
goos: darwin
goarch: arm64
pkg: cmd/go
cpu: Apple M1 Max
                     │   old.txt   │               new.txt               │
                     │   sec/op    │   sec/op     vs base                │
ListModules/Empty-10   7.768m ± 5%   7.768m ± 6%        ~ (p=0.989 n=20)
ListModules/Cmd-10     272.3m ± 2%   137.8m ± 2%  -49.40% (p=0.000 n=20)
ListModules/K8S-10     10.741 ± 2%    2.525 ± 5%  -76.49% (p=0.000 n=20)
geomean                283.2m        139.3m       -50.82%

                     │   old.txt    │               new.txt               │
                     │  sys-sec/op  │  sys-sec/op   vs base               │
ListModules/Empty-10   2.380m ±  9%   2.443m ±  9%       ~ (p=0.314 n=20)
ListModules/Cmd-10     51.84m ± 13%   47.27m ± 14%       ~ (p=0.289 n=20)
ListModules/K8S-10      1.660 ±  8%    1.485 ± 28%       ~ (p=0.512 n=20)
geomean                58.95m         55.56m        -5.75%

                     │   old.txt    │               new.txt                │
                     │ user-sec/op  │ user-sec/op   vs base                │
ListModules/Empty-10   3.034m ±  4%   3.053m ±  3%        ~ (p=0.445 n=20)
ListModules/Cmd-10     18.01m ± 11%   15.39m ±  5%  -14.55% (p=0.000 n=20)
ListModules/K8S-10     407.6m ± 11%   209.2m ± 49%  -48.67% (p=0.000 n=20)
geomean                28.13m         21.42m        -23.86%

Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Aug 24, 2024
Benchmark from the go-list-benchmark branch shows the following result:

goos: darwin
goarch: arm64
pkg: cmd/go
cpu: Apple M1 Max
                     │   old.txt   │               new.txt               │
                     │   sec/op    │   sec/op     vs base                │
ListModules/Empty-10   7.768m ± 5%   7.768m ± 6%        ~ (p=0.989 n=20)
ListModules/Cmd-10     272.3m ± 2%   137.8m ± 2%  -49.40% (p=0.000 n=20)
ListModules/K8S-10     10.741 ± 2%    2.525 ± 5%  -76.49% (p=0.000 n=20)
geomean                283.2m        139.3m       -50.82%

                     │   old.txt    │               new.txt               │
                     │  sys-sec/op  │  sys-sec/op   vs base               │
ListModules/Empty-10   2.380m ±  9%   2.443m ±  9%       ~ (p=0.314 n=20)
ListModules/Cmd-10     51.84m ± 13%   47.27m ± 14%       ~ (p=0.289 n=20)
ListModules/K8S-10      1.660 ±  8%    1.485 ± 28%       ~ (p=0.512 n=20)
geomean                58.95m         55.56m        -5.75%

                     │   old.txt    │               new.txt                │
                     │ user-sec/op  │ user-sec/op   vs base                │
ListModules/Empty-10   3.034m ±  4%   3.053m ±  3%        ~ (p=0.445 n=20)
ListModules/Cmd-10     18.01m ± 11%   15.39m ±  5%  -14.55% (p=0.000 n=20)
ListModules/K8S-10     407.6m ± 11%   209.2m ± 49%  -48.67% (p=0.000 n=20)
geomean                28.13m         21.42m        -23.86%

Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Aug 24, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/608135 mentions this issue: cmd/go: add benchmark for go list -m

goto1134 added a commit to goto1134/go that referenced this issue Aug 24, 2024
goto1134 added a commit to goto1134/go that referenced this issue Aug 24, 2024
goto1134 added a commit to goto1134/go that referenced this issue Aug 26, 2024
Benchmark from the go-list-benchmark branch shows the following result:

goos: darwin
goarch: arm64
pkg: cmd/go
cpu: Apple M1 Max
                     │   old.txt   │               new.txt               │
                     │   sec/op    │   sec/op     vs base                │
ListModules/Empty-10   7.768m ± 5%   7.768m ± 6%        ~ (p=0.989 n=20)
ListModules/Cmd-10     272.3m ± 2%   137.8m ± 2%  -49.40% (p=0.000 n=20)
ListModules/K8S-10     10.741 ± 2%    2.525 ± 5%  -76.49% (p=0.000 n=20)
geomean                283.2m        139.3m       -50.82%

                     │   old.txt    │               new.txt               │
                     │  sys-sec/op  │  sys-sec/op   vs base               │
ListModules/Empty-10   2.380m ±  9%   2.443m ±  9%       ~ (p=0.314 n=20)
ListModules/Cmd-10     51.84m ± 13%   47.27m ± 14%       ~ (p=0.289 n=20)
ListModules/K8S-10      1.660 ±  8%    1.485 ± 28%       ~ (p=0.512 n=20)
geomean                58.95m         55.56m        -5.75%

                     │   old.txt    │               new.txt                │
                     │ user-sec/op  │ user-sec/op   vs base                │
ListModules/Empty-10   3.034m ±  4%   3.053m ±  3%        ~ (p=0.445 n=20)
ListModules/Cmd-10     18.01m ± 11%   15.39m ±  5%  -14.55% (p=0.000 n=20)
ListModules/K8S-10     407.6m ± 11%   209.2m ± 49%  -48.67% (p=0.000 n=20)
geomean                28.13m         21.42m        -23.86%

Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Aug 26, 2024
Benchmark from the go-list-benchmark branch shows the following result:

goos: darwin
goarch: arm64
pkg: cmd/go
cpu: Apple M1 Max
                     │   old.txt   │               new.txt               │
                     │   sec/op    │   sec/op     vs base                │
ListModules/Empty-10   7.768m ± 5%   7.768m ± 6%        ~ (p=0.989 n=20)
ListModules/Cmd-10     272.3m ± 2%   137.8m ± 2%  -49.40% (p=0.000 n=20)
ListModules/K8S-10     10.741 ± 2%    2.525 ± 5%  -76.49% (p=0.000 n=20)
geomean                283.2m        139.3m       -50.82%

                     │   old.txt    │               new.txt               │
                     │  sys-sec/op  │  sys-sec/op   vs base               │
ListModules/Empty-10   2.380m ±  9%   2.443m ±  9%       ~ (p=0.314 n=20)
ListModules/Cmd-10     51.84m ± 13%   47.27m ± 14%       ~ (p=0.289 n=20)
ListModules/K8S-10      1.660 ±  8%    1.485 ± 28%       ~ (p=0.512 n=20)
geomean                58.95m         55.56m        -5.75%

                     │   old.txt    │               new.txt                │
                     │ user-sec/op  │ user-sec/op   vs base                │
ListModules/Empty-10   3.034m ±  4%   3.053m ±  3%        ~ (p=0.445 n=20)
ListModules/Cmd-10     18.01m ± 11%   15.39m ±  5%  -14.55% (p=0.000 n=20)
ListModules/K8S-10     407.6m ± 11%   209.2m ± 49%  -48.67% (p=0.000 n=20)
geomean                28.13m         21.42m        -23.86%

Fixes golang#63136
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. ToolSpeed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants