From 73edd7b20868825223bce7947587fb1a1ddab213 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 3 Apr 2016 18:27:17 +0000 Subject: [PATCH] cmd/link: simplify readSymName, taking advantage of bufio.Reader Now that cmd/link uses bufio.Reader, take advantage of it. I find this new version easier to reason about. Reduces allocations by 1.1% when linking a basic HTTP server. Numbers are stable with each round measuring using: rm prof.mem; go tool link -o foo -memprofile=prof.mem -memprofilerate=1 foo.a Before: 65.36MB of 74.53MB total (87.70%) Dropped 157 nodes (cum <= 0.37MB) Showing top 10 nodes out of 39 (cum >= 1.47MB) flat flat% sum% cum cum% 21.48MB 28.81% 28.81% 21.48MB 28.81% cmd/link/internal/ld.Linklookup 16.04MB 21.52% 50.33% 16.04MB 21.52% cmd/link/internal/ld.(*objReader).readSlices 4.61MB 6.19% 56.52% 4.61MB 6.19% cmd/link/internal/ld.(*objReader).readSymName 4.51MB 6.05% 62.57% 6.32MB 8.48% cmd/link/internal/ld.writelines 4.50MB 6.03% 68.60% 4.50MB 6.03% cmd/link/internal/ld.Symgrow 4.02MB 5.39% 73.99% 4.02MB 5.39% cmd/link/internal/ld.linknew 3.98MB 5.34% 79.33% 3.98MB 5.34% cmd/link/internal/ld.setaddrplus 2.96MB 3.97% 83.30% 28.78MB 38.62% cmd/link/internal/ld.(*objReader).readRef 1.81MB 2.43% 85.73% 1.81MB 2.43% cmd/link/internal/ld.newcfaoffsetattr 1.47MB 1.97% 87.70% 1.47MB 1.97% cmd/link/internal/ld.(*objReader).readSym After: 64.66MB of 73.87MB total (87.53%) Dropped 156 nodes (cum <= 0.37MB) Showing top 10 nodes out of 40 (cum >= 1.47MB) flat flat% sum% cum cum% 21.48MB 29.08% 29.08% 21.48MB 29.08% cmd/link/internal/ld.Linklookup 16.04MB 21.71% 50.79% 16.04MB 21.71% cmd/link/internal/ld.(*objReader).readSlices 4.51MB 6.10% 56.90% 6.32MB 8.56% cmd/link/internal/ld.writelines 4.50MB 6.09% 62.99% 4.50MB 6.09% cmd/link/internal/ld.Symgrow 4.02MB 5.44% 68.42% 4.02MB 5.44% cmd/link/internal/ld.linknew 3.98MB 5.38% 73.81% 3.98MB 5.38% cmd/link/internal/ld.setaddrplus 3.90MB 5.28% 79.09% 3.90MB 5.28% cmd/link/internal/ld.(*objReader).readSymName 2.96MB 4.01% 83.09% 28.08MB 38.01% cmd/link/internal/ld.(*objReader).readRef 1.81MB 2.45% 85.55% 1.81MB 2.45% cmd/link/internal/ld.newcfaoffsetattr 1.47MB 1.99% 87.53% 1.47MB 1.99% cmd/link/internal/ld.(*objReader).readSym Also tested locally with asserts that that the calculated length is always correct and thus the adjName buf never reallocates. Change-Id: I19e3e8bfa6a12bcd8b5216f6232f42c122e4f80e Reviewed-on: https://go-review.googlesource.com/21481 Reviewed-by: David Crawshaw Run-TryBot: David Crawshaw TryBot-Result: Gobot Gobot --- src/cmd/link/internal/ld/objfile.go | 37 ++++++++++++++++++----------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/cmd/link/internal/ld/objfile.go b/src/cmd/link/internal/ld/objfile.go index bb6408aa820..b81dec6fd37 100644 --- a/src/cmd/link/internal/ld/objfile.go +++ b/src/cmd/link/internal/ld/objfile.go @@ -523,36 +523,38 @@ func (r *objReader) readData() []byte { // readSymName reads a symbol name, replacing all "". with pkg. func (r *objReader) readSymName() string { - rdBuf := r.rdBuf pkg := r.pkg n := r.readInt() if n == 0 { r.readInt64() return "" } - - if len(rdBuf) < n { - rdBuf = make([]byte, n, 2*n) + origName, err := r.rd.Peek(n) + if err != nil { + log.Fatalf("%s: unexpectedly long symbol name", r.pn) } - origName := rdBuf[:n] - r.readFull(origName) - adjName := rdBuf[n:n] + // Calculate needed scratch space, accounting for the growth + // of all the `"".` substrings to pkg+".": + need := len(origName) + maxInt(0, bytes.Count(origName, emptyPkg)*(len(pkg)+len(".")-len(emptyPkg))) + if len(r.rdBuf) < need { + r.rdBuf = make([]byte, need) + } + adjName := r.rdBuf[:0] for { i := bytes.Index(origName, emptyPkg) if i == -1 { - adjName = append(adjName, origName...) - break + s := string(append(adjName, origName...)) + // Read past the peeked origName, now that we're done with it, + // using the rfBuf (also no longer used) as the scratch space. + // TODO: use bufio.Reader.Discard if available instead? + r.readFull(r.rdBuf[:n]) + return s } adjName = append(adjName, origName[:i]...) adjName = append(adjName, pkg...) adjName = append(adjName, '.') origName = origName[i+len(emptyPkg):] } - name := string(adjName) - if len(adjName) > len(rdBuf) { - r.rdBuf = adjName // save the larger buffer for reuse - } - return name } // Reads the index of a symbol reference and resolves it to a symbol @@ -560,3 +562,10 @@ func (r *objReader) readSymIndex() *LSym { i := r.readInt() return r.refs[i] } + +func maxInt(a, b int) int { + if a > b { + return a + } + return b +}