go/types/objectpath: avoid sorting methods for gopls

Gopls can rely on a deterministic ordering of methods, and therefore
does not need to sort in the objectpath algorithm, which turns out to be
enormously expensive on certain repositories (see golang/go#61178).

Add hooks to skip method sorting during encode/decode, and use this for
analysis facts. This CL is intentionally surgical, to avoid unintended
side-effects for this change in behavior. Notably, other uses of
objectpath are unaffected.

For golang/go#61178

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

Author: findleyr

Diff for: go/analysis/unitchecker/unitchecker.go

@@ -294,7 +294,7 @@ func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]re
 		}
 		return nil, nil // no .vetx file, no facts
 	}
-	facts, err := facts.NewDecoder(pkg).Decode(read)
+	facts, err := facts.NewDecoder(pkg).Decode(false, read)
 	if err != nil {
 		return nil, err
 	}
@@ -393,7 +393,7 @@ func run(fset *token.FileSet, cfg *Config, analyzers []*analysis.Analyzer) ([]re
 		results[i].diagnostics = act.diagnostics
 	}
 
-	data := facts.Encode()
+	data := facts.Encode(false)
 	if err := ioutil.WriteFile(cfg.VetxOutput, data, 0666); err != nil {
 		return nil, fmt.Errorf("failed to write analysis facts: %v", err)
 	}

Diff for: go/types/objectpath/objectpath.go

@@ -29,6 +29,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	_ "unsafe"
 
 	"golang.org/x/tools/internal/typeparams"
 )
@@ -121,8 +122,17 @@ func For(obj types.Object) (Path, error) {
 // 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 {
-	scopeMemo        map[*types.Scope][]types.Object // memoization of scopeObjects
-	namedMethodsMemo map[*types.Named][]*types.Func  // memoization of namedMethods()
+	scopeMemo         map[*types.Scope][]types.Object // memoization of scopeObjects
+	namedMethodsMemo  map[*types.Named][]*types.Func  // memoization of namedMethods()
+	skipMethodSorting bool
+}
+
+// Exposed to gopls via golang.org/x/tools/internal/typesinternal
+// TODO(golang/go#61443): eliminate this parameter one way or the other.
+//
+//go:linkname skipMethodSorting
+func skipMethodSorting(enc *Encoder) {
+	enc.skipMethodSorting = true
 }
 
 // For returns the path to an object relative to its package,
@@ -314,16 +324,31 @@ func (enc *Encoder) For(obj types.Object) (Path, error) {
 		// Inspect declared methods of defined types.
 		if T, ok := o.Type().(*types.Named); ok {
 			path = append(path, opType)
-			// Note that method index here is always with respect
-			// to canonical ordering of methods, regardless of how
-			// they appear in the underlying type.
-			for i, m := range enc.namedMethods(T) {
-				path2 := appendOpArg(path, opMethod, i)
-				if m == obj {
-					return Path(path2), nil // found declared method
+			if !enc.skipMethodSorting {
+				// Note that method index here is always with respect
+				// to canonical ordering of methods, regardless of how
+				// they appear in the underlying type.
+				for i, m := range enc.namedMethods(T) {
+					path2 := appendOpArg(path, opMethod, i)
+					if m == obj {
+						return Path(path2), nil // found declared method
+					}
+					if r := find(obj, m.Type(), append(path2, opType), nil); r != nil {
+						return Path(r), nil
+					}
 				}
-				if r := find(obj, m.Type(), append(path2, opType), nil); r != nil {
-					return Path(r), nil
+			} else {
+				// This branch must match the logic in the branch above, using go/types
+				// APIs without sorting.
+				for i := 0; i < T.NumMethods(); i++ {
+					m := T.Method(i)
+					path2 := appendOpArg(path, opMethod, i)
+					if m == obj {
+						return Path(path2), nil // found declared method
+					}
+					if r := find(obj, m.Type(), append(path2, opType), nil); r != nil {
+						return Path(r), nil
+					}
 				}
 			}
 		}
@@ -418,10 +443,23 @@ func (enc *Encoder) concreteMethod(meth *types.Func) (Path, bool) {
 	path := make([]byte, 0, len(name)+8)
 	path = append(path, name...)
 	path = append(path, opType)
-	for i, m := range enc.namedMethods(named) {
-		if m == meth {
-			path = appendOpArg(path, opMethod, i)
-			return Path(path), true
+
+	if !enc.skipMethodSorting {
+		for i, m := range enc.namedMethods(named) {
+			if m == meth {
+				path = appendOpArg(path, opMethod, i)
+				return Path(path), true
+			}
+		}
+	} else {
+		// This branch must match the logic of the branch above, using go/types
+		// APIs without sorting.
+		for i := 0; i < named.NumMethods(); i++ {
+			m := named.Method(i)
+			if m == meth {
+				path = appendOpArg(path, opMethod, i)
+				return Path(path), true
+			}
 		}
 	}
 
@@ -534,6 +572,12 @@ func findTypeParam(obj types.Object, list *typeparams.TypeParamList, path []byte
 
 // Object returns the object denoted by path p within the package pkg.
 func Object(pkg *types.Package, p Path) (types.Object, error) {
+	return object(pkg, p, false)
+}
+
+// Note: the skipMethodSorting parameter must match the value of
+// Encoder.skipMethodSorting used during encoding.
+func object(pkg *types.Package, p Path, skipMethodSorting bool) (types.Object, error) {
 	if p == "" {
 		return nil, fmt.Errorf("empty path")
 	}
@@ -697,11 +741,15 @@ func Object(pkg *types.Package, p Path) (types.Object, error) {
 				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))
+				if index >= t.NumMethods() {
+					return nil, fmt.Errorf("method index %d out of range [0-%d)", index, t.NumMethods())
+				}
+				if skipMethodSorting {
+					obj = t.Method(index)
+				} else {
+					methods := namedMethods(t) // (unmemoized)
+					obj = methods[index]       // Id-ordered
 				}
-				obj = methods[index] // Id-ordered
 
 			default:
 				return nil, fmt.Errorf("cannot apply %q to %s (got %T, want interface or named)", code, t, t)

Diff for: gopls/internal/lsp/cache/analysis.go

@@ -1172,7 +1172,7 @@ func (act *action) exec() (interface{}, *actionSummary, error) {
 	// of PkgPaths and objectpaths.)
 
 	// Read and decode analysis facts for each direct import.
-	factset, err := pkg.factsDecoder.Decode(func(pkgPath string) ([]byte, error) {
+	factset, err := pkg.factsDecoder.Decode(true, func(pkgPath string) ([]byte, error) {
 		if !hasFacts {
 			return nil, nil // analyzer doesn't use facts, so no vdeps
 		}
@@ -1314,7 +1314,7 @@ func (act *action) exec() (interface{}, *actionSummary, error) {
 		panic(fmt.Sprintf("%v: Pass.ExportPackageFact(%T) called after Run", act, fact))
 	}
 
-	factsdata := factset.Encode()
+	factsdata := factset.Encode(true)
 	return result, &actionSummary{
 		Diagnostics: diagnostics,
 		Facts:       factsdata,

Diff for: internal/facts/facts.go

@@ -48,6 +48,7 @@ import (
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/types/objectpath"
+	"golang.org/x/tools/internal/typesinternal"
 )
 
 const debug = false
@@ -204,7 +205,9 @@ type GetPackageFunc = func(pkgPath string) *types.Package
 //
 // Concurrent calls to Decode are safe, so long as the
 // [GetPackageFunc] (if any) is also concurrency-safe.
-func (d *Decoder) Decode(read func(pkgPath string) ([]byte, error)) (*Set, error) {
+//
+// TODO(golang/go#61443): eliminate skipMethodSorting one way or the other.
+func (d *Decoder) Decode(skipMethodSorting bool, read func(pkgPath string) ([]byte, error)) (*Set, error) {
 	// Read facts from imported packages.
 	// Facts may describe indirectly imported packages, or their objects.
 	m := make(map[key]analysis.Fact) // one big bucket
@@ -244,7 +247,7 @@ func (d *Decoder) Decode(read func(pkgPath string) ([]byte, error)) (*Set, error
 			key := key{pkg: factPkg, t: reflect.TypeOf(f.Fact)}
 			if f.Object != "" {
 				// object fact
-				obj, err := objectpath.Object(factPkg, f.Object)
+				obj, err := typesinternal.ObjectpathObject(factPkg, f.Object, skipMethodSorting)
 				if err != nil {
 					// (most likely due to unexported object)
 					// TODO(adonovan): audit for other possibilities.
@@ -268,7 +271,11 @@ func (d *Decoder) Decode(read func(pkgPath string) ([]byte, error)) (*Set, error
 //
 // It may fail if one of the Facts could not be gob-encoded, but this is
 // a sign of a bug in an Analyzer.
-func (s *Set) Encode() []byte {
+func (s *Set) Encode(skipMethodSorting bool) []byte {
+	encoder := new(objectpath.Encoder)
+	if skipMethodSorting {
+		typesinternal.SkipEncoderMethodSorting(encoder)
+	}
 
 	// TODO(adonovan): opt: use a more efficient encoding
 	// that avoids repeating PkgPath for each fact.
@@ -283,7 +290,7 @@ func (s *Set) Encode() []byte {
 		}
 		var object objectpath.Path
 		if k.obj != nil {
-			path, err := objectpath.For(k.obj)
+			path, err := encoder.For(k.obj)
 			if err != nil {
 				if debug {
 					log.Printf("discarding fact %s about %s\n", fact, k.obj)

Diff for: internal/facts/facts_test.go

@@ -311,7 +311,7 @@ func testEncodeDecode(t *testing.T, files map[string]string, tests []pkgLookups)
 		}
 
 		// decode
-		facts, err := facts.NewDecoder(pkg).Decode(read)
+		facts, err := facts.NewDecoder(pkg).Decode(false, read)
 		if err != nil {
 			t.Fatalf("Decode failed: %v", err)
 		}
@@ -345,7 +345,7 @@ func testEncodeDecode(t *testing.T, files map[string]string, tests []pkgLookups)
 		}
 
 		// encode
-		factmap[pkg.Path()] = facts.Encode()
+		factmap[pkg.Path()] = facts.Encode(false)
 	}
 }
 
@@ -413,7 +413,7 @@ func TestFactFilter(t *testing.T) {
 	}
 
 	obj := pkg.Scope().Lookup("A")
-	s, err := facts.NewDecoder(pkg).Decode(func(pkgPath string) ([]byte, error) { return nil, nil })
+	s, err := facts.NewDecoder(pkg).Decode(false, func(pkgPath string) ([]byte, error) { return nil, nil })
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -526,7 +526,7 @@ func TestMalformed(t *testing.T) {
 				packages[pkg.Path()] = pkg
 
 				// decode facts
-				facts, err := facts.NewDecoder(pkg).Decode(read)
+				facts, err := facts.NewDecoder(pkg).Decode(false, read)
 				if err != nil {
 					t.Fatalf("Decode failed: %v", err)
 				}
@@ -553,7 +553,7 @@ func TestMalformed(t *testing.T) {
 				}
 
 				// encode facts
-				factmap[pkg.Path()] = facts.Encode()
+				factmap[pkg.Path()] = facts.Encode(false)
 			}
 		})
 	}

Diff for: 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,17 @@ func ReadGo116ErrorData(err types.Error) (code ErrorCode, start, end token.Pos,
 }
 
 var SetGoVersion = func(conf *types.Config, version string) bool { return false }
+
+// SkipEncoderMethodSorting marks the encoder as not requiring sorted methods,
+// as an optimization for gopls (which guarantees the order of parsed source files).
+//
+// TODO(golang/go#61443): eliminate this parameter one way or the other.
+//
+//go:linkname SkipEncoderMethodSorting golang.org/x/tools/go/types/objectpath.skipMethodSorting
+func SkipEncoderMethodSorting(enc *objectpath.Encoder)
+
+// ObjectpathObject is like objectpath.Object, but allows suppressing method
+// sorting (which is not necessary for gopls).
+//
+//go:linkname ObjectpathObject golang.org/x/tools/go/types/objectpath.object
+func ObjectpathObject(pkg *types.Package, p objectpath.Path, skipMethodSorting bool) (types.Object, error)