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.

Sign up for GitHub

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 does not scale with GOMAXPROCS #63136

Open
goto1134 opened this issue Sep 21, 2023 · 5 comments · May be fixed by #63137
Open

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

goto1134 opened this issue Sep 21, 2023 · 5 comments · May be fixed by #63137
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
Backlog

Comments

@goto1134
Copy link

goto1134 commented Sep 21, 2023 •
edited

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
goto1134 added a commit to goto1134/go that referenced this issue Sep 21, 2023
@goto1134
cmd/go: scale go list with GOMAXPROCS
8289881 
Fixes golang#63136
@goto1134 goto1134 linked a pull request Sep 21, 2023 that will close this issue
Open
@gopherbot
Copy link

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
@goto1134
cmd/go: removes a mutex and preserves module order
dfaef9b 
Fixes golang#63136
@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
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
@goto1134
cmd/go: add benchmark for go list optimization
f46057d 
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
cmd/go: scale go list with GOMAXPROCS
ec6602f 
Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Jan 9, 2024
@goto1134
cmd/go: removes a mutex and preserves module order
d8d4832 
Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Jan 9, 2024
@goto1134
cmd/go: adds a benchmark for go list optimization
0ff5d17 
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
cmd/go: add benchmark for go list optimization
b329f84 
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
@goto1134
cmd/go: add benchmark for go list optimization
4380bf2 
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
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
cmd/go: scale go list with GOMAXPROCS
87c3ea2 
Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Apr 30, 2024
@goto1134
cmd/go: removes a mutex and preserves module order
563f626 
Fixes golang#63136
goto1134 added a commit to goto1134/go that referenced this issue Apr 30, 2024
@goto1134
cmd/go: add benchmark for go list optimization
06fccc5 
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
@goto1134
cmd/go: add benchmark for go list optimization
3b6312a 
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
@goto1134
cmd/go: add benchmark for go list optimization
cb81ce5 
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
@goto1134
cmd/go: add benchmark for go list optimization
e193250 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

cmd/go: scale go list with GOMAXPROCS goto1134/go
5 participants
@dany74q @AdallomRoy @bcmills @gopherbot @goto1134