Skip to content

Commit c35476f

Browse files
committed
gopls: commands to start/stop profiling, and a new benchmark metric
Total CPU used by gopls is a critical metric for our users, yet was not previously captured in any benchmark. This change uses the new pprof parsing utilities added in CL 507885 to instrument a cpu_seconds benchmark metric, for now just associated with the DidChange benchmark. This is achieved via new LSP commands that start and stop a profile. The benchmark runner uses these commands to bracket the critical section of the benchmark, then parses the resulting profile for its total sampled CPU. Additionally, the benchmark runner is updated to actually check for the existence of the custom command before instrumenting the new metric. This allows it to be compatible with testing older versions of gopls. The same technique is adopted for memstats metrics. I only instrumented BenchmarkDidChange, because the profile file schema is getting truly out of hand. I'll try to simplify it before instrumenting all the other benchmarks, but want to do that in a separate CL. For golang/go#60926 Change-Id: Ia082bad49e8d30c567a7c07e050511d49b93738b Reviewed-on: https://go-review.googlesource.com/c/tools/+/508449 gopls-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Alan Donovan <adonovan@google.com> Run-TryBot: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
1 parent ec9bc53 commit c35476f

File tree

14 files changed

+356
-23
lines changed

14 files changed

+356
-23
lines changed

gopls/doc/commands.md

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -364,14 +364,14 @@ Args:
364364
// Optional: the address (including port) for the debug server to listen on.
365365
// If not provided, the debug server will bind to "localhost:0", and the
366366
// full debug URL will be contained in the result.
367-
//
367+
//
368368
// If there is more than one gopls instance along the serving path (i.e. you
369369
// are using a daemon), each gopls instance will attempt to start debugging.
370370
// If Addr specifies a port, only the daemon will be able to bind to that
371371
// port, and each intermediate gopls instance will fail to start debugging.
372372
// For this reason it is recommended not to specify a port (or equivalently,
373373
// to specify ":0").
374-
//
374+
//
375375
// If the server was already debugging this field has no effect, and the
376376
// result will contain the previously configured debug URL(s).
377377
"Addr": string,
@@ -385,7 +385,7 @@ Result:
385385
// The URLs to use to access the debug servers, for all gopls instances in
386386
// the serving path. For the common case of a single gopls instance (i.e. no
387387
// daemon), this will be exactly one address.
388-
//
388+
//
389389
// In the case of one or more gopls instances forwarding the LSP to a daemon,
390390
// URLs will contain debug addresses for each server in the serving path, in
391391
// serving order. The daemon debug address will be the last entry in the
@@ -396,6 +396,48 @@ Result:
396396
}
397397
```
398398

399+
### **start capturing a profile of gopls' execution.**
400+
Identifier: `gopls.start_profile`
401+
402+
Start a new pprof profile. Before using the resulting file, profiling must
403+
be stopped with a corresponding call to StopProfile.
404+
405+
This command is intended for internal use only, by the gopls benchmark
406+
runner.
407+
408+
Args:
409+
410+
```
411+
struct{}
412+
```
413+
414+
Result:
415+
416+
```
417+
struct{}
418+
```
419+
420+
### **stop an ongoing profile.**
421+
Identifier: `gopls.stop_profile`
422+
423+
This command is intended for internal use only, by the gopls benchmark
424+
runner.
425+
426+
Args:
427+
428+
```
429+
struct{}
430+
```
431+
432+
Result:
433+
434+
```
435+
{
436+
// File is the profile file name.
437+
"File": string,
438+
}
439+
```
440+
399441
### **Run test(s) (legacy)**
400442
Identifier: `gopls.test`
401443

gopls/doc/generate.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,11 @@ func structDoc(fields []*commandmeta.Field, level int) string {
465465
if fld.Doc != "" && level == 0 {
466466
doclines := strings.Split(fld.Doc, "\n")
467467
for _, line := range doclines {
468-
fmt.Fprintf(&b, "%s\t// %s\n", indent, line)
468+
text := ""
469+
if line != "" {
470+
text = " " + line
471+
}
472+
fmt.Fprintf(&b, "%s\t//%s\n", indent, text)
469473
}
470474
}
471475
tag := strings.Split(fld.JSONTag, ",")[0]

gopls/internal/lsp/command.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"os/exec"
1717
"path/filepath"
1818
"runtime"
19+
"runtime/pprof"
1920
"sort"
2021
"strings"
2122
"time"
@@ -843,6 +844,48 @@ func (c *commandHandler) StartDebugging(ctx context.Context, args command.Debugg
843844
return result, nil
844845
}
845846

847+
func (c *commandHandler) StartProfile(ctx context.Context, args command.StartProfileArgs) (result command.StartProfileResult, _ error) {
848+
file, err := os.CreateTemp("", "gopls-profile-*")
849+
if err != nil {
850+
return result, fmt.Errorf("creating temp profile file: %v", err)
851+
}
852+
853+
c.s.ongoingProfileMu.Lock()
854+
defer c.s.ongoingProfileMu.Unlock()
855+
856+
if c.s.ongoingProfile != nil {
857+
file.Close() // ignore error
858+
return result, fmt.Errorf("profile already started (for %q)", c.s.ongoingProfile.Name())
859+
}
860+
861+
if err := pprof.StartCPUProfile(file); err != nil {
862+
file.Close() // ignore error
863+
return result, fmt.Errorf("starting profile: %v", err)
864+
}
865+
866+
c.s.ongoingProfile = file
867+
return result, nil
868+
}
869+
870+
func (c *commandHandler) StopProfile(ctx context.Context, args command.StopProfileArgs) (result command.StopProfileResult, _ error) {
871+
c.s.ongoingProfileMu.Lock()
872+
defer c.s.ongoingProfileMu.Unlock()
873+
874+
prof := c.s.ongoingProfile
875+
c.s.ongoingProfile = nil
876+
877+
if prof == nil {
878+
return result, fmt.Errorf("no ongoing profile")
879+
}
880+
881+
pprof.StopCPUProfile()
882+
if err := prof.Close(); err != nil {
883+
return result, fmt.Errorf("closing profile file: %v", err)
884+
}
885+
result.File = prof.Name()
886+
return result, nil
887+
}
888+
846889
// Copy of pkgLoadConfig defined in internal/lsp/cmd/vulncheck.go
847890
// TODO(hyangah): decide where to define this.
848891
type pkgLoadConfig struct {

gopls/internal/lsp/command/command_gen.go

Lines changed: 40 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gopls/internal/lsp/command/interface.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,21 @@ type Interface interface {
145145
// address.
146146
StartDebugging(context.Context, DebuggingArgs) (DebuggingResult, error)
147147

148+
// StartProfile: start capturing a profile of gopls' execution.
149+
//
150+
// Start a new pprof profile. Before using the resulting file, profiling must
151+
// be stopped with a corresponding call to StopProfile.
152+
//
153+
// This command is intended for internal use only, by the gopls benchmark
154+
// runner.
155+
StartProfile(context.Context, StartProfileArgs) (StartProfileResult, error)
156+
157+
// StopProfile: stop an ongoing profile.
158+
//
159+
// This command is intended for internal use only, by the gopls benchmark
160+
// runner.
161+
StopProfile(context.Context, StopProfileArgs) (StopProfileResult, error)
162+
148163
// RunGovulncheck: Run govulncheck.
149164
//
150165
// Run vulnerability check (`govulncheck`).
@@ -327,6 +342,30 @@ type DebuggingResult struct {
327342
URLs []string
328343
}
329344

345+
// StartProfileArgs holds the arguments to the StartProfile command.
346+
//
347+
// It is a placeholder for future compatibility.
348+
type StartProfileArgs struct {
349+
}
350+
351+
// StartProfileResult holds the result of the StartProfile command.
352+
//
353+
// It is a placeholder for future compatibility.
354+
type StartProfileResult struct {
355+
}
356+
357+
// StopProfileArgs holds the arguments to the StopProfile command.
358+
//
359+
// It is a placeholder for future compatibility.
360+
type StopProfileArgs struct {
361+
}
362+
363+
// StopProfileResult holds the result to the StopProfile command.
364+
type StopProfileResult struct {
365+
// File is the profile file name.
366+
File string
367+
}
368+
330369
type ResetGoModDiagnosticsArgs struct {
331370
URIArg
332371

gopls/internal/lsp/fake/editor.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,16 @@ func (e *Editor) initialize(ctx context.Context) error {
305305
return nil
306306
}
307307

308+
// HasCommand reports whether the connected server supports the command with the given ID.
309+
func (e *Editor) HasCommand(id string) bool {
310+
for _, command := range e.serverCapabilities.ExecuteCommandProvider.Commands {
311+
if command == id {
312+
return true
313+
}
314+
}
315+
return false
316+
}
317+
308318
// makeWorkspaceFolders creates a slice of workspace folders to use for
309319
// this editing session, based on the editor configuration.
310320
func makeWorkspaceFolders(sandbox *Sandbox, paths []string) (folders []protocol.WorkspaceFolder) {

gopls/internal/lsp/regtest/wrappers.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,41 @@ func (e *Env) ExecuteCommand(params *protocol.ExecuteCommandParams, result inter
370370
}
371371
}
372372

373+
// StartProfile starts a CPU profile with the given name, using the
374+
// gopls.start_profile custom command. It calls t.Fatal on any error.
375+
//
376+
// The resulting stop function must be called to stop profiling (using the
377+
// gopls.stop_profile custom command).
378+
func (e *Env) StartProfile() (stop func() string) {
379+
// TODO(golang/go#61217): revisit the ergonomics of these command APIs.
380+
//
381+
// This would be a lot simpler if we generated params constructors.
382+
args, err := command.MarshalArgs(command.StartProfileArgs{})
383+
if err != nil {
384+
e.T.Fatal(err)
385+
}
386+
params := &protocol.ExecuteCommandParams{
387+
Command: command.StartProfile.ID(),
388+
Arguments: args,
389+
}
390+
var result command.StartProfileResult
391+
e.ExecuteCommand(params, &result)
392+
393+
return func() string {
394+
stopArgs, err := command.MarshalArgs(command.StopProfileArgs{})
395+
if err != nil {
396+
e.T.Fatal(err)
397+
}
398+
stopParams := &protocol.ExecuteCommandParams{
399+
Command: command.StopProfile.ID(),
400+
Arguments: stopArgs,
401+
}
402+
var result command.StopProfileResult
403+
e.ExecuteCommand(stopParams, &result)
404+
return result.File
405+
}
406+
}
407+
373408
// InlayHints calls textDocument/inlayHints for the given path, calling t.Fatal on
374409
// any error.
375410
func (e *Env) InlayHints(path string) []protocol.InlayHint {

gopls/internal/lsp/server.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ package lsp
1010
import (
1111
"context"
1212
"fmt"
13+
"os"
1314
"sync"
1415

1516
"golang.org/x/tools/gopls/internal/lsp/cache"
@@ -109,6 +110,11 @@ type Server struct {
109110
// report with an error message.
110111
criticalErrorStatusMu sync.Mutex
111112
criticalErrorStatus *progress.WorkDone
113+
114+
// Track an ongoing CPU profile created with the StartProfile command and
115+
// terminated with the StopProfile command.
116+
ongoingProfileMu sync.Mutex
117+
ongoingProfile *os.File // if non-nil, an ongoing profile is writing to this file
112118
}
113119

114120
func (s *Server) workDoneProgressCancel(ctx context.Context, params *protocol.WorkDoneProgressCancelParams) error {

gopls/internal/lsp/source/api_json.go

Lines changed: 16 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)