From fdb0da65a1fa4c7cd8f054e37b9b26e54c36af47 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Tue, 28 Feb 2023 22:13:34 -0500 Subject: [PATCH] gopls/internal/regtest/bench: add a benchmark for diagnosing a change Add a benchmark that measures how long it takes gopls to fully complete diagnostics from a change. Change-Id: Icb19ccf658ebe6604337283cb6e13773c3b95f18 Reviewed-on: https://go-review.googlesource.com/c/tools/+/472875 Reviewed-by: Alan Donovan Run-TryBot: Robert Findley gopls-CI: kokoro TryBot-Result: Gopher Robot --- .../internal/regtest/bench/didchange_test.go | 75 +++++++++++++++---- 1 file changed, 59 insertions(+), 16 deletions(-) diff --git a/gopls/internal/regtest/bench/didchange_test.go b/gopls/internal/regtest/bench/didchange_test.go index bb639ee15ac..3a7cb10b6a7 100644 --- a/gopls/internal/regtest/bench/didchange_test.go +++ b/gopls/internal/regtest/bench/didchange_test.go @@ -6,41 +6,47 @@ package bench import ( "fmt" + "sync/atomic" "testing" + "time" + "golang.org/x/tools/gopls/internal/lsp/fake" "golang.org/x/tools/gopls/internal/lsp/protocol" ) +// Use a global edit counter as bench function may execute multiple times, and +// we want to avoid cache hits. Use time.Now to also avoid cache hits from the +// shared file cache. +var editID int64 = time.Now().UnixNano() + +var didChangeTests = []struct { + repo string + file string +}{ + {"istio", "pkg/fuzz/util.go"}, + {"kubernetes", "pkg/controller/lookup_cache.go"}, + {"kuma", "api/generic/insights.go"}, + {"pkgsite", "internal/frontend/server.go"}, + {"starlark", "starlark/eval.go"}, + {"tools", "internal/lsp/cache/snapshot.go"}, +} + // BenchmarkDidChange benchmarks modifications of a single file by making // synthetic modifications in a comment. It controls pacing by waiting for the // server to actually start processing the didChange notification before // proceeding. Notably it does not wait for diagnostics to complete. func BenchmarkDidChange(b *testing.B) { - tests := []struct { - repo string - file string - }{ - {"istio", "pkg/fuzz/util.go"}, - {"kubernetes", "pkg/controller/lookup_cache.go"}, - {"kuma", "api/generic/insights.go"}, - {"pkgsite", "internal/frontend/server.go"}, - {"starlark", "starlark/eval.go"}, - {"tools", "internal/lsp/cache/snapshot.go"}, - } - - for _, test := range tests { - edits := 0 // bench function may execute multiple times + for _, test := range didChangeTests { b.Run(test.repo, func(b *testing.B) { env := getRepo(b, test.repo).sharedEnv(b) env.OpenFile(test.file) - env.AfterChange() // Insert the text we'll be modifying at the top of the file. env.EditBuffer(test.file, protocol.TextEdit{NewText: "// __REGTEST_PLACEHOLDER_0__\n"}) env.AfterChange() b.ResetTimer() for i := 0; i < b.N; i++ { - edits++ + edits := atomic.AddInt64(&editID, 1) env.EditBuffer(test.file, protocol.TextEdit{ Range: protocol.Range{ Start: protocol.Position{Line: 0, Character: 0}, @@ -54,3 +60,40 @@ func BenchmarkDidChange(b *testing.B) { }) } } + +func BenchmarkDiagnoseChange(b *testing.B) { + for _, test := range didChangeTests { + b.Run(test.repo, func(b *testing.B) { + // Use a new env to avoid the diagnostic delay: we want to measure how + // long it takes to produce the diagnostics. + env := repos[test.repo].newEnv(b, "diagnoseChange", fake.EditorConfig{ + Settings: map[string]interface{}{ + "diagnosticsDelay": "0s", + }, + }) + env.OpenFile(test.file) + // Insert the text we'll be modifying at the top of the file. + env.EditBuffer(test.file, protocol.TextEdit{NewText: "// __REGTEST_PLACEHOLDER_0__\n"}) + env.AfterChange() + b.ResetTimer() + + // We must use an extra subtest layer here, so that we only set up the + // shared env once (otherwise we pay additional overhead and the profiling + // flags don't work). + b.Run("diagnose", func(b *testing.B) { + for i := 0; i < b.N; i++ { + edits := atomic.AddInt64(&editID, 1) + env.EditBuffer(test.file, protocol.TextEdit{ + Range: protocol.Range{ + Start: protocol.Position{Line: 0, Character: 0}, + End: protocol.Position{Line: 1, Character: 0}, + }, + // Increment the placeholder text, to ensure cache misses. + NewText: fmt.Sprintf("// __REGTEST_PLACEHOLDER_%d__\n", edits), + }) + env.AfterChange() + } + }) + }) + } +}