From 57e038615d945b610f4b62c40ddeb1fd40130649 Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Sat, 4 Mar 2017 07:09:33 -0800 Subject: [PATCH] cmd/internal/src: cache prefixed filenames MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit CL 37234 introduced string concatenation into some hot code. This CL does that work earlier and caches the result. Updates #19386 Performance impact vs master: name old time/op new time/op delta Template 223ms ± 5% 216ms ± 5% -2.98% (p=0.001 n=20+20) Unicode 98.7ms ± 4% 99.0ms ± 4% ~ (p=0.749 n=20+19) GoTypes 631ms ± 4% 626ms ± 4% ~ (p=0.253 n=20+20) Compiler 2.91s ± 1% 2.87s ± 3% -1.11% (p=0.005 n=18+20) SSA 4.48s ± 2% 4.36s ± 2% -2.77% (p=0.000 n=20+20) Flate 130ms ± 2% 129ms ± 6% ~ (p=0.428 n=19+20) GoParser 160ms ± 4% 157ms ± 3% -1.62% (p=0.005 n=20+18) Reflect 395ms ± 2% 394ms ± 4% ~ (p=0.445 n=20+20) Tar 120ms ± 5% 118ms ± 6% ~ (p=0.101 n=19+20) XML 224ms ± 3% 223ms ± 3% ~ (p=0.544 n=19+19) name old user-ns/op new user-ns/op delta Template 291user-ms ± 5% 265user-ms ± 5% -9.02% (p=0.000 n=20+19) Unicode 140user-ms ± 3% 139user-ms ± 8% ~ (p=0.904 n=20+20) GoTypes 844user-ms ± 3% 849user-ms ± 3% ~ (p=0.251 n=20+18) Compiler 4.06user-s ± 5% 3.98user-s ± 2% ~ (p=0.056 n=20+20) SSA 6.89user-s ± 5% 6.50user-s ± 3% -5.61% (p=0.000 n=20+20) Flate 164user-ms ± 5% 163user-ms ± 4% ~ (p=0.365 n=20+19) GoParser 206user-ms ± 6% 204user-ms ± 4% ~ (p=0.534 n=20+18) Reflect 501user-ms ± 4% 505user-ms ± 5% ~ (p=0.383 n=20+20) Tar 151user-ms ± 3% 152user-ms ± 7% ~ (p=0.798 n=17+20) XML 283user-ms ± 7% 280user-ms ± 5% ~ (p=0.301 n=20+20) name old alloc/op new alloc/op delta Template 42.5MB ± 0% 40.2MB ± 0% -5.59% (p=0.000 n=20+20) Unicode 31.7MB ± 0% 31.0MB ± 0% -2.19% (p=0.000 n=20+18) GoTypes 124MB ± 0% 117MB ± 0% -5.90% (p=0.000 n=20+20) Compiler 533MB ± 0% 490MB ± 0% -8.07% (p=0.000 n=20+20) SSA 989MB ± 0% 893MB ± 0% -9.74% (p=0.000 n=20+20) Flate 27.8MB ± 0% 26.1MB ± 0% -5.92% (p=0.000 n=20+20) GoParser 34.3MB ± 0% 32.1MB ± 0% -6.43% (p=0.000 n=19+20) Reflect 84.6MB ± 0% 81.4MB ± 0% -3.84% (p=0.000 n=20+20) Tar 28.8MB ± 0% 27.7MB ± 0% -3.89% (p=0.000 n=20+20) XML 47.2MB ± 0% 44.2MB ± 0% -6.45% (p=0.000 n=20+19) name old allocs/op new allocs/op delta Template 420k ± 1% 381k ± 1% -9.35% (p=0.000 n=20+20) Unicode 338k ± 1% 324k ± 1% -4.29% (p=0.000 n=20+19) GoTypes 1.28M ± 0% 1.15M ± 0% -10.30% (p=0.000 n=20+20) Compiler 5.06M ± 0% 4.41M ± 0% -12.92% (p=0.000 n=20+20) SSA 9.14M ± 0% 7.91M ± 0% -13.46% (p=0.000 n=19+20) Flate 267k ± 0% 241k ± 1% -9.53% (p=0.000 n=20+20) GoParser 347k ± 1% 312k ± 0% -10.15% (p=0.000 n=19+20) Reflect 1.07M ± 0% 1.00M ± 0% -6.86% (p=0.000 n=20+20) Tar 274k ± 1% 256k ± 1% -6.73% (p=0.000 n=20+20) XML 448k ± 0% 398k ± 0% -11.17% (p=0.000 n=20+18) Performance impact when applied together with CL 37234 atop CL 37234's parent commit (i.e. as if it were a part of CL 37234), to show that this commit makes CL 37234 completely performance-neutral: name old time/op new time/op delta Template 222ms ±14% 222ms ±14% ~ (p=1.000 n=14+15) Unicode 104ms ±18% 106ms ±18% ~ (p=0.650 n=13+14) GoTypes 653ms ± 7% 638ms ± 5% ~ (p=0.145 n=14+12) Compiler 3.10s ± 1% 3.13s ±10% ~ (p=1.000 n=2+2) SSA 4.73s ±11% 4.68s ±11% ~ (p=0.567 n=15+15) Flate 136ms ± 4% 133ms ± 7% ~ (p=0.231 n=12+14) GoParser 163ms ±11% 169ms ±10% ~ (p=0.352 n=14+14) Reflect 415ms ±15% 423ms ±20% ~ (p=0.715 n=15+14) Tar 133ms ±17% 130ms ±23% ~ (p=0.252 n=14+15) XML 236ms ±16% 235ms ±14% ~ (p=0.874 n=14+14) name old user-ns/op new user-ns/op delta Template 271user-ms ±10% 271user-ms ±10% ~ (p=0.780 n=14+15) Unicode 143user-ms ± 5% 146user-ms ±11% ~ (p=0.432 n=12+14) GoTypes 864user-ms ± 5% 866user-ms ± 9% ~ (p=0.905 n=14+13) Compiler 4.17user-s ± 1% 4.26user-s ± 7% ~ (p=1.000 n=2+2) SSA 6.79user-s ± 8% 6.79user-s ± 6% ~ (p=0.902 n=15+15) Flate 169user-ms ± 8% 164user-ms ± 5% -3.13% (p=0.014 n=14+14) GoParser 212user-ms ± 7% 217user-ms ±22% ~ (p=1.000 n=13+15) Reflect 521user-ms ± 7% 533user-ms ±15% ~ (p=0.511 n=14+14) Tar 165user-ms ±17% 161user-ms ±15% ~ (p=0.345 n=15+15) XML 294user-ms ±11% 292user-ms ±10% ~ (p=0.839 n=14+14) name old alloc/op new alloc/op delta Template 39.9MB ± 0% 39.9MB ± 0% ~ (p=0.621 n=15+14) Unicode 31.0MB ± 0% 31.0MB ± 0% ~ (p=0.098 n=13+15) GoTypes 117MB ± 0% 117MB ± 0% ~ (p=0.775 n=15+15) Compiler 488MB ± 0% 488MB ± 0% ~ (p=0.333 n=2+2) SSA 892MB ± 0% 892MB ± 0% +0.03% (p=0.000 n=15+15) Flate 26.1MB ± 0% 26.1MB ± 0% ~ (p=0.098 n=15+15) GoParser 31.8MB ± 0% 31.8MB ± 0% ~ (p=0.525 n=15+13) Reflect 81.2MB ± 0% 81.2MB ± 0% +0.06% (p=0.001 n=12+14) Tar 27.5MB ± 0% 27.5MB ± 0% ~ (p=0.595 n=15+15) XML 44.1MB ± 0% 44.1MB ± 0% ~ (p=0.486 n=15+15) name old allocs/op new allocs/op delta Template 378k ± 1% 378k ± 0% ~ (p=0.949 n=15+14) Unicode 324k ± 0% 324k ± 1% ~ (p=0.057 n=14+15) GoTypes 1.15M ± 0% 1.15M ± 0% ~ (p=0.461 n=15+15) Compiler 4.39M ± 0% 4.39M ± 0% ~ (p=0.333 n=2+2) SSA 7.90M ± 0% 7.90M ± 0% +0.06% (p=0.008 n=15+15) Flate 240k ± 1% 241k ± 0% ~ (p=0.233 n=15+15) GoParser 309k ± 1% 309k ± 0% ~ (p=0.867 n=15+12) Reflect 1.00M ± 0% 1.00M ± 0% ~ (p=0.139 n=12+15) Tar 254k ± 1% 253k ± 1% ~ (p=0.345 n=15+15) XML 398k ± 0% 397k ± 1% ~ (p=0.267 n=15+15) Change-Id: Ic999a0f456a371c99eebba0f9747263a13836e33 Reviewed-on: https://go-review.googlesource.com/37766 Run-TryBot: Josh Bleecher Snyder TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/cmd/internal/obj/line.go | 9 +++------ src/cmd/internal/obj/line_test.go | 2 +- src/cmd/internal/src/pos.go | 26 ++++++++++++++++++++++++-- src/cmd/link/internal/ld/pcln.go | 3 ++- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/cmd/internal/obj/line.go b/src/cmd/internal/obj/line.go index be6b36da41..540d56460d 100644 --- a/src/cmd/internal/obj/line.go +++ b/src/cmd/internal/obj/line.go @@ -74,16 +74,13 @@ func (ctxt *Link) AddImport(pkg string) { ctxt.Imports = append(ctxt.Imports, pkg) } -const FileSymPrefix = "gofile.." - func linkgetlineFromPos(ctxt *Link, xpos src.XPos) (f *LSym, l int32) { pos := ctxt.PosTable.Pos(xpos) - filename := pos.AbsFilename() - if !pos.IsKnown() || filename == "" { - return Linklookup(ctxt, FileSymPrefix+"??", 0), 0 + if !pos.IsKnown() { + pos = src.Pos{} } // TODO(gri) Should this use relative or absolute line number? - return Linklookup(ctxt, FileSymPrefix+filename, 0), int32(pos.RelLine()) + return Linklookup(ctxt, pos.SymFilename(), 0), int32(pos.RelLine()) } func fieldtrack(ctxt *Link, cursym *LSym) { diff --git a/src/cmd/internal/obj/line_test.go b/src/cmd/internal/obj/line_test.go index 928a008001..63cc29587c 100644 --- a/src/cmd/internal/obj/line_test.go +++ b/src/cmd/internal/obj/line_test.go @@ -32,7 +32,7 @@ func TestLinkgetlineFromPos(t *testing.T) { for _, test := range tests { f, l := linkgetlineFromPos(ctxt, ctxt.PosTable.XPos(test.pos)) got := fmt.Sprintf("%s:%d", f.Name, l) - if got != FileSymPrefix+test.want { + if got != src.FileSymPrefix+test.want { t.Errorf("linkgetline(%v) = %q, want %q", test.pos, got, test.want) } } diff --git a/src/cmd/internal/src/pos.go b/src/cmd/internal/src/pos.go index 35e213a6c5..198fdf7292 100644 --- a/src/cmd/internal/src/pos.go +++ b/src/cmd/internal/src/pos.go @@ -75,6 +75,10 @@ func (p Pos) RelLine() uint { b := p.base; return b.Line() + p.Line() - b.Pos(). // AbsFilename() returns the absolute filename recorded with the position's base. func (p Pos) AbsFilename() string { return p.base.AbsFilename() } +// SymFilename() returns the absolute filename recorded with the position's base, +// prefixed by FileSymPrefix to make it appropriate for use as a linker symbol. +func (p Pos) SymFilename() string { return p.base.SymFilename() } + func (p Pos) String() string { if !p.IsKnown() { return "" @@ -117,6 +121,7 @@ type PosBase struct { pos Pos filename string // file name used to open source file, for error messages absFilename string // absolute file name, for PC-Line tables + symFilename string // cached symbol file name, to avoid repeated string concatenation line uint // relative line number at pos inl int // inlining index (see cmd/internal/obj/inl.go) } @@ -125,7 +130,12 @@ type PosBase struct { // absolute) filenames. func NewFileBase(filename, absFilename string) *PosBase { if filename != "" { - base := &PosBase{filename: filename, absFilename: absFilename, inl: -1} + base := &PosBase{ + filename: filename, + absFilename: absFilename, + symFilename: FileSymPrefix + absFilename, + inl: -1, + } base.pos = MakePos(base, 0, 0) return base } @@ -136,7 +146,7 @@ func NewFileBase(filename, absFilename string) *PosBase { // //line filename:line // at position pos. func NewLinePragmaBase(pos Pos, filename string, line uint) *PosBase { - return &PosBase{pos, filename, filename, line - 1, -1} + return &PosBase{pos, filename, filename, FileSymPrefix + filename, line - 1, -1} } // NewInliningBase returns a copy of the old PosBase with the given inlining @@ -185,6 +195,18 @@ func (b *PosBase) AbsFilename() string { return "" } +const FileSymPrefix = "gofile.." + +// SymFilename returns the absolute filename recorded with the base, +// prefixed by FileSymPrefix to make it appropriate for use as a linker symbol. +// If b is nil, SymFilename returns FileSymPrefix + "??". +func (b *PosBase) SymFilename() string { + if b != nil { + return b.symFilename + } + return FileSymPrefix + "??" +} + // Line returns the line number recorded with the base. // If b == nil, the result is 0. func (b *PosBase) Line() uint { diff --git a/src/cmd/link/internal/ld/pcln.go b/src/cmd/link/internal/ld/pcln.go index c944da9f28..6df09bd817 100644 --- a/src/cmd/link/internal/ld/pcln.go +++ b/src/cmd/link/internal/ld/pcln.go @@ -6,6 +6,7 @@ package ld import ( "cmd/internal/obj" + "cmd/internal/src" "log" "os" "path/filepath" @@ -114,7 +115,7 @@ func numberfile(ctxt *Link, file *Symbol) { ctxt.Filesyms = append(ctxt.Filesyms, file) file.Value = int64(len(ctxt.Filesyms)) file.Type = obj.SFILEPATH - path := file.Name[len(obj.FileSymPrefix):] + path := file.Name[len(src.FileSymPrefix):] file.Name = expandGoroot(path) } } -- 2.48.1