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: commit a6ff433d6a927e8ad8eaa6828127233296d12ce5 reduces concurrency in compiler #48490

Closed
dominikh opened this issue Sep 20, 2021 · 5 comments
Closed

cmd/go: commit a6ff433d6a927e8ad8eaa6828127233296d12ce5 reduces concurrency in compiler #48490

dominikh opened this issue Sep 20, 2021 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker ToolSpeed
Milestone
Go1.18

Comments

@dominikh
Copy link
Member

Commit a6ff433 changes the way flags get passed to the compiler. As an unintended side-effect, the -c (for concurrency) flag seems to no longer get passed from go build to go tool compile. This can be seen in the output of go build -x, and it can be felt when running make.bash, which went from ~600% CPU utilization on the commit's parent to ~390% CPU utilization.

/cc @cuonglm @bcmills @mvdan
@bcmills bcmills added this to the Go1.18 milestone Sep 20, 2021
@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker ToolSpeed labels Sep 20, 2021
@mvdan
Copy link
Member

mvdan commented Sep 20, 2021

This probably means we should have an integreation test to ensure that the -c flag gets plumbed through to the compiler properly :)

@cuonglm
Copy link
Member

cuonglm commented Sep 20, 2021

So it seems the flags order change affect gcBackendConcurrency:

go/src/cmd/go/internal/work/gc.go

Line 185 in 79159f2

if c := gcBackendConcurrency(gcflags); c > 1 {

@cuonglm cuonglm self-assigned this Sep 20, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/351049 mentions this issue: cmd/go: fix go command fails to perform concurrent compilation

@bcmills bcmills mentioned this issue Sep 20, 2021
Open
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/351849 mentions this issue: cmd/go: move gc concurrency level computation near gcflags

gopherbot pushed a commit that referenced this issue Sep 24, 2021
@cuonglm
cmd/go: move gc concurrency level computation near gcflags
9ce601d 
So after constructing "args" variable, "gcflags" is not used anywhere.
It makes the code easier to maintain, and prevent subtle bug like #48490.

Change-Id: I41653536480880a8a6f9fbf6cfa8a461b6fb3208
Reviewed-on: https://go-review.googlesource.com/c/go/+/351849
Reviewed-by: Bryan C. Mills <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Cuong Manh Le <[email protected]>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/353949 mentions this issue: cmd/go: enable build concurrent backend on darwin/arm64

gopherbot pushed a commit that referenced this issue Oct 5, 2021
@cuonglm
cmd/go: enable test build concurrent backend on darwin/arm64
123393a 
After CL 353871, darwin/arm64 now do concurrent build, so enable the
test for it.

Updates #48490

Change-Id: I29336f6fc7d7d2f463d8ad2a620534bd7f048d2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/353949
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
@rsc rsc unassigned cuonglm Jun 23, 2022
@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. release-blocker ToolSpeed
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
5 participants
@dominikh @mvdan @cuonglm @bcmills @gopherbot