From 4927b9a9ffeb5e33f6586b0f9000387d8ea20730 Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Thu, 30 Mar 2017 16:23:01 -0700 Subject: [PATCH] cmd/compile: remove makefuncdatasym_nsym global MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This causes a minor reduction in allocations, because the old funcdatasym names were being interned unnecessarily. Updates #15756 name old alloc/op new alloc/op delta Template 39.9MB ± 0% 39.9MB ± 0% ~ (p=0.280 n=10+10) Unicode 29.9MB ± 0% 29.8MB ± 0% -0.26% (p=0.000 n=10+10) GoTypes 113MB ± 0% 113MB ± 0% -0.12% (p=0.000 n=10+10) SSA 855MB ± 0% 855MB ± 0% -0.03% (p=0.001 n=10+10) Flate 25.4MB ± 0% 25.3MB ± 0% -0.30% (p=0.000 n=10+10) GoParser 31.9MB ± 0% 31.8MB ± 0% ~ (p=0.065 n=10+9) Reflect 78.4MB ± 0% 78.2MB ± 0% -0.15% (p=0.000 n=9+10) Tar 26.7MB ± 0% 26.7MB ± 0% -0.17% (p=0.000 n=9+10) XML 42.3MB ± 0% 42.4MB ± 0% +0.07% (p=0.011 n=10+10) name old allocs/op new allocs/op delta Template 390k ± 0% 390k ± 0% ~ (p=0.905 n=9+10) Unicode 319k ± 1% 319k ± 1% ~ (p=0.724 n=10+10) GoTypes 1.14M ± 0% 1.14M ± 0% ~ (p=0.393 n=10+10) SSA 7.60M ± 0% 7.60M ± 0% ~ (p=0.604 n=9+10) Flate 235k ± 1% 234k ± 1% ~ (p=0.105 n=10+10) GoParser 317k ± 0% 316k ± 1% ~ (p=0.280 n=10+10) Reflect 979k ± 0% 979k ± 0% ~ (p=0.315 n=10+10) Tar 251k ± 0% 251k ± 1% ~ (p=0.762 n=8+10) XML 393k ± 0% 394k ± 1% ~ (p=0.095 n=9+10) name old text-bytes new text-bytes delta HelloSize 684k ± 0% 684k ± 0% ~ (all equal) name old data-bytes new data-bytes delta HelloSize 138k ± 0% 138k ± 0% ~ (all equal) name old exe-bytes new exe-bytes delta HelloSize 1.03M ± 0% 1.03M ± 0% ~ (all equal) Change-Id: Idba33da4e89c325984ac46e4852cf12e4a7fd1a9 Reviewed-on: https://go-review.googlesource.com/39032 Run-TryBot: Josh Bleecher Snyder Reviewed-by: Robert Griesemer --- src/cmd/compile/fmt_test.go | 1 + src/cmd/compile/internal/gc/pgen.go | 28 ++++++++++++++++++++++++---- src/cmd/compile/internal/gc/ssa.go | 4 ++-- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/cmd/compile/fmt_test.go b/src/cmd/compile/fmt_test.go index 865761a23da..1dc4434e25b 100644 --- a/src/cmd/compile/fmt_test.go +++ b/src/cmd/compile/fmt_test.go @@ -654,6 +654,7 @@ var knownFormats = map[string]string{ "cmd/compile/internal/syntax.token %s": "", "cmd/internal/src.Pos %s": "", "cmd/internal/src.Pos %v": "", + "cmd/internal/src.XPos %v": "", "error %v": "", "float64 %.2f": "", "float64 %.3f": "", diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go index fdebae763a1..71d9b8f9e36 100644 --- a/src/cmd/compile/internal/gc/pgen.go +++ b/src/cmd/compile/internal/gc/pgen.go @@ -16,11 +16,31 @@ import ( // "Portable" code generation. -var makefuncdatasym_nsym int +func makefuncdatasym(pp *Progs, nameprefix string, funcdatakind int64, curfn *Node) *Sym { + // This symbol requires a unique, reproducible name; + // unique to avoid duplicate symbols, + // and reproducible for reproducible builds and toolstash. + // The function name will usually suffice. + suffix := curfn.Func.Nname.Sym.Name + if suffix == "_" { + // It is possible to have multiple functions called _, + // so in this rare case, use instead the function's position. + // This formatted string will be meaningless gibberish, but that's ok. + // It will be unique and reproducible, and it is rare anyway. + // Note that we can't just always use the position; + // it is possible to have multiple autogenerated functions at the same position. + // Fortunately, no autogenerated functions are called _. + if curfn.Pos == autogeneratedPos { + Fatalf("autogenerated func _") + } + suffix = fmt.Sprintf("%v", curfn.Pos) + } + // Add in the package path as well. + // When generating wrappers, we can end up compiling a function belonging + // to other packages, which might have a name that collides with one in our package. + symname := nameprefix + curfn.Func.Nname.Sym.Pkg.Path + "." + suffix -func makefuncdatasym(pp *Progs, nameprefix string, funcdatakind int64) *Sym { - sym := lookupN(nameprefix, makefuncdatasym_nsym) - makefuncdatasym_nsym++ + sym := lookup(symname) p := pp.Prog(obj.AFUNCDATA) Addrconst(&p.From, funcdatakind) p.To.Type = obj.TYPE_MEM diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go index 794b8017f1f..3a49a775b60 100644 --- a/src/cmd/compile/internal/gc/ssa.go +++ b/src/cmd/compile/internal/gc/ssa.go @@ -4274,8 +4274,8 @@ func genssa(f *ssa.Func, pp *Progs) { e := f.Frontend().(*ssafn) // Generate GC bitmaps. - gcargs := makefuncdatasym(pp, "gcargs·", obj.FUNCDATA_ArgsPointerMaps) - gclocals := makefuncdatasym(pp, "gclocals·", obj.FUNCDATA_LocalsPointerMaps) + gcargs := makefuncdatasym(pp, "gcargs·", obj.FUNCDATA_ArgsPointerMaps, e.curfn) + gclocals := makefuncdatasym(pp, "gclocals·", obj.FUNCDATA_LocalsPointerMaps, e.curfn) s.stackMapIndex = liveness(e, f, gcargs, gclocals) // Remember where each block starts.