Skip to content

Commit

Permalink
go/types/objectpath: add Encoder type, to amortize Scope.Names
Browse files Browse the repository at this point in the history
This change adds a new Encoder type with For method, that
is functionally equivalent to the old For method but avoids
the surprising cost of repeated calls to Scope.Names, which
allocates and sorts a slice.

See golang/go#58668
Fixes golang/go#51017

Change-Id: I328e60c60f9bc312af253a0509aa029c41960d41
Reviewed-on: https://go-review.googlesource.com/c/tools/+/470678
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Tim King <taking@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
adonovan committed Apr 8, 2023
1 parent 45ef829 commit a5338c9
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 39 deletions.
66 changes: 31 additions & 35 deletions go/types/objectpath/objectpath.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,20 @@ const (
opObj = 'O' // .Obj() (Named, TypeParam)
)

// For is equivalent to new(Encoder).For(obj).
//
// It may be more efficient to reuse a single Encoder across several calls.
func For(obj types.Object) (Path, error) {
return new(Encoder).For(obj)
}

// An Encoder amortizes the cost of encoding the paths of multiple objects.
// The zero value of an Encoder is ready to use.
type Encoder struct {
scopeNamesMemo map[*types.Scope][]string // memoization of Scope.Names()
namedMethodsMemo map[*types.Named][]*types.Func // memoization of namedMethods()
}

// For returns the path to an object relative to its package,
// or an error if the object is not accessible from the package's Scope.
//
Expand Down Expand Up @@ -145,24 +159,7 @@ const (
// .Type().Field(0) (field Var X)
//
// where p is the package (*types.Package) to which X belongs.
func For(obj types.Object) (Path, error) {
return newEncoderFor()(obj)
}

// An encoder amortizes the cost of encoding the paths of multiple objects.
// Nonexported pending approval of proposal 58668.
type encoder struct {
scopeNamesMemo map[*types.Scope][]string // memoization of Scope.Names()
namedMethodsMemo map[*types.Named][]*types.Func // memoization of namedMethods()
}

// Exposed to gopls via golang.org/x/tools/internal/typesinternal
// pending approval of proposal 58668.
//
//go:linkname newEncoderFor
func newEncoderFor() func(types.Object) (Path, error) { return new(encoder).For }

func (enc *encoder) For(obj types.Object) (Path, error) {
func (enc *Encoder) For(obj types.Object) (Path, error) {
pkg := obj.Pkg()

// This table lists the cases of interest.
Expand Down Expand Up @@ -341,7 +338,7 @@ func appendOpArg(path []byte, op byte, arg int) []byte {
// This function is just an optimization that avoids the general scope walking
// approach. You are expected to fall back to the general approach if this
// function fails.
func (enc *encoder) concreteMethod(meth *types.Func) (Path, bool) {
func (enc *Encoder) concreteMethod(meth *types.Func) (Path, bool) {
// Concrete methods can only be declared on package-scoped named types. For
// that reason we can skip the expensive walk over the package scope: the
// path will always be package -> named type -> method. We can trivially get
Expand Down Expand Up @@ -730,23 +727,8 @@ func namedMethods(named *types.Named) []*types.Func {
return methods
}

// scopeNames is a memoization of scope.Names. Callers must not modify the result.
func (enc *encoder) scopeNames(scope *types.Scope) []string {
m := enc.scopeNamesMemo
if m == nil {
m = make(map[*types.Scope][]string)
enc.scopeNamesMemo = m
}
names, ok := m[scope]
if !ok {
names = scope.Names() // allocates and sorts
m[scope] = names
}
return names
}

// namedMethods is a memoization of the namedMethods function. Callers must not modify the result.
func (enc *encoder) namedMethods(named *types.Named) []*types.Func {
func (enc *Encoder) namedMethods(named *types.Named) []*types.Func {
m := enc.namedMethodsMemo
if m == nil {
m = make(map[*types.Named][]*types.Func)
Expand All @@ -758,5 +740,19 @@ func (enc *encoder) namedMethods(named *types.Named) []*types.Func {
m[named] = methods
}
return methods
}

// scopeNames is a memoization of scope.Names. Callers must not modify the result.
func (enc *Encoder) scopeNames(scope *types.Scope) []string {
m := enc.scopeNamesMemo
if m == nil {
m = make(map[*types.Scope][]string)
enc.scopeNamesMemo = m
}
names, ok := m[scope]
if !ok {
names = scope.Names() // allocates and sorts
m[scope] = names
}
return names
}
3 changes: 1 addition & 2 deletions gopls/internal/lsp/source/methodsets/methodsets.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ import (
"golang.org/x/tools/go/types/objectpath"
"golang.org/x/tools/gopls/internal/lsp/safetoken"
"golang.org/x/tools/internal/typeparams"
"golang.org/x/tools/internal/typesinternal"
)

// An Index records the non-empty method sets of all package-level
Expand Down Expand Up @@ -232,7 +231,7 @@ func (b *indexBuilder) build(fset *token.FileSet, pkg *types.Package) *Index {
return gobPosition{b.string(posn.Filename), posn.Offset, len(obj.Name())}
}

objectpathFor := typesinternal.NewObjectpathFunc()
objectpathFor := new(objectpath.Encoder).For

// setindexInfo sets the (Posn, PkgPath, ObjectPath) fields for each method declaration.
setIndexInfo := func(m *gobMethod, method *types.Func) {
Expand Down
3 changes: 1 addition & 2 deletions gopls/internal/lsp/source/xrefs/xrefs.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"golang.org/x/tools/go/types/objectpath"
"golang.org/x/tools/gopls/internal/lsp/protocol"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/internal/typesinternal"
)

// Index constructs a serializable index of outbound cross-references
Expand All @@ -42,7 +41,7 @@ func Index(files []*source.ParsedGoFile, pkg *types.Package, info *types.Info) [
return objects
}

objectpathFor := typesinternal.NewObjectpathFunc()
objectpathFor := new(objectpath.Encoder).For

for fileIndex, pgf := range files {

Expand Down

0 comments on commit a5338c9

Please sign in to comment.