internal/lsp: tighten up completion budget check

Tweak a couple things to improve how we reduce our search scope based
on remaining time budget:

- Check our budget on the first candidate rather than waiting for the
  1000th candidate. If type checking is slow you can be out of budget
  before you even begin.
- Reduce our budget check interval from 1000 candidates to 100
  candidates. This just helps us adjust our search scope faster.

The first tweak required me to raise the completion budget for tests
because 100ms is not always enough. I moved the budget into the
completion options so that tests can raise it.

Change-Id: I1aa7909d7baf9c998bc830c960dc579eb33db12a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195419
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

internal/lsp/lsp_test.go

@@ -15,6 +15,7 @@ import (
 	"sort"
 	"strings"
 	"testing"
+	"time"
 
 	"golang.org/x/tools/go/packages/packagestest"
 	"golang.org/x/tools/internal/lsp/cache"
@@ -60,6 +61,8 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) {
 		source.Sum: {},
 	}
 	options.HoverKind = source.SynopsisDocumentation
+	// Crank this up so tests don't flake.
+	options.Completion.Budget = 5 * time.Second
 	session.SetOptions(options)
 	options.Env = data.Config.Env
 	session.NewView(ctx, viewName, span.FileURI(data.Config.Dir), options)

internal/lsp/source/completion.go

@@ -111,12 +111,6 @@ const (
 
 	// lowScore indicates an irrelevant or not useful completion item.
 	lowScore float64 = 0.01
-
-	// completionBudget is the soft latency goal for completion requests. Most
-	// requests finish in a couple milliseconds, but in some cases deep
-	// completions can take much longer. As we use up our budget we dynamically
-	// reduce the search scope to ensure we return timely results.
-	completionBudget = 100 * time.Millisecond
 )
 
 // matcher matches a candidate's label against the user input. The

internal/lsp/source/deep_completion.go

@@ -100,11 +100,9 @@ func (c *completer) shouldPrune() bool {
 		return false
 	}
 
-	c.deepState.candidateCount++
-
-	// Check our remaining budget every 1000 candidates.
-	if c.deepState.candidateCount%1000 == 0 {
-		spent := float64(time.Since(c.startTime)) / float64(completionBudget)
+	// Check our remaining budget every 100 candidates.
+	if c.deepState.candidateCount%100 == 0 {
+		spent := float64(time.Since(c.startTime)) / float64(c.opts.Budget)
 
 		switch {
 		case spent >= 0.90:
@@ -125,6 +123,8 @@ func (c *completer) shouldPrune() bool {
 		}
 	}
 
+	c.deepState.candidateCount++
+
 	if c.deepState.maxDepth >= 0 {
 		return len(c.deepState.chain) >= c.deepState.maxDepth
 	}

internal/lsp/source/options.go

@@ -7,6 +7,7 @@ package source
 import (
 	"fmt"
 	"os"
+	"time"
 
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/telemetry/tag"
@@ -32,6 +33,7 @@ var (
 			Documentation: true,
 			Deep:          true,
 			FuzzyMatching: true,
+			Budget:        100 * time.Millisecond,
 		},
 	}
 )
@@ -70,6 +72,13 @@ type CompletionOptions struct {
 	Documentation     bool
 	FullDocumentation bool
 	Placeholders      bool
+
+	// Budget is the soft latency goal for completion requests. Most
+	// requests finish in a couple milliseconds, but in some cases deep
+	// completions can take much longer. As we use up our budget we
+	// dynamically reduce the search scope to ensure we return timely
+	// results.
+	Budget time.Duration
 }
 
 type HoverKind int

internal/lsp/source/source_test.go

@@ -14,6 +14,7 @@ import (
 	"sort"
 	"strings"
 	"testing"
+	"time"
 
 	"golang.org/x/tools/go/packages/packagestest"
 	"golang.org/x/tools/internal/lsp/cache"
@@ -107,6 +108,8 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
 			Deep:          deepComplete,
 			FuzzyMatching: fuzzyMatch,
 			Unimported:    unimported,
+			// Crank this up so tests don't flake.
+			Budget: 5 * time.Second,
 		})
 		if err != nil {
 			t.Fatalf("failed for %v: %v", src, err)
@@ -169,6 +172,8 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests
 				Deep:          strings.Contains(string(src.URI()), "deepcomplete"),
 				FuzzyMatching: strings.Contains(string(src.URI()), "fuzzymatch"),
 				Placeholders:  usePlaceholders,
+				// Crank this up so tests don't flake.
+				Budget: 5 * time.Second,
 			})
 			if err != nil {
 				t.Fatalf("failed for %v: %v", src, err)