go/token: make mutex locking in unpack cheaper

I was profiling the cpu usage of go/printer's only benchmark,
and found that token.File.Unpack was one of the top offenders.

It was mainly the deferred unlock that took a big chunk of time,
and to my surprise, reoving the use of defer helped significantly:

	name      old time/op    new time/op    delta
	Print-16    5.61ms ± 2%    5.38ms ± 1%  -4.04%  (p=0.000 n=10+8)

	name      old speed      new speed      delta
	Print-16  9.27MB/s ± 2%  9.64MB/s ± 1%  +4.03%  (p=0.000 n=9+8)

	name      old alloc/op   new alloc/op   delta
	Print-16     332kB ± 0%     332kB ± 0%    ~     (p=0.363 n=10+10)

	name      old allocs/op  new allocs/op  delta
	Print-16     3.45k ± 0%     3.45k ± 0%    ~     (all equal)

It seems like #38471 is to blame, as the defer prevents Unlock from
being inlined. Add a TODO as a reminder to come back here once the
compiler issue is fixed.

Change-Id: I5a1c6d36a8e8357435a305a1bc0970ee0358b08a
Reviewed-on: https://go-review.googlesource.com/c/go/+/424920
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/go/token/position.go b/src/go/token/position.go
index 5ca86a28e5047df6e42ed4466ed795b9b76376e9..b12a8e408605acd7946ed90707854f94dcd549a9 100644 (file)
--- a/src/go/token/position.go
+++ b/src/go/token/position.go
@@ -286,7 +286,6 @@ func searchLineInfos(a []lineInfo, x int) int {
 // possibly adjusted by //line comments; otherwise those comments are ignored.
 func (f *File) unpack(offset int, adjusted bool) (filename string, line, column int) {
        f.mutex.Lock()
-       defer f.mutex.Unlock()
        filename = f.name
        if i := searchInts(f.lines, offset); i >= 0 {
                line, column = i+1, offset-f.lines[i]+1
@@ -314,6 +313,9 @@ func (f *File) unpack(offset int, adjusted bool) (filename string, line, column
                        }
                }
        }
+       // TODO(mvdan): move Unlock back under Lock with a defer statement once
+       // https://go.dev/issue/38471 is fixed to remove the performance penalty.
+       f.mutex.Unlock()
        return
 }