golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.22k stars 17.7k forks source link

cmd/go: don't compile native code(c, c++) to objects when running go command in list and vet mode #62362

Open zhuah opened 1 year ago

zhuah commented 1 year ago

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

$ go version
go version go1.21.0 darwin/amd64

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

What did you do?

Gopls underlying calls go list with -compiled=true flag to get CompiledGoFiles field, which contains original go source file in a package, and also cgo-generated go source files. That's enough for gopls to working, but the go list command will also compile c/c++ files to object files, this is totally redundant for gopls.

My work heavily relies on cgo, when loading workspace or doing some changes that affects the cgo package in vscode, the edtor becomes very slow and cpu usage goes 100% because of the compilation of c/c++ files, especially when the codebase is very large, as i described in #50151 years ago.

I have do some experiments in cmd/go source code by skip the compilation of c/c++ code by wrapping these code in the if statement:

if !b.IsCmdList {
    // .......
}

https://github.com/golang/go/blob/1f8f2ab9661c78876d8a8cb0ccc4625728842b26/src/cmd/go/internal/work/exec.go#L3319-L3401

Gopls works fine after recompiled the go command, it seems nothing broken. I'm not sure whether this change will break go build cache or not.

There is also another solution, gopls could call go list with -compiled=false and generate CompiledGoFiles by calling cgo command by itself, but it requires gopls to parse the go source files to get C include paths and pass it to cgo command.

I think it worth a change in go toolchain for go list and go vet, it improves dev experience very much, i don't see any necessaries to compile c/c++ file to objects in these two cases, not only for gopls, but also other tools that uses go list, of course we could introduce some new env/flag to keep backward compatibility if need.

bcmills commented 1 year ago

I believe that the cgo tool itself needs the compiled object files in order to generate the Go source files for the wrapper.

(@ianlancetaylor can correct me if I'm wrong..?)

zhuah commented 1 year ago

@bcmills As far as know, cgo doesn't process native source files, it requires only header files for type/function declaration, not implementation in source file.

This link also shows that the go tool does the compilation: https://github.com/golang/go/blob/1f8f2ab9661c78876d8a8cb0ccc4625728842b26/src/cmd/go/internal/work/exec.go#L3319-L3401

ianlancetaylor commented 1 year ago

cmd/cgo does compile some C files to object files, which it uses while generating Go files. cmd/cgo also generates C files that it does not compile, but that cmd/go compiles. Those C files compiled by cmd/go are only needed if you are going to build a Go package or executable. It seems fine to omit those compilations as long as we don't try to build a binary or cache a .a file.

zhuah commented 1 year ago

@ianlancetaylor Agreed, my package contains a big c/cpp lib, it's a huge pain to work at this package in vscode+gopls currently, i need to pkill clang many many times to stop those redundant compilations because they take all my cpus, this issue has troubled me for a long time since #50151.

dzbarsky commented 1 year ago

I'm pretty sure this is a longstanding issue, see https://github.com/golang/go/issues/34229