From a5338c9fa79213d770eb203ce603fecf456f1289 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 23 Feb 2023 13:49:41 -0500 Subject: [PATCH] go/types/objectpath: add Encoder type, to amortize Scope.Names 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 Reviewed-by: Tim King Run-TryBot: Alan Donovan TryBot-Result: Gopher Robot --- go/types/objectpath/objectpath.go | 66 +++++++++---------- .../lsp/source/methodsets/methodsets.go | 3 +- gopls/internal/lsp/source/xrefs/xrefs.go | 3 +- 3 files changed, 33 insertions(+), 39 deletions(-) diff --git a/go/types/objectpath/objectpath.go b/go/types/objectpath/objectpath.go index be8f5a867e6..e064a1a2926 100644 --- a/go/types/objectpath/objectpath.go +++ b/go/types/objectpath/objectpath.go @@ -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. // @@ -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. @@ -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 @@ -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) @@ -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 } diff --git a/gopls/internal/lsp/source/methodsets/methodsets.go b/gopls/internal/lsp/source/methodsets/methodsets.go index f8963b9139f..dac369badbb 100644 --- a/gopls/internal/lsp/source/methodsets/methodsets.go +++ b/gopls/internal/lsp/source/methodsets/methodsets.go @@ -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 @@ -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) { diff --git a/gopls/internal/lsp/source/xrefs/xrefs.go b/gopls/internal/lsp/source/xrefs/xrefs.go index eda69640134..6231f888430 100644 --- a/gopls/internal/lsp/source/xrefs/xrefs.go +++ b/gopls/internal/lsp/source/xrefs/xrefs.go @@ -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 @@ -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 {