go/types/objectpath: add encoder type, to amortize allocation

This change adds a new encoder type with For method, that
is functionally equivalent to the old For function but avoids
the surprising cost of repeated calls to Scope.Names and
canonicalize (now called namedMethods), both of which
allocate and sorts a slice.

The former canonicalize function was previously applied to
interface types too, but this was costly and unnecessary
as they are already sorted.

The public API change will have to wait for proposal 58668
to be accepted and thus fix issue 51017; this change uses
the linkname hack to expose the function to x/tools to fix a
pressing bug in gopls.

See golang/go#58668
Updates golang/go#51017

Change-Id: Id3e8726517d31371b9376b0e47e320f8b6c5e085
Reviewed-on: https://go-review.googlesource.com/c/tools/+/470679
TryBot-Result: Gopher Robot <[email protected]>
gopls-CI: kokoro <[email protected]>
Run-TryBot: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: adonovan

go/types/objectpath/objectpath.go

@@ -31,6 +31,8 @@ import (
 	"strings"
 
 	"golang.org/x/tools/internal/typeparams"
+
+	_ "unsafe" // for go:linkname
 )
 
 // A Path is an opaque name that identifies a types.Object
@@ -111,7 +113,7 @@ const (
 	opObj    = 'O' // .Obj()		 (Named, TypeParam)
 )
 
-// The For function returns the path to an object relative to its package,
+// 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.
 //
 // The For function guarantees to return a path only for the following objects:
@@ -144,6 +146,23 @@ const (
 //
 // 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) {
 	pkg := obj.Pkg()
 
 	// This table lists the cases of interest.
@@ -225,7 +244,7 @@ func For(obj types.Object) (Path, error) {
 			return "", fmt.Errorf("func is not a method: %v", obj)
 		}
 
-		if path, ok := concreteMethod(obj); ok {
+		if path, ok := enc.concreteMethod(obj); ok {
 			// Fast path for concrete methods that avoids looping over scope.
 			return path, nil
 		}
@@ -241,7 +260,7 @@ func For(obj types.Object) (Path, error) {
 	// the best paths because non-types may
 	// refer to types, but not the reverse.
 	empty := make([]byte, 0, 48) // initial space
-	names := scope.Names()
+	names := enc.scopeNames(scope)
 	for _, name := range names {
 		o := scope.Lookup(name)
 		tname, ok := o.(*types.TypeName)
@@ -294,9 +313,7 @@ func For(obj types.Object) (Path, error) {
 			// Note that method index here is always with respect
 			// to canonical ordering of methods, regardless of how
 			// they appear in the underlying type.
-			canonical := canonicalize(T)
-			for i := 0; i < len(canonical); i++ {
-				m := canonical[i]
+			for i, m := range enc.namedMethods(T) {
 				path2 := appendOpArg(path, opMethod, i)
 				if m == obj {
 					return Path(path2), nil // found declared method
@@ -324,7 +341,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 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
@@ -397,8 +414,7 @@ func concreteMethod(meth *types.Func) (Path, bool) {
 	path := make([]byte, 0, len(name)+8)
 	path = append(path, name...)
 	path = append(path, opType)
-	canonical := canonicalize(named)
-	for i, m := range canonical {
+	for i, m := range enc.namedMethods(named) {
 		if m == meth {
 			path = appendOpArg(path, opMethod, i)
 			return Path(path), true
@@ -663,15 +679,23 @@ func Object(pkg *types.Package, p Path) (types.Object, error) {
 			t = nil
 
 		case opMethod:
-			hasMethods, ok := t.(hasMethods) // Interface or Named
-			if !ok {
+			switch t := t.(type) {
+			case *types.Interface:
+				if index >= t.NumMethods() {
+					return nil, fmt.Errorf("method index %d out of range [0-%d)", index, t.NumMethods())
+				}
+				obj = t.Method(index) // Id-ordered
+
+			case *types.Named:
+				methods := namedMethods(t) // (unmemoized)
+				if index >= len(methods) {
+					return nil, fmt.Errorf("method index %d out of range [0-%d)", index, len(methods))
+				}
+				obj = methods[index] // Id-ordered
+
+			default:
 				return nil, fmt.Errorf("cannot apply %q to %s (got %T, want interface or named)", code, t, t)
 			}
-			canonical := canonicalize(hasMethods)
-			if n := len(canonical); index >= n {
-				return nil, fmt.Errorf("method index %d out of range [0-%d)", index, n)
-			}
-			obj = canonical[index]
 			t = nil
 
 		case opObj:
@@ -694,27 +718,45 @@ func Object(pkg *types.Package, p Path) (types.Object, error) {
 	return obj, nil // success
 }
 
-// hasMethods is an abstraction of *types.{Interface,Named}. This is pulled up
-// because it is used by methodOrdering, which is in turn used by both encoding
-// and decoding.
-type hasMethods interface {
-	Method(int) *types.Func
-	NumMethods() int
+// namedMethods returns the methods of a Named type in ascending Id order.
+func namedMethods(named *types.Named) []*types.Func {
+	methods := make([]*types.Func, named.NumMethods())
+	for i := range methods {
+		methods[i] = named.Method(i)
+	}
+	sort.Slice(methods, func(i, j int) bool {
+		return methods[i].Id() < methods[j].Id()
+	})
+	return methods
 }
 
-// canonicalize returns a canonical order for the methods in a hasMethod.
-func canonicalize(hm hasMethods) []*types.Func {
-	count := hm.NumMethods()
-	if count <= 0 {
-		return nil
+// 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
 	}
-	canon := make([]*types.Func, count)
-	for i := 0; i < count; i++ {
-		canon[i] = hm.Method(i)
+	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 {
+	m := enc.namedMethodsMemo
+	if m == nil {
+		m = make(map[*types.Named][]*types.Func)
+		enc.namedMethodsMemo = m
 	}
-	less := func(i, j int) bool {
-		return canon[i].Id() < canon[j].Id()
+	methods, ok := m[named]
+	if !ok {
+		methods = namedMethods(named) // allocates and sorts
+		m[named] = methods
 	}
-	sort.Slice(canon, less)
-	return canon
+	return methods
+
 }

gopls/internal/lsp/source/methodsets/methodsets.go

@@ -54,6 +54,7 @@ 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
@@ -202,6 +203,8 @@ 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()
+
 	// setindexInfo sets the (Posn, PkgPath, ObjectPath) fields for each method declaration.
 	setIndexInfo := func(m *gobMethod, method *types.Func) {
 		// error.Error has empty Position, PkgPath, and ObjectPath.
@@ -214,7 +217,7 @@ func (b *indexBuilder) build(fset *token.FileSet, pkg *types.Package) *Index {
 
 		// Instantiations of generic methods don't have an
 		// object path, so we use the generic.
-		if p, err := objectpath.For(typeparams.OriginMethod(method)); err != nil {
+		if p, err := objectpathFor(typeparams.OriginMethod(method)); err != nil {
 			panic(err) // can't happen for a method of a package-level type
 		} else {
 			m.ObjectPath = b.string(string(p))

gopls/internal/lsp/source/xrefs/xrefs.go

@@ -19,6 +19,7 @@ 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
@@ -41,6 +42,8 @@ func Index(files []*source.ParsedGoFile, pkg *types.Package, info *types.Info) [
 		return objects
 	}
 
+	objectpathFor := typesinternal.NewObjectpathFunc()
+
 	for fileIndex, pgf := range files {
 
 		nodeRange := func(n ast.Node) protocol.Range {
@@ -65,7 +68,7 @@ func Index(files []*source.ParsedGoFile, pkg *types.Package, info *types.Info) [
 						objects := getObjects(obj.Pkg())
 						gobObj, ok := objects[obj]
 						if !ok {
-							path, err := objectpath.For(obj)
+							path, err := objectpathFor(obj)
 							if err != nil {
 								// Capitalized but not exported
 								// (e.g. local const/var/type).

internal/typesinternal/types.go

@@ -11,6 +11,8 @@ import (
 	"go/types"
 	"reflect"
 	"unsafe"
+
+	"golang.org/x/tools/go/types/objectpath"
 )
 
 func SetUsesCgo(conf *types.Config) bool {
@@ -50,3 +52,10 @@ func ReadGo116ErrorData(err types.Error) (code ErrorCode, start, end token.Pos,
 }
 
 var SetGoVersion = func(conf *types.Config, version string) bool { return false }
+
+// NewObjectpathEncoder returns a function closure equivalent to
+// objectpath.For but amortized for multiple (sequential) calls.
+// It is a temporary workaround, pending the approval of proposal 58668.
+//
+//go:linkname NewObjectpathFunc golang.org/x/tools/go/types/objectpath.newEncoderFor
+func NewObjectpathFunc() func(types.Object) (objectpath.Path, error)