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: don't compile native code(c, c++) to objects when running go command in list and vet mode #62362

Open
zhuah opened this issue Aug 30, 2023 · 5 comments
Labels
GoCommand cmd/go gopls Issues related to the Go language server, gopls. ToolSpeed

Comments

@zhuah
Copy link

zhuah commented Aug 30, 2023

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 {
    // .......
}

// Use sequential object file names to keep them distinct
// and short enough to fit in the .a header file name slots.
// We no longer collect them all into _all.o, and we'd like
// tools to see both the .o suffix and unique names, so
// we need to make them short enough not to be truncated
// in the final archive.
oseq := 0
nextOfile := func() string {
oseq++
return objdir + fmt.Sprintf("_x%03d.o", oseq)
}
// gcc
cflags := str.StringList(cgoCPPFLAGS, cgoCFLAGS)
for _, cfile := range cfiles {
ofile := nextOfile()
if err := b.gcc(a, p, a.Objdir, ofile, cflags, objdir+cfile); err != nil {
return nil, nil, err
}
outObj = append(outObj, ofile)
}
for _, file := range gccfiles {
ofile := nextOfile()
if err := b.gcc(a, p, a.Objdir, ofile, cflags, file); err != nil {
return nil, nil, err
}
outObj = append(outObj, ofile)
}
cxxflags := str.StringList(cgoCPPFLAGS, cgoCXXFLAGS)
for _, file := range gxxfiles {
ofile := nextOfile()
if err := b.gxx(a, p, a.Objdir, ofile, cxxflags, file); err != nil {
return nil, nil, err
}
outObj = append(outObj, ofile)
}
for _, file := range mfiles {
ofile := nextOfile()
if err := b.gcc(a, p, a.Objdir, ofile, cflags, file); err != nil {
return nil, nil, err
}
outObj = append(outObj, ofile)
}
fflags := str.StringList(cgoCPPFLAGS, cgoFFLAGS)
for _, file := range ffiles {
ofile := nextOfile()
if err := b.gfortran(a, p, a.Objdir, ofile, fflags, file); err != nil {
return nil, nil, err
}
outObj = append(outObj, ofile)
}
switch cfg.BuildToolchainName {
case "gc":
importGo := objdir + "_cgo_import.go"
dynOutGo, dynOutObj, err := b.dynimport(a, p, objdir, importGo, cgoExe, cflags, cgoLDFLAGS, outObj)
if err != nil {
return nil, nil, err
}
if dynOutGo != "" {
outGo = append(outGo, dynOutGo)
}
if dynOutObj != "" {
outObj = append(outObj, dynOutObj)
}
case "gccgo":
defunC := objdir + "_cgo_defun.c"
defunObj := objdir + "_cgo_defun.o"
if err := BuildToolchain.cc(b, a, defunObj, defunC); err != nil {
return nil, nil, err
}
outObj = append(outObj, defunObj)
default:
noCompiler()
}

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 bcmills added ToolSpeed GoCommand cmd/go gopls Issues related to the Go language server, gopls. labels Aug 30, 2023
@bcmills
Copy link
Contributor

bcmills commented Aug 30, 2023

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
Copy link
Author

zhuah commented Aug 31, 2023

@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:

// Use sequential object file names to keep them distinct
// and short enough to fit in the .a header file name slots.
// We no longer collect them all into _all.o, and we'd like
// tools to see both the .o suffix and unique names, so
// we need to make them short enough not to be truncated
// in the final archive.
oseq := 0
nextOfile := func() string {
oseq++
return objdir + fmt.Sprintf("_x%03d.o", oseq)
}
// gcc
cflags := str.StringList(cgoCPPFLAGS, cgoCFLAGS)
for _, cfile := range cfiles {
ofile := nextOfile()
if err := b.gcc(a, p, a.Objdir, ofile, cflags, objdir+cfile); err != nil {
return nil, nil, err
}
outObj = append(outObj, ofile)
}
for _, file := range gccfiles {
ofile := nextOfile()
if err := b.gcc(a, p, a.Objdir, ofile, cflags, file); err != nil {
return nil, nil, err
}
outObj = append(outObj, ofile)
}
cxxflags := str.StringList(cgoCPPFLAGS, cgoCXXFLAGS)
for _, file := range gxxfiles {
ofile := nextOfile()
if err := b.gxx(a, p, a.Objdir, ofile, cxxflags, file); err != nil {
return nil, nil, err
}
outObj = append(outObj, ofile)
}
for _, file := range mfiles {
ofile := nextOfile()
if err := b.gcc(a, p, a.Objdir, ofile, cflags, file); err != nil {
return nil, nil, err
}
outObj = append(outObj, ofile)
}
fflags := str.StringList(cgoCPPFLAGS, cgoFFLAGS)
for _, file := range ffiles {
ofile := nextOfile()
if err := b.gfortran(a, p, a.Objdir, ofile, fflags, file); err != nil {
return nil, nil, err
}
outObj = append(outObj, ofile)
}
switch cfg.BuildToolchainName {
case "gc":
importGo := objdir + "_cgo_import.go"
dynOutGo, dynOutObj, err := b.dynimport(a, p, objdir, importGo, cgoExe, cflags, cgoLDFLAGS, outObj)
if err != nil {
return nil, nil, err
}
if dynOutGo != "" {
outGo = append(outGo, dynOutGo)
}
if dynOutObj != "" {
outObj = append(outObj, dynOutObj)
}
case "gccgo":
defunC := objdir + "_cgo_defun.c"
defunObj := objdir + "_cgo_defun.o"
if err := BuildToolchain.cc(b, a, defunObj, defunC); err != nil {
return nil, nil, err
}
outObj = append(outObj, defunObj)
default:
noCompiler()
}

@ianlancetaylor
Copy link
Contributor

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
Copy link
Author

zhuah commented Aug 31, 2023

@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
Copy link

dzbarsky commented Sep 1, 2023

I'm pretty sure this is a longstanding issue, see #34229

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go gopls Issues related to the Go language server, gopls. ToolSpeed
Projects
None yet
Development

No branches or pull requests

4 participants