internal/test/integration: materialize startedWork and completedWork
While benchmarking, I noticed that the didChange benchmarks become much
slower on successive runs in the same session (-count=10), despite using
the same amount of server-side CPU per operation. Investigation revealed
that this was due to the time spent client side repeatedly counting
started work items. That predicate initially assumed a small number of
operations, but for benchmarks there will be tens or hundreds of
thousands of work items, as the didChange benchmarks run fast and share
a common session.
Fix this by materializing the startedWork and completedWork maps.
Change-Id: I4e96648cfd0f5af2285a35891f43a77143798856
Reviewed-on: https://go-review.googlesource.com/c/tools/+/586455
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/test/integration/env.go b/gopls/internal/test/integration/env.go
index 1d9091c..b8a695f 100644
--- a/gopls/internal/test/integration/env.go
+++ b/gopls/internal/test/integration/env.go
@@ -55,8 +55,10 @@
return &Awaiter{
workdir: workdir,
state: State{
- diagnostics: make(map[string]*protocol.PublishDiagnosticsParams),
- work: make(map[protocol.ProgressToken]*workProgress),
+ diagnostics: make(map[string]*protocol.PublishDiagnosticsParams),
+ work: make(map[protocol.ProgressToken]*workProgress),
+ startedWork: make(map[string]uint64),
+ completedWork: make(map[string]uint64),
},
waiters: make(map[int]*condition),
}
@@ -91,35 +93,16 @@
unregistrations []*protocol.UnregistrationParams
// outstandingWork is a map of token->work summary. All tokens are assumed to
- // be string, though the spec allows for numeric tokens as well. When work
- // completes, it is deleted from this map.
- work map[protocol.ProgressToken]*workProgress
-}
-
-// completedWork counts complete work items by title.
-func (s State) completedWork() map[string]uint64 {
- completed := make(map[string]uint64)
- for _, work := range s.work {
- if work.complete {
- completed[work.title]++
- }
- }
- return completed
-}
-
-// startedWork counts started (and possibly complete) work items.
-func (s State) startedWork() map[string]uint64 {
- started := make(map[string]uint64)
- for _, work := range s.work {
- started[work.title]++
- }
- return started
+ // be string, though the spec allows for numeric tokens as well.
+ work map[protocol.ProgressToken]*workProgress
+ startedWork map[string]uint64 // title -> count of 'begin'
+ completedWork map[string]uint64 // title -> count of 'end'
}
type workProgress struct {
title, msg, endMsg string
percent float64
- complete bool // seen 'end'.
+ complete bool // seen 'end'
}
// This method, provided for debugging, accesses mutable fields without a lock,
@@ -157,7 +140,7 @@
fmt.Fprintf(&b, "\t%s: %.2f\n", name, state.percent)
}
b.WriteString("#### completed work:\n")
- for name, count := range s.completedWork() {
+ for name, count := range s.completedWork {
fmt.Fprintf(&b, "\t%s: %d\n", name, count)
}
return b.String()
@@ -236,6 +219,7 @@
switch kind := v["kind"]; kind {
case "begin":
work.title = v["title"].(string)
+ a.state.startedWork[work.title]++
if msg, ok := v["message"]; ok {
work.msg = msg.(string)
}
@@ -248,6 +232,7 @@
}
case "end":
work.complete = true
+ a.state.completedWork[work.title]++
if msg, ok := v["message"]; ok {
work.endMsg = msg.(string)
}
diff --git a/gopls/internal/test/integration/env_test.go b/gopls/internal/test/integration/env_test.go
index 02bacd0..32203f7 100644
--- a/gopls/internal/test/integration/env_test.go
+++ b/gopls/internal/test/integration/env_test.go
@@ -15,7 +15,9 @@
func TestProgressUpdating(t *testing.T) {
a := &Awaiter{
state: State{
- work: make(map[protocol.ProgressToken]*workProgress),
+ work: make(map[protocol.ProgressToken]*workProgress),
+ startedWork: make(map[string]uint64),
+ completedWork: make(map[string]uint64),
},
}
ctx := context.Background()
@@ -55,8 +57,8 @@
t.Fatal(err)
}
}
- if !a.state.work["foo"].complete {
- t.Error("work entry \"foo\" is incomplete, want complete")
+ if got, want := a.state.completedWork["foo work"], uint64(1); got != want {
+ t.Errorf(`completedWork["foo work"] = %d, want %d`, got, want)
}
got := *a.state.work["bar"]
want := workProgress{title: "bar work", percent: 42}
diff --git a/gopls/internal/test/integration/expectation.go b/gopls/internal/test/integration/expectation.go
index b749800..fcc8459 100644
--- a/gopls/internal/test/integration/expectation.go
+++ b/gopls/internal/test/integration/expectation.go
@@ -407,7 +407,7 @@
// See CompletedWork.
func StartedWork(title string, atLeast uint64) Expectation {
check := func(s State) Verdict {
- if s.startedWork()[title] >= atLeast {
+ if s.startedWork[title] >= atLeast {
return Met
}
return Unmet
@@ -424,8 +424,8 @@
// progress notification title to identify the work we expect to be completed.
func CompletedWork(title string, count uint64, atLeast bool) Expectation {
check := func(s State) Verdict {
- completed := s.completedWork()
- if completed[title] == count || atLeast && completed[title] > count {
+ completed := s.completedWork[title]
+ if completed == count || atLeast && completed > count {
return Met
}
return Unmet
