Skip to content

Commit b742cb9

Browse files
committed
gopls/internal/regtest/bench: add a benchmark for diagnosing saves
As we discovered while investigating golang/go#60089, mod tidy operations can significantly affect the amount of time it takes gopls to diagnose a saved file. Add a benchmark for this operation. For reference, this new benchmark takes 8s+ on google-cloud-go, vs 300ms for DiagnoseChange (without the save). Updates golang/go#60089 Change-Id: Ie88bd63dd7d205b8629173e7f84aa1aa9858016b Reviewed-on: https://go-review.googlesource.com/c/tools/+/496435 TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]> Run-TryBot: Robert Findley <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent 4d66324 commit b742cb9

File tree

1 file changed

+61
-39
lines changed

1 file changed

+61
-39
lines changed

gopls/internal/regtest/bench/didchange_test.go

Lines changed: 61 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@ import (
1919
// shared file cache.
2020
var editID int64 = time.Now().UnixNano()
2121

22-
var didChangeTests = []struct {
22+
type changeTest struct {
2323
repo string
2424
file string
25-
}{
26-
{"google-cloud-go", "httpreplay/httpreplay.go"},
25+
}
26+
27+
var didChangeTests = []changeTest{
28+
{"google-cloud-go", "internal/annotate.go"},
2729
{"istio", "pkg/fuzz/util.go"},
2830
{"kubernetes", "pkg/controller/lookup_cache.go"},
2931
{"kuma", "api/generic/insights.go"},
@@ -64,43 +66,63 @@ func BenchmarkDidChange(b *testing.B) {
6466

6567
func BenchmarkDiagnoseChange(b *testing.B) {
6668
for _, test := range didChangeTests {
67-
b.Run(test.repo, func(b *testing.B) {
68-
sharedEnv := getRepo(b, test.repo).sharedEnv(b)
69-
config := fake.EditorConfig{
70-
Env: map[string]string{
71-
"GOPATH": sharedEnv.Sandbox.GOPATH(),
72-
},
73-
Settings: map[string]interface{}{
74-
"diagnosticsDelay": "0s",
75-
},
76-
}
77-
// Use a new env to avoid the diagnostic delay: we want to measure how
78-
// long it takes to produce the diagnostics.
79-
env := getRepo(b, test.repo).newEnv(b, "diagnoseChange", config)
80-
defer env.Close()
81-
env.OpenFile(test.file)
82-
// Insert the text we'll be modifying at the top of the file.
83-
env.EditBuffer(test.file, protocol.TextEdit{NewText: "// __REGTEST_PLACEHOLDER_0__\n"})
84-
env.AfterChange()
85-
b.ResetTimer()
69+
runChangeDiagnosticsBenchmark(b, test, false)
70+
}
71+
}
72+
73+
// TODO(rfindley): add a benchmark for with a metadata-affecting change, when
74+
// this matters.
75+
func BenchmarkDiagnoseSave(b *testing.B) {
76+
for _, test := range didChangeTests {
77+
runChangeDiagnosticsBenchmark(b, test, true)
78+
}
79+
}
80+
81+
// runChangeDiagnosticsBenchmark runs a benchmark to edit the test file and
82+
// await the resulting diagnostics pass. If save is set, the file is also saved.
83+
func runChangeDiagnosticsBenchmark(b *testing.B, test changeTest, save bool) {
84+
b.Run(test.repo, func(b *testing.B) {
85+
sharedEnv := getRepo(b, test.repo).sharedEnv(b)
86+
config := fake.EditorConfig{
87+
Env: map[string]string{
88+
"GOPATH": sharedEnv.Sandbox.GOPATH(),
89+
},
90+
Settings: map[string]interface{}{
91+
"diagnosticsDelay": "0s",
92+
},
93+
}
94+
// Use a new env to avoid the diagnostic delay: we want to measure how
95+
// long it takes to produce the diagnostics.
96+
env := getRepo(b, test.repo).newEnv(b, "diagnoseSave", config)
97+
defer env.Close()
98+
env.OpenFile(test.file)
99+
// Insert the text we'll be modifying at the top of the file.
100+
env.EditBuffer(test.file, protocol.TextEdit{NewText: "// __REGTEST_PLACEHOLDER_0__\n"})
101+
if save {
102+
env.SaveBuffer(test.file)
103+
}
104+
env.AfterChange()
105+
b.ResetTimer()
86106

87-
// We must use an extra subtest layer here, so that we only set up the
88-
// shared env once (otherwise we pay additional overhead and the profiling
89-
// flags don't work).
90-
b.Run("diagnose", func(b *testing.B) {
91-
for i := 0; i < b.N; i++ {
92-
edits := atomic.AddInt64(&editID, 1)
93-
env.EditBuffer(test.file, protocol.TextEdit{
94-
Range: protocol.Range{
95-
Start: protocol.Position{Line: 0, Character: 0},
96-
End: protocol.Position{Line: 1, Character: 0},
97-
},
98-
// Increment the placeholder text, to ensure cache misses.
99-
NewText: fmt.Sprintf("// __REGTEST_PLACEHOLDER_%d__\n", edits),
100-
})
101-
env.AfterChange()
107+
// We must use an extra subtest layer here, so that we only set up the
108+
// shared env once (otherwise we pay additional overhead and the profiling
109+
// flags don't work).
110+
b.Run("diagnose", func(b *testing.B) {
111+
for i := 0; i < b.N; i++ {
112+
edits := atomic.AddInt64(&editID, 1)
113+
env.EditBuffer(test.file, protocol.TextEdit{
114+
Range: protocol.Range{
115+
Start: protocol.Position{Line: 0, Character: 0},
116+
End: protocol.Position{Line: 1, Character: 0},
117+
},
118+
// Increment the placeholder text, to ensure cache misses.
119+
NewText: fmt.Sprintf("// __REGTEST_PLACEHOLDER_%d__\n", edits),
120+
})
121+
if save {
122+
env.SaveBuffer(test.file)
102123
}
103-
})
124+
env.AfterChange()
125+
}
104126
})
105-
}
127+
})
106128
}

0 commit comments

Comments
 (0)