gopls/internal/lsp/source: minor space optimizations

Memory profiles show heavy allocation for the stack
and the function closure of FindDeclAndField.
This change moves both outside the loop, reducing
this function's fraction of allocation from 6.7%
before to 5.0% after, and reducing running time
by 3-7% on these benchmarks.

before
BenchmarkStructCompletion/completion-8               100          10432280 ns/op
BenchmarkImportCompletion/completion-8              1350            921785 ns/op
BenchmarkSliceCompletion/completion-8                100          10876852 ns/op
BenchmarkFuncDeepCompletion/completion-8             142           7136768 ns/op
BenchmarkCompletionFollowingEdit/completion-8         63          21267031 ns/op

After
BenchmarkStructCompletion/completion-8               100          10030458 ns/op
BenchmarkImportCompletion/completion-8              1311            918306 ns/op
BenchmarkSliceCompletion/completion-8                100          10179937 ns/op
BenchmarkFuncDeepCompletion/completion-8             150           6986303 ns/op
BenchmarkCompletionFollowingEdit/completion-8         63          20575987 ns/op

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

Author: adonovan

gopls/internal/lsp/source/hover.go

@@ -904,78 +904,80 @@ func FindDeclAndField(files []*ast.File, pos token.Pos) (decl ast.Decl, field *a
 	}()
 
 	// Visit the files in search of the node at pos.
-	var stack []ast.Node
-	for _, file := range files {
-		ast.Inspect(file, func(n ast.Node) bool {
-			if n != nil {
-				stack = append(stack, n) // push
-			} else {
-				stack = stack[:len(stack)-1] // pop
-				return false
-			}
+	stack := make([]ast.Node, 0, 20)
+	// Allocate the closure once, outside the loop.
+	f := func(n ast.Node) bool {
+		if n != nil {
+			stack = append(stack, n) // push
+		} else {
+			stack = stack[:len(stack)-1] // pop
+			return false
+		}
 
-			// Skip subtrees (incl. files) that don't contain the search point.
-			if !(n.Pos() <= pos && pos < n.End()) {
-				return false
-			}
+		// Skip subtrees (incl. files) that don't contain the search point.
+		if !(n.Pos() <= pos && pos < n.End()) {
+			return false
+		}
 
-			switch n := n.(type) {
-			case *ast.Field:
-				checkField := func(f ast.Node) {
-					if f.Pos() == pos {
-						field = n
-						for i := len(stack) - 1; i >= 0; i-- {
-							if d, ok := stack[i].(ast.Decl); ok {
-								decl = d // innermost enclosing decl
-								break
-							}
+		switch n := n.(type) {
+		case *ast.Field:
+			checkField := func(f ast.Node) {
+				if f.Pos() == pos {
+					field = n
+					for i := len(stack) - 1; i >= 0; i-- {
+						if d, ok := stack[i].(ast.Decl); ok {
+							decl = d // innermost enclosing decl
+							break
 						}
-						panic(nil) // found
 					}
+					panic(nil) // found
 				}
+			}
 
-				// Check *ast.Field itself. This handles embedded
-				// fields which have no associated *ast.Ident name.
-				checkField(n)
+			// Check *ast.Field itself. This handles embedded
+			// fields which have no associated *ast.Ident name.
+			checkField(n)
 
-				// Check each field name since you can have
-				// multiple names for the same type expression.
-				for _, name := range n.Names {
-					checkField(name)
-				}
+			// Check each field name since you can have
+			// multiple names for the same type expression.
+			for _, name := range n.Names {
+				checkField(name)
+			}
 
-				// Also check "X" in "...X". This makes it easy
-				// to format variadic signature params properly.
-				if ell, ok := n.Type.(*ast.Ellipsis); ok && ell.Elt != nil {
-					checkField(ell.Elt)
-				}
+			// Also check "X" in "...X". This makes it easy
+			// to format variadic signature params properly.
+			if ell, ok := n.Type.(*ast.Ellipsis); ok && ell.Elt != nil {
+				checkField(ell.Elt)
+			}
 
-			case *ast.FuncDecl:
-				if n.Name.Pos() == pos {
-					decl = n
-					panic(nil) // found
-				}
+		case *ast.FuncDecl:
+			if n.Name.Pos() == pos {
+				decl = n
+				panic(nil) // found
+			}
 
-			case *ast.GenDecl:
-				for _, spec := range n.Specs {
-					switch spec := spec.(type) {
-					case *ast.TypeSpec:
-						if spec.Name.Pos() == pos {
+		case *ast.GenDecl:
+			for _, spec := range n.Specs {
+				switch spec := spec.(type) {
+				case *ast.TypeSpec:
+					if spec.Name.Pos() == pos {
+						decl = n
+						panic(nil) // found
+					}
+				case *ast.ValueSpec:
+					for _, id := range spec.Names {
+						if id.Pos() == pos {
 							decl = n
 							panic(nil) // found
 						}
-					case *ast.ValueSpec:
-						for _, id := range spec.Names {
-							if id.Pos() == pos {
-								decl = n
-								panic(nil) // found
-							}
-						}
 					}
 				}
 			}
-			return true
-		})
+		}
+		return true
+	}
+	for _, file := range files {
+		ast.Inspect(file, f)
 	}
 
 	return nil, nil