From 6e97c71cb7ef2fe139abb9cd07a1aceec16711f4 Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Mon, 17 Apr 2017 13:52:01 -0700 Subject: [PATCH] cmd/internal/obj: split Link.hash into version 0 and 1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Though LSym.Version is an int, it can only have the value 0 or 1. Using that, split Link.hash into two maps, one for version 0 (which is far more common) and one for version 1. This lets use just the name for lookups, which is both faster and more compact. This matters because Link.hash map lookups are frequent, and will be contended once the backend is concurrent. name old time/op new time/op delta Template 194ms ± 3% 192ms ± 5% -1.46% (p=0.000 n=47+49) Unicode 84.5ms ± 3% 83.8ms ± 3% -0.81% (p=0.011 n=50+49) GoTypes 543ms ± 2% 545ms ± 4% ~ (p=0.566 n=46+49) Compiler 2.48s ± 2% 2.48s ± 3% ~ (p=0.706 n=47+50) SSA 5.94s ± 3% 5.98s ± 2% +0.55% (p=0.040 n=49+50) Flate 119ms ± 6% 119ms ± 4% ~ (p=0.681 n=48+47) GoParser 145ms ± 4% 145ms ± 3% ~ (p=0.662 n=47+49) Reflect 348ms ± 3% 344ms ± 3% -1.17% (p=0.000 n=47+47) Tar 105ms ± 4% 104ms ± 3% ~ (p=0.155 n=50+47) XML 197ms ± 2% 197ms ± 3% ~ (p=0.666 n=49+49) [Geo mean] 332ms 331ms -0.37% name old user-time/op new user-time/op delta Template 230ms ±10% 226ms ±10% -1.85% (p=0.041 n=50+50) Unicode 104ms ± 6% 103ms ± 5% ~ (p=0.076 n=49+49) GoTypes 707ms ± 4% 705ms ± 5% ~ (p=0.521 n=50+50) Compiler 3.30s ± 3% 3.33s ± 4% +0.76% (p=0.003 n=50+49) SSA 8.17s ± 4% 8.23s ± 3% +0.66% (p=0.030 n=50+49) Flate 139ms ± 6% 138ms ± 8% ~ (p=0.184 n=49+48) GoParser 174ms ± 5% 172ms ± 6% ~ (p=0.107 n=48+49) Reflect 431ms ± 8% 420ms ± 5% -2.57% (p=0.000 n=50+46) Tar 119ms ± 6% 118ms ± 7% -0.95% (p=0.033 n=50+49) XML 236ms ± 4% 236ms ± 4% ~ (p=0.935 n=50+48) [Geo mean] 410ms 407ms -0.67% name old alloc/op new alloc/op delta Template 38.7MB ± 0% 38.6MB ± 0% -0.29% (p=0.008 n=5+5) Unicode 29.8MB ± 0% 29.7MB ± 0% -0.24% (p=0.008 n=5+5) GoTypes 113MB ± 0% 113MB ± 0% -0.29% (p=0.008 n=5+5) Compiler 462MB ± 0% 462MB ± 0% -0.12% (p=0.008 n=5+5) SSA 1.27GB ± 0% 1.27GB ± 0% -0.05% (p=0.008 n=5+5) Flate 25.2MB ± 0% 25.1MB ± 0% -0.37% (p=0.008 n=5+5) GoParser 31.7MB ± 0% 31.6MB ± 0% ~ (p=0.056 n=5+5) Reflect 77.5MB ± 0% 77.2MB ± 0% -0.38% (p=0.008 n=5+5) Tar 26.4MB ± 0% 26.3MB ± 0% ~ (p=0.151 n=5+5) XML 41.9MB ± 0% 41.9MB ± 0% -0.20% (p=0.032 n=5+5) [Geo mean] 74.5MB 74.3MB -0.23% name old allocs/op new allocs/op delta Template 378k ± 1% 377k ± 1% ~ (p=0.690 n=5+5) Unicode 321k ± 0% 322k ± 0% ~ (p=0.595 n=5+5) GoTypes 1.14M ± 0% 1.14M ± 0% ~ (p=0.310 n=5+5) Compiler 4.25M ± 0% 4.25M ± 0% ~ (p=0.151 n=5+5) SSA 9.84M ± 0% 9.84M ± 0% ~ (p=0.841 n=5+5) Flate 232k ± 1% 232k ± 0% ~ (p=0.690 n=5+5) GoParser 315k ± 1% 315k ± 1% ~ (p=0.841 n=5+5) Reflect 970k ± 0% 970k ± 0% ~ (p=0.841 n=5+5) Tar 248k ± 0% 248k ± 1% ~ (p=0.841 n=5+5) XML 389k ± 0% 389k ± 0% ~ (p=1.000 n=5+5) [Geo mean] 724k 724k +0.01% Updates #15756 Change-Id: I2646332e89f0444ca9d5a41d7172537d904ed636 Reviewed-on: https://go-review.googlesource.com/41050 Run-TryBot: Josh Bleecher Snyder TryBot-Result: Gobot Gobot Reviewed-by: Matthew Dempsky --- src/cmd/internal/obj/line_test.go | 3 ++- src/cmd/internal/obj/link.go | 8 ++------ src/cmd/internal/obj/sym.go | 19 ++++++++++++++----- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/cmd/internal/obj/line_test.go b/src/cmd/internal/obj/line_test.go index 956302f8f1..329f245464 100644 --- a/src/cmd/internal/obj/line_test.go +++ b/src/cmd/internal/obj/line_test.go @@ -12,7 +12,8 @@ import ( func TestLinkgetlineFromPos(t *testing.T) { ctxt := new(Link) - ctxt.hash = make(map[SymVer]*LSym) + ctxt.hash = make(map[string]*LSym) + ctxt.vhash = make(map[string]*LSym) afile := src.NewFileBase("a.go", "a.go") bfile := src.NewFileBase("b.go", "/foo/bar/b.go") diff --git a/src/cmd/internal/obj/link.go b/src/cmd/internal/obj/link.go index 840df52256..dafb9359ae 100644 --- a/src/cmd/internal/obj/link.go +++ b/src/cmd/internal/obj/link.go @@ -480,7 +480,8 @@ type Link struct { Flag_optimize bool Bso *bufio.Writer Pathname string - hash map[SymVer]*LSym + hash map[string]*LSym // name -> sym mapping for version == 0 + vhash map[string]*LSym // name -> sym mapping for version == 1 PosTable src.PosTable InlTree InlTree // global inlining tree used by gc/inl.go Imports []string @@ -522,11 +523,6 @@ func (ctxt *Link) FixedFrameSize() int64 { } } -type SymVer struct { - Name string - Version int // TODO: make int16 to match LSym.Version? -} - // LinkArch is the definition of a single architecture. type LinkArch struct { *sys.Arch diff --git a/src/cmd/internal/obj/sym.go b/src/cmd/internal/obj/sym.go index 2514160769..83f5a18801 100644 --- a/src/cmd/internal/obj/sym.go +++ b/src/cmd/internal/obj/sym.go @@ -40,7 +40,8 @@ import ( func Linknew(arch *LinkArch) *Link { ctxt := new(Link) - ctxt.hash = make(map[SymVer]*LSym) + ctxt.hash = make(map[string]*LSym) + ctxt.vhash = make(map[string]*LSym) ctxt.Arch = arch ctxt.Pathname = objabi.WorkingDir() @@ -63,13 +64,21 @@ func (ctxt *Link) Lookup(name string, v int) *LSym { // LookupInit looks up the symbol with name name and version v. // If it does not exist, it creates it and passes it to initfn for one-time initialization. func (ctxt *Link) LookupInit(name string, v int, init func(s *LSym)) *LSym { - s := ctxt.hash[SymVer{name, v}] - if s != nil { + var m map[string]*LSym + switch v { + case 0: + m = ctxt.hash + case 1: + m = ctxt.vhash + default: + ctxt.Diag("LookupInit: bad version %d", v) + } + if s := m[name]; s != nil { return s } - s = &LSym{Name: name, Version: int16(v)} - ctxt.hash[SymVer{name, v}] = s + s := &LSym{Name: name, Version: int16(v)} + m[name] = s if init != nil { init(s) }