go/token: use atomics not Mutex for last file cache

Previously, FileSet would cache the last *File found by a lookup,
using a full (exclusive) mutex within FileSet.File, turning a logical
read operation into an update. This was one of the largest sources
of contention in gopls.  This change uses atomic load/store on the
'last' field without a mutex.

Also, in FileSet.AddFile, allocate the File outside the critical
section; all the other operations are typically cheap.

Fixes #53507

Change-Id: Ice8641650d8495b25b0428e9b9320837ff2ca7e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/411909
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This commit is contained in:
Alan Donovan 2022-06-29 13:08:11 -04:00
parent e5017a93fc
commit 0750107074
2 changed files with 20 additions and 16 deletions

View file

@ -8,6 +8,7 @@ import (
"fmt" "fmt"
"sort" "sort"
"sync" "sync"
"sync/atomic"
) )
// ----------------------------------------------------------------------------- // -----------------------------------------------------------------------------
@ -366,10 +367,10 @@ func (f *File) Position(p Pos) (pos Position) {
// interval later, using the FileSet.Base should be used as argument // interval later, using the FileSet.Base should be used as argument
// for FileSet.AddFile. // for FileSet.AddFile.
type FileSet struct { type FileSet struct {
mutex sync.RWMutex // protects the file set mutex sync.RWMutex // protects the file set
base int // base offset for the next file base int // base offset for the next file
files []*File // list of files in the order added to the set files []*File // list of files in the order added to the set
last *File // cache of last file looked up last atomic.Pointer[File] // cache of last file looked up
} }
// NewFileSet creates a new file set. // NewFileSet creates a new file set.
@ -405,6 +406,9 @@ func (s *FileSet) Base() int {
// For convenience, File.Pos may be used to create file-specific position // For convenience, File.Pos may be used to create file-specific position
// values from a file offset. // values from a file offset.
func (s *FileSet) AddFile(filename string, base, size int) *File { func (s *FileSet) AddFile(filename string, base, size int) *File {
// Allocate f outside the critical section.
f := &File{name: filename, size: size, lines: []int{0}}
s.mutex.Lock() s.mutex.Lock()
defer s.mutex.Unlock() defer s.mutex.Unlock()
if base < 0 { if base < 0 {
@ -413,11 +417,11 @@ func (s *FileSet) AddFile(filename string, base, size int) *File {
if base < s.base { if base < s.base {
panic(fmt.Sprintf("invalid base %d (should be >= %d)", base, s.base)) panic(fmt.Sprintf("invalid base %d (should be >= %d)", base, s.base))
} }
f.base = base
if size < 0 { if size < 0 {
panic(fmt.Sprintf("invalid size %d (should be >= 0)", size)) panic(fmt.Sprintf("invalid size %d (should be >= 0)", size))
} }
// base >= s.base && size >= 0 // base >= s.base && size >= 0
f := &File{name: filename, base: base, size: size, lines: []int{0}}
base += size + 1 // +1 because EOF also has a position base += size + 1 // +1 because EOF also has a position
if base < 0 { if base < 0 {
panic("token.Pos offset overflow (> 2G of source code in file set)") panic("token.Pos offset overflow (> 2G of source code in file set)")
@ -425,7 +429,7 @@ func (s *FileSet) AddFile(filename string, base, size int) *File {
// add the file to the file set // add the file to the file set
s.base = base s.base = base
s.files = append(s.files, f) s.files = append(s.files, f)
s.last = f s.last.Store(f)
return f return f
} }
@ -450,25 +454,25 @@ func searchFiles(a []*File, x int) int {
} }
func (s *FileSet) file(p Pos) *File { func (s *FileSet) file(p Pos) *File {
s.mutex.RLock() // common case: p is in last file.
// common case: p is in last file if f := s.last.Load(); f != nil && f.base <= int(p) && int(p) <= f.base+f.size {
if f := s.last; f != nil && f.base <= int(p) && int(p) <= f.base+f.size {
s.mutex.RUnlock()
return f return f
} }
s.mutex.RLock()
defer s.mutex.RUnlock()
// p is not in last file - search all files // p is not in last file - search all files
if i := searchFiles(s.files, int(p)); i >= 0 { if i := searchFiles(s.files, int(p)); i >= 0 {
f := s.files[i] f := s.files[i]
// f.base <= int(p) by definition of searchFiles // f.base <= int(p) by definition of searchFiles
if int(p) <= f.base+f.size { if int(p) <= f.base+f.size {
s.mutex.RUnlock() // Update cache of last file. A race is ok,
s.mutex.Lock() // but an exclusive lock causes heavy contention.
s.last = f // race is ok - s.last is only a cache s.last.Store(f)
s.mutex.Unlock()
return f return f
} }
} }
s.mutex.RUnlock()
return nil return nil
} }

View file

@ -39,7 +39,7 @@ func (s *FileSet) Read(decode func(any) error) error {
} }
} }
s.files = files s.files = files
s.last = nil s.last.Store(nil)
s.mutex.Unlock() s.mutex.Unlock()
return nil return nil