]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/internal/obj: write package path at compile time if possible
authorCherry Zhang <cherryyz@google.com>
Wed, 1 May 2019 00:47:48 +0000 (20:47 -0400)
committerCherry Zhang <cherryyz@google.com>
Mon, 6 May 2019 18:17:03 +0000 (18:17 +0000)
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 <austin@google.com>
src/cmd/asm/main.go
src/cmd/compile/internal/gc/obj.go
src/cmd/go/internal/work/gc.go
src/cmd/internal/obj/objfile.go
src/cmd/nm/nm_test.go

index 447d1afde3a08507a3ba13d6b97244cfd16e30fa..fc6acc74c05180c2f115e17ec14345bad6f7bae0 100644 (file)
@@ -83,7 +83,7 @@ func main() {
                }
        }
        if ok && !*flags.SymABIs {
-               obj.WriteObjFile(ctxt, buf)
+               obj.WriteObjFile(ctxt, buf, "")
        }
        if !ok || diag {
                if failedFile != "" {
index d0ba6ffb75ef2f64aadd6b0ce8ba6613e6fbf12e..0a421729f44e1db38d23837888f5475c091c73f0 100644 (file)
@@ -164,7 +164,7 @@ func dumpLinkerObj(bout *bio.Writer) {
 
        addGCLocals()
 
-       obj.WriteObjFile(Ctxt, bout.Writer)
+       obj.WriteObjFile(Ctxt, bout.Writer, myimportpath)
 }
 
 func addptabs() {
index 756a89f3abad4ca773c99ed29509672e71efbfc8..e7d5fae4de7742aa86832d9e9d102c561e686902 100644 (file)
@@ -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 {
index 6921df3675e76575141fdab5da396dcbb737c119..62d41e11ebc31194d400d8af65267b41b44cc675 100644 (file)
@@ -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)
        }
index e47d57d9cb3161f2d57dee97827771ad9c8b639e..dcd9f360050ceb06bac399955fff4e6677c683a3 100644 (file)
@@ -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})