From 586d0755e2cd6a51f0837c6b7748e93d58b966f1 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Mon, 18 May 2020 18:20:18 -0400 Subject: [PATCH] [dev.link] cmd/link: only do name expansion when needed MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Most Go objects are compiled with known package path, so the symbol name is already fully expanded. Nevertheless, currently in the linker strings.Replace is called unconditionally, and most of the time it doesn't do anything. This CL records a per-object flag in the object file, and do the name expansion only when the name is not expanded at compile time. This gives small speedups for the linker. Linking cmd/compile: name old time/op new time/op delta Loadlib 35.1ms ± 2% 32.8ms ± 4% -6.43% (p=0.008 n=5+5) Symtab 15.8ms ± 2% 14.0ms ± 8% -11.45% (p=0.008 n=5+5) TotalTime 399ms ± 1% 385ms ± 2% -3.63% (p=0.008 n=5+5) Change-Id: I735084971a051cd9be4284ad294c284cd5b545f4 Reviewed-on: https://go-review.googlesource.com/c/go/+/234490 Run-TryBot: Cherry Zhang TryBot-Result: Gobot Gobot Reviewed-by: Jeremy Faller Reviewed-by: Than McIntosh --- src/cmd/internal/goobj2/objfile.go | 6 +++++- src/cmd/internal/obj/objfile2.go | 3 +++ src/cmd/link/internal/loader/loader.go | 25 ++++++++++++++++++++----- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/cmd/internal/goobj2/objfile.go b/src/cmd/internal/goobj2/objfile.go index fde482e079..dc5174f85e 100644 --- a/src/cmd/internal/goobj2/objfile.go +++ b/src/cmd/internal/goobj2/objfile.go @@ -228,7 +228,8 @@ const SymSize = stringRefSize + 2 + 1 + 1 + 4 + 4 const SymABIstatic = ^uint16(0) const ( - ObjFlagShared = 1 << iota + ObjFlagShared = 1 << iota // this object is built with -shared + ObjFlagNeedNameExpansion // the linker needs to expand `"".` to package path in symbol names ) const ( @@ -675,3 +676,6 @@ func (r *Reader) ReadOnly() bool { func (r *Reader) Flags() uint32 { return r.h.Flags } + +func (r *Reader) Shared() bool { return r.Flags()&ObjFlagShared != 0 } +func (r *Reader) NeedNameExpansion() bool { return r.Flags()&ObjFlagNeedNameExpansion != 0 } diff --git a/src/cmd/internal/obj/objfile2.go b/src/cmd/internal/obj/objfile2.go index 0b3b2be41b..c28ae569d3 100644 --- a/src/cmd/internal/obj/objfile2.go +++ b/src/cmd/internal/obj/objfile2.go @@ -38,6 +38,9 @@ func WriteObjFile(ctxt *Link, b *bio.Writer, pkgpath string) { if ctxt.Flag_shared { flags |= goobj2.ObjFlagShared } + if pkgpath == "" { + flags |= goobj2.ObjFlagNeedNameExpansion + } h := goobj2.Header{ Magic: goobj2.Magic, Fingerprint: ctxt.Fingerprint, diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go index 5696c51100..26b17ce007 100644 --- a/src/cmd/link/internal/loader/loader.go +++ b/src/cmd/link/internal/loader/loader.go @@ -661,7 +661,11 @@ func (l *Loader) SymName(i Sym) string { return pp.name } r, li := l.toLocal(i) - return strings.Replace(r.Sym(li).Name(r.Reader), "\"\".", r.pkgprefix, -1) + name := r.Sym(li).Name(r.Reader) + if !r.NeedNameExpansion() { + return name + } + return strings.Replace(name, "\"\".", r.pkgprefix, -1) } // Returns the version of the i-th symbol. @@ -843,7 +847,7 @@ func (l *Loader) AttrShared(i Sym) bool { // might make more sense to copy the flag value out of the // object into a larger bitmap during preload. r, _ := l.toLocal(i) - return (r.Flags() & goobj2.ObjFlagShared) != 0 + return r.Shared() } return l.attrShared.Has(l.extIndex(i)) } @@ -1948,9 +1952,13 @@ func (l *Loader) preloadSyms(r *oReader, kind int) { panic("preloadSyms: bad kind") } l.growAttrBitmaps(len(l.objSyms) + int(end-start)) + needNameExpansion := r.NeedNameExpansion() for i := start; i < end; i++ { osym := r.Sym(i) - name := strings.Replace(osym.Name(r.Reader), "\"\".", r.pkgprefix, -1) + name := osym.Name(r.Reader) + if needNameExpansion { + name = strings.Replace(name, "\"\".", r.pkgprefix, -1) + } v := abiToVer(osym.ABI(), r.version) dupok := osym.Dupok() gi, added := l.AddSym(name, v, r, i, kind, dupok, sym.AbiSymKindToSymKind[objabi.SymKind(osym.Type())]) @@ -1998,9 +2006,13 @@ func (l *Loader) LoadNonpkgSyms(arch *sys.Arch) { func loadObjRefs(l *Loader, r *oReader, arch *sys.Arch) { ndef := uint32(r.NSym() + r.NNonpkgdef()) + needNameExpansion := r.NeedNameExpansion() for i, n := uint32(0), uint32(r.NNonpkgref()); i < n; i++ { osym := r.Sym(ndef + i) - name := strings.Replace(osym.Name(r.Reader), "\"\".", r.pkgprefix, -1) + name := osym.Name(r.Reader) + if needNameExpansion { + name = strings.Replace(name, "\"\".", r.pkgprefix, -1) + } v := abiToVer(osym.ABI(), r.version) r.syms[ndef+i] = l.LookupOrCreateSym(name, v) gi := r.syms[ndef+i] @@ -2109,7 +2121,10 @@ func (l *Loader) cloneToExternal(symIdx Sym) { // Read the particulars from object. r, li := l.toLocal(symIdx) osym := r.Sym(li) - sname := strings.Replace(osym.Name(r.Reader), "\"\".", r.pkgprefix, -1) + sname := osym.Name(r.Reader) + if r.NeedNameExpansion() { + sname = strings.Replace(sname, "\"\".", r.pkgprefix, -1) + } sver := abiToVer(osym.ABI(), r.version) skind := sym.AbiSymKindToSymKind[objabi.SymKind(osym.Type())] -- 2.48.1