Golang x/tools/gopls: progress notifications for more operations

gopls version

Build info

command-line-arguments (unknown) @ github.com/BurntSushi/toml@v1.4.1-0.20240526193622-a339e1f7089c h1:pxW6RcqyfI9/kWtOwnv/G+AzdKuy2ZrqINhenH4HyNs= github.com/google/go-cmp@v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/kr/pretty@v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/text@v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/rogpeppe/go-internal@v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= github.com/ryboe/q@v1.0.24 h1:GgnsKqyULXRp6fw9gZOO3ZEsBHmsFVampM1TLWw9uvU= golang.org/x/exp/typeparams@v0.0.0-20241210194714-1829a127f884 h1:1xaZTydL5Gsg78QharTwKfA9FY9CZ1VQj6D/AZEvHR0= golang.org/x/mod@v0.23.0 h1:Zb7khfcRGKk+kqfxFaP5tZqCnDZMjC5VtUBs87Hr6QM= golang.org/x/sync@v0.11.0 h1:GGz8+XQP4FvTTrjZPzNKTMFtSXH80RAzG+5ghFPgK9w= golang.org/x/telemetry@v0.0.0-20241220003058-cc96b6e0d3d9 h1:L2k9GUV2TpQKVRGMjN94qfUMgUwOFimSQ6gipyJIjKw= golang.org/x/text@v0.22.0 h1:bofq7m3/HAFvbF51jz3Q9wLg3jkvSPuiZu/pD1XwgtM= golang.org/x/tools@v0.28.0 => ../ golang.org/x/tools/gopls@(devel) golang.org/x/vuln@v1.1.3 h1:NPGnvPOTgnjBc9HTaUx+nj+EaUYxl5SJOWqaDYGaFYw= honnef.co/go/tools@v0.5.1 h1:4bH5o3b5ZULQ4UrBmP+63W9r7qIkqJClEA9ko5YKx+I= mvdan.cc/gofumpt@v0.7.0 h1:bg91ttqXmi9y2xawvkuMXyvAA/1ZGJqYAEGjXuP0JXU= mvdan.cc/xurls/v2@v2.5.0 h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8= go: go1.24.0

go env

AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/briandillmann/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/briandillmann/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/8j/_m26x9551t9b2zjfxw0d86380000gp/T/go-build2964405487=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/briandillmann/go/src/github.com/golang/tools/gopls/go.mod'
GOMODCACHE='/Users/briandillmann/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/briandillmann/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/briandillmann/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/briandillmann/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/briandillmann/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.0.darwin-arm64/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.0'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

Slowness can be reproduced against the CockroachDB repo, using goto implementation:

git clone git@github.com:cockroachdb/cockroach.git
git checkout 562d26b3c86fbc0fb5363ad2bfeab118c2e08c58
# if there's a better command to first index the codebase, so that latency can be observed on a warm gopls server, add it here.
gopls implementation ./pkg/kv/kvserver/split/decider.go:48:4

What did you see happen?

Goto Implementation can take a long time, even on a warm server.

What did you expect to see?

It's not that I think goto implementation should execute faster, but for operations which take a long time, I would expect progress notifications to be sent, so that there's an indicator that gopls is working on my request.

Original request here: https://gophers.slack.com/archives/CJZH85XCZ/p1739487184239879

Editor and settings

Editor is neovim. Lack of progress notifications on slow requests should be observable for all gopls integrations however.

Logs

No response

Comment From: findleyr

Thanks. I think it is generally reasonable to start progress notifications after a delay, for any operation that could take a long time. In this case, we may want to look into what's taking so long, but we'd welcome a contribution similar to the PR you shared in slack.

Comment From: gabyhelp

Related Issues

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

Comment From: angles-n-daemons

Profile from slow implementation call

Comment From: angles-n-daemons

implementation.txt

^ the above is a .prof file, (wouldn't let me drop it in as .prof)

Comment From: angles-n-daemons

snapshot.MethodSets is the line which required the most time. It took 50+ seconds, roughly 98% of the call time.

https://github.com/golang/tools/blob/master/gopls/internal/golang/implementation.go#L175

Comment From: findleyr

Thank you, @angles-n-daemons. Can you run gopls stats -anon from your workspace and share the output? It contains some information about the dimensions of your repo.

Comment From: angles-n-daemons

Initializing workspace...


done (53.928417083s)
Gathering bug reports...      done (1.872516667s)
Querying memstats...          done (739.390333ms)
Querying workspace stats...   done (74.438ms)
Collecting directory info...  done (4.0448345s)
{
  "DirStats": {
    "Files": 215525,
    "TestdataFiles": 4961,
    "GoFiles": 8250,
    "ModFiles": 6,
    "Dirs": 32482
  },
  "GOARCH": "arm64",
  "GOOS": "darwin",
  "GOPACKAGESDRIVER": "",
  "GOPLSCACHE": "",
  "GoVersion": "go1.24.0",
  "GoplsVersion": "(unknown)",
  "InitialWorkspaceLoadDuration": "53.928417083s",
  "MemStats": {
    "HeapAlloc": 1645693560,
    "HeapInUse": 2872909824,
    "TotalAlloc": 31967301072
  },
  "WorkspaceStats": {
    "Files": {
      "Total": 20273,
      "Largest": 4950165,
      "Errs": 0
    },
    "Views": [
      {
        "GoCommandVersion": "go1.22.8",
        "AllPackages": {
          "Packages": 8717,
          "LargestPackage": 827,
          "CompiledGoFiles": 76634,
          "Modules": 362
        },
        "WorkspacePackages": {
          "Packages": 2037,
          "LargestPackage": 348,
          "CompiledGoFiles": 11977,
          "Modules": 1
        },
        "Diagnostics": 10
      }
    ]
  }
}

Comment From: angles-n-daemons

This is for the CockroachDB repo (edit, this command was run from within the repo):

https://github.com/cockroachdb/cockroach

Comment From: angles-n-daemons

Seems like at the highest level, we'd need to emit progress reports at the Snapshot.forEachPackage function.

I can pass a reporter down from server.Implemetation -> golang.Implementation -> golang.implementation -> Snapshot.MethodSets -> Snapshot.forEachPackage but that would be changing a lot of signatures.

I could also embed a reporter on the Context, but there's a lack of type safety there.

Do you have any thoughts on this?

Comment From: findleyr

Sorry for the delayed response.

Let's start by 1. Passing a (possibly nil) *progress.Tracker through to Snapshot.MethodSets. 2. Encapsulating the "slow operation" logic of Snapshot.Analyze, perhaps as a method on the progress.Tracker.

It's not the type safety that makes me hesitant to stash on the Context--I'm sure we'd catch any bug related to mismatching context. Rather, it's the fact the presence of a tracker affects the behavior of the operation. It would be too easy to pass the context to some other suboperation of Implementation that ends up in forEachPackage, resulting in (for example) confusingly duplicated progress bars.

Comment From: angles-n-daemons

Perfect, yeah that makes sense - I'll try to have some draft out by this week.

Comment From: angles-n-daemons

I've gotten a bit of a draft out, though I'm having trouble getting it to actually report. If anyone is interested, I'm going to put it down for a bit, may come back to it later:

https://github.com/golang/tools/compare/master...angles-n-daemons:progressreporting?expand=1

Comment From: shashank-priyadarshi

@angles-n-daemons @findleyr I can take this up!

Comment From: angles-n-daemons

@shashank-priyadarshi that'd be great, let me know if I can help in any way!