From bdecfcb5fcc8705df7d2130310cf6b395f24e4c8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Daniel=20Mart=C3=AD?= Date: Fri, 19 Aug 2022 10:24:08 +0100 Subject: [PATCH] go/token: make mutex locking in unpack cheaper MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Run-TryBot: Daniel Martí Reviewed-by: Bryan Mills TryBot-Result: Gopher Robot --- src/go/token/position.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/go/token/position.go b/src/go/token/position.go index 5ca86a28e5..b12a8e4086 100644 --- 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 } -- 2.48.1