From: Cherry Zhang Date: Wed, 1 May 2019 00:47:48 +0000 (-0400) Subject: cmd/internal/obj: write package path at compile time if possible X-Git-Tag: go1.13beta1~407 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=cc5eaf9346d49c7e51a18ecc4d80c4f0456b4742;p=gostls13.git cmd/internal/obj: write package path at compile time if possible Currently, when the compiler emits a symbol name in the object file, it uses "". for the package path of the package being compiled. This is then expanded in the linker to the actual package path. With CL 173938, it does not need an allocation if the symbol name does not need expansion. In many cases, the compiler actually knows the package path (through the -p flag), so we could just write it out in compile time, without fixing it up in the linker. This reduces allocations in the linker. In case that the package path is not known (compiler's -p flag is missing, or the object file is generated by the assembler), the linker still does the expansion. This reduces ~100MB allocations (~10% inuse_space) in linking k8s.io/kubernetes/cmd/kube-apiserver on Linux/AMD64. Also makes the linker a little faster: linking cmd/go on Linux/AMD64: Real 1.13 ± 1% 1.11 ± 1% -2.13% (p=0.000 n=10+10) User 1.17 ± 3% 1.14 ± 5% -3.14% (p=0.003 n=10+10) Sys 0.34 ±15% 0.34 ±15% ~ (p=0.986 n=10+10) The caveat is that the object files get slightly bigger. On Linux/AMD64, runtime.a gets 2.1% bigger, cmd/compile/internal/ssa (which has a longer import path) gets 2.8% bigger. This reveals that when building an unnamed plugin (e.g. go build -buildmode=plugin x.go), the go command passes different package paths to the compiler and to the linker. Before this CL there seems nothing obviously broken, but given that the compiler already emits the package's import path in various places (e.g. debug info), I guess it is possible that this leads to some unexpected behavior. Now that the compiler writes the package path in more places, this disagreement actually leads to unresolved symbols. Adjust the go command to use the same package path for both compiling and linking. Change-Id: I19f08981f51db577871c906e08d9e0fd588a2dd8 Reviewed-on: https://go-review.googlesource.com/c/go/+/174657 Reviewed-by: Austin Clements --- diff --git a/src/cmd/asm/main.go b/src/cmd/asm/main.go index 447d1afde3..fc6acc74c0 100644 --- a/src/cmd/asm/main.go +++ b/src/cmd/asm/main.go @@ -83,7 +83,7 @@ func main() { } } if ok && !*flags.SymABIs { - obj.WriteObjFile(ctxt, buf) + obj.WriteObjFile(ctxt, buf, "") } if !ok || diag { if failedFile != "" { diff --git a/src/cmd/compile/internal/gc/obj.go b/src/cmd/compile/internal/gc/obj.go index d0ba6ffb75..0a421729f4 100644 --- a/src/cmd/compile/internal/gc/obj.go +++ b/src/cmd/compile/internal/gc/obj.go @@ -164,7 +164,7 @@ func dumpLinkerObj(bout *bio.Writer) { addGCLocals() - obj.WriteObjFile(Ctxt, bout.Writer) + obj.WriteObjFile(Ctxt, bout.Writer, myimportpath) } func addptabs() { diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go index 756a89f3ab..e7d5fae4de 100644 --- a/src/cmd/go/internal/work/gc.go +++ b/src/cmd/go/internal/work/gc.go @@ -510,7 +510,21 @@ func pluginPath(a *Action) string { return p.ImportPath } h := sha1.New() - fmt.Fprintf(h, "build ID: %s\n", a.buildID) + buildID := a.buildID + if a.Mode == "link" { + // For linking, use the main package's build ID instead of + // the binary's build ID, so it is the same hash used in + // compiling and linking. + // When compiling, we use actionID/actionID (instead of + // actionID/contentID) as a temporary build ID to compute + // the hash. Do the same here. (See buildid.go:useCache) + // The build ID matters because it affects the overall hash + // in the plugin's pseudo-import path returned below. + // We need to use the same import path when compiling and linking. + id := strings.Split(buildID, buildIDSeparator) + buildID = id[1] + buildIDSeparator + id[1] + } + fmt.Fprintf(h, "build ID: %s\n", buildID) for _, file := range str.StringList(p.GoFiles, p.CgoFiles, p.SFiles) { data, err := ioutil.ReadFile(filepath.Join(p.Dir, file)) if err != nil { diff --git a/src/cmd/internal/obj/objfile.go b/src/cmd/internal/obj/objfile.go index 6921df3675..62d41e11eb 100644 --- a/src/cmd/internal/obj/objfile.go +++ b/src/cmd/internal/obj/objfile.go @@ -15,6 +15,7 @@ import ( "log" "path/filepath" "sort" + "strings" "sync" ) @@ -33,6 +34,8 @@ type objWriter struct { nAutom int nFuncdata int nFile int + + pkgpath string // the package import path (escaped), "" if unknown } func (w *objWriter) addLengths(s *LSym) { @@ -71,15 +74,16 @@ func (w *objWriter) writeLengths() { w.writeInt(int64(w.nFile)) } -func newObjWriter(ctxt *Link, b *bufio.Writer) *objWriter { +func newObjWriter(ctxt *Link, b *bufio.Writer, pkgpath string) *objWriter { return &objWriter{ - ctxt: ctxt, - wr: b, + ctxt: ctxt, + wr: b, + pkgpath: objabi.PathToPrefix(pkgpath), } } -func WriteObjFile(ctxt *Link, b *bufio.Writer) { - w := newObjWriter(ctxt, b) +func WriteObjFile(ctxt *Link, b *bufio.Writer, pkgpath string) { + w := newObjWriter(ctxt, b, pkgpath) // Magic header w.wr.WriteString("\x00go112ld") @@ -170,6 +174,10 @@ func (w *objWriter) writeRef(s *LSym, isPath bool) { w.wr.WriteByte(symPrefix) if isPath { w.writeString(filepath.ToSlash(s.Name)) + } else if w.pkgpath != "" { + // w.pkgpath is already escaped. + n := strings.Replace(s.Name, "\"\".", w.pkgpath+".", -1) + w.writeString(n) } else { w.writeString(s.Name) } diff --git a/src/cmd/nm/nm_test.go b/src/cmd/nm/nm_test.go index e47d57d9cb..dcd9f36005 100644 --- a/src/cmd/nm/nm_test.go +++ b/src/cmd/nm/nm_test.go @@ -266,12 +266,12 @@ func testGoLib(t *testing.T, iscgo bool) { Found bool } var syms = []symType{ - {"B", "%22%22.Testdata", false, false}, - {"T", "%22%22.Testfunc", false, false}, + {"B", "mylib.Testdata", false, false}, + {"T", "mylib.Testfunc", false, false}, } if iscgo { - syms = append(syms, symType{"B", "%22%22.TestCgodata", false, false}) - syms = append(syms, symType{"T", "%22%22.TestCgofunc", false, false}) + syms = append(syms, symType{"B", "mylib.TestCgodata", false, false}) + syms = append(syms, symType{"T", "mylib.TestCgofunc", false, false}) if runtime.GOOS == "darwin" || (runtime.GOOS == "windows" && runtime.GOARCH == "386") { syms = append(syms, symType{"D", "_cgodata", true, false}) syms = append(syms, symType{"T", "_cgofunc", true, false})