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>
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 1e0c5f2..063367a 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -15,6 +15,7 @@
"sort"
"strings"
"testing"
+ "time"
"golang.org/x/tools/go/packages/packagestest"
"golang.org/x/tools/internal/lsp/cache"
@@ -60,6 +61,8 @@
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 9c64de9..e9281c0 100644
--- a/internal/lsp/source/completion.go
+++ b/internal/lsp/source/completion.go
@@ -111,12 +111,6 @@
// 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 3e2e6e8..b7eeacd 100644
--- a/internal/lsp/source/deep_completion.go
+++ b/internal/lsp/source/deep_completion.go
@@ -100,11 +100,9 @@
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 @@
}
}
+ 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 4bdcd3d..205ed9a 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -7,6 +7,7 @@
import (
"fmt"
"os"
+ "time"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/telemetry/tag"
@@ -32,6 +33,7 @@
Documentation: true,
Deep: true,
FuzzyMatching: true,
+ Budget: 100 * time.Millisecond,
},
}
)
@@ -70,6 +72,13 @@
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 57c9a1c..7411829 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -14,6 +14,7 @@
"sort"
"strings"
"testing"
+ "time"
"golang.org/x/tools/go/packages/packagestest"
"golang.org/x/tools/internal/lsp/cache"
@@ -107,6 +108,8 @@
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 @@
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)
