Skip to content

x/tools/gopls: progress notifications for more operations #71751

Open
@angles-n-daemons

Description

@angles-n-daemons

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

Activity

added
goplsIssues related to the Go language server, gopls.
ToolsThis label describes issues relating to any tools in the x/tools repository.
on Feb 14, 2025
added this to the Unreleased milestone on Feb 14, 2025
findleyr

findleyr commented on Feb 14, 2025

@findleyr
Member

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.

added
FeatureRequestIssues asking for a new feature that does not need a proposal.
on Feb 14, 2025
angles-n-daemons

angles-n-daemons commented on Feb 22, 2025

@angles-n-daemons
Author

Image

Profile from slow implementation call

angles-n-daemons

angles-n-daemons commented on Feb 22, 2025

@angles-n-daemons
Author

implementation.txt

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

angles-n-daemons

angles-n-daemons commented on Feb 22, 2025

@angles-n-daemons
Author

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

findleyr

findleyr commented on Feb 24, 2025

@findleyr
Member

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.

angles-n-daemons

angles-n-daemons commented on Feb 25, 2025

@angles-n-daemons
Author
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
      }
    ]
  }
}
angles-n-daemons

angles-n-daemons commented on Feb 25, 2025

@angles-n-daemons
Author

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

https://github.com/cockroachdb/cockroach

angles-n-daemons

angles-n-daemons commented on Feb 25, 2025

@angles-n-daemons
Author

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?

findleyr

findleyr commented on Mar 13, 2025

@findleyr
Member

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.

angles-n-daemons

angles-n-daemons commented on Mar 24, 2025

@angles-n-daemons
Author

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

angles-n-daemons

angles-n-daemons commented on Apr 25, 2025

@angles-n-daemons
Author

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

shashank-priyadarshi

shashank-priyadarshi commented on Apr 30, 2025

@shashank-priyadarshi

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    FeatureRequestIssues asking for a new feature that does not need a proposal.ToolsThis label describes issues relating to any tools in the x/tools repository.goplsIssues related to the Go language server, gopls.help wanted

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @angles-n-daemons@gopherbot@findleyr@shashank-priyadarshi@gabyhelp

        Issue actions

          x/tools/gopls: progress notifications for more operations · Issue #71751 · golang/go