From 5999de1043556149c077b0778261dcb62e0f8cd3 Mon Sep 17 00:00:00 2001 From: Muir Manders Date: Sat, 14 Sep 2019 09:29:08 -0700 Subject: [PATCH] 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 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot --- internal/lsp/lsp_test.go | 3 +++ internal/lsp/source/completion.go | 6 ------ internal/lsp/source/deep_completion.go | 10 +++++----- internal/lsp/source/options.go | 9 +++++++++ internal/lsp/source/source_test.go | 5 +++++ 5 files changed, 22 insertions(+), 11 deletions(-) diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 1e0c5f263c3..063367a8fe3 100644 --- a/internal/lsp/lsp_test.go +++ b/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) diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 9c64de9e28d..e9281c046cb 100644 --- a/internal/lsp/source/completion.go +++ b/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 diff --git a/internal/lsp/source/deep_completion.go b/internal/lsp/source/deep_completion.go index 3e2e6e8a602..b7eeacdae09 100644 --- a/internal/lsp/source/deep_completion.go +++ b/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 } diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 4bdcd3d9b14..205ed9ac072 100644 --- a/internal/lsp/source/options.go +++ b/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 diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 57c9a1ce354..7411829218f 100644 --- a/internal/lsp/source/source_test.go +++ b/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)