x/tools/gopls: too slow on large package #33531
Comments
|
Some additional info... When compiling the latest (master) locally I see a whole list of |
|
The panic was easy enough to fix, I'll open a PR for that. Now that the panic is gone, I only see a bunch of |
|
Just opened a PR for the panic part of this report: https://go-review.googlesource.com/c/tools/+/189397 |
|
Thanks for reporting this issue and fixing the nil pointer error. I'll investigate this as soon as I get a chance. |
stamblerre
added
the
NeedsInvestigation
|
When I open this directory in VSCode, it seems like it works, but it is just very slow. The output I see for the check command is:
|
stamblerre
changed the title
stamblerre
changed the title
|
@stamblerre almost, but I'm also getting some I think your staying below the 1024 limit because of the worker in the telemetry package having a limit of 1000. The actual code that is causing the issue is here. If I update this loop so it uses a semaphore with a limit below 256, the problem goes away: But I'm not sure if you think this is the correct place to fix this problem? And also I'm not sure if 250 is low enough as it's pretty close to 256 and I'm not sure what other files could possibly be open at this point in the code? Example output$gopls -rpc.trace -v check aws/resource_aws_rds_cluster.go
2019/08/08 09:07:19 Info:137.714259ms for GOROOT= GOPATH=/Users/svanharmelen/GoCode GO111MODULE=on PWD=/Users/svanharmelen/GoCode/src/github.com/terraform-providers/terraform-provider-aws go "list" "-e" "-json" "-compiled=true" "-test=true" "-export=false" "-deps=true" "-find=false" "--" "builtin", stderr: <<>>
2019/08/08 09:07:19 Info:Build info
----------
golang.org/x/tools/gopls v0.1.3
golang.org/x/tools/gopls@(devel)
golang.org/x/sync@v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
golang.org/x/tools@v0.0.0-20190723021737-8bb11ff117ca => ../
golang.org/x/xerrors@v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc=
|
svanharmelen
changed the title
|
Of course the semaphore could also just be a simple |
|
Limiting the number of open files to the correct OS-specific values does sound like a reasonable solution, though perhaps we could build tag the constants per-OS to avoid unnecessarily constraining Linux. If you'd like to contribute this fix, please feel free to send me a CL for it. |
|
Though, I'm actually surprised that this fix works. The number of files being parsed simultaneously should be constrained here, with a maximum limit of 20. |
|
Ah... I think I found it... It seems the semaphore is used, after the file is being read: So all the reads are done at (more or less) the same time. And while the telemetry worker has a throttling side effect, it's limit is too high for the MacOS limit. Do you agree this is the root cause? If so the fix is pretty straight forward (just swapping a few lines), so I can make a PR for that. |
|
Oh wow, thanks for catching that. I can't believe we missed it in review. Please do send a PR if you can! |
|
Created a PR just now: https://go-review.googlesource.com/c/tools/+/189437 |
stamblerre
changed the title
|
Now that the |
stamblerre
changed the title
|
Ah, thanks for updating the issue! I just asked about this on Slack as well (didn't saw you updated this already) 👍 |
gopherbot
added
the
Tools
|
One issue here is almost certainly the LSP protocol forcing us to send a notification per file. More often than not, diagnostic updates will be package-oriented, and even then the availibility of diagnostics is more a function of the architecture of For example, type checking in response to a change results in diagnostics for a package and its reverse dependencies. The results from type checking are not incremental. When type checking is complete, all diagnostics for all files are known at that point in time: having to send separate diagnostic notifications per file is just overhead. This per-file approach also places overhead on the LSP client as well. In @stamblerre - any thoughts on this? Have you discussed a change/addition to the LSP spec on this point? |
|
Checking back in - |
stamblerre
changed the title
|
I would say that it's OK to close this one. At least for me things work reasonably ATM... |
|
@stamblerre |
|
Reopening to investigate if we've regressed. |
findleyr
added
the
gopls/performance
|
The issue is no longer happening on https://github.com/terraform-providers/terraform-provider-aws repository as it has been split into multiple smaller packages recently. |
|
Thanks @kamilturek; just going through old performance bugs now that gopls@v0.12.0-pre.1 has been released (a major rewrite for memory usage and performance). Closing this as resolved. |
What version of Go are you using (
go version)?Does this issue reproduce with the latest release?
Yes, it does.
What operating system and processor architecture are you using (
go env)?go envOutputWhat did you do?
What did you expect to see?
A successful check without errors. What let me to start debugging is actually that completion started to take very long (several seconds). I assume this will at least be part of the reason, but regardless these errors are unexpected.
What did you see instead?
➜ workspace git clone https://github.com/terraform-providers/terraform-provider-aws Cloning into 'terraform-provider-aws'... remote: Enumerating objects: 10, done. remote: Counting objects: 100% (10/10), done. remote: Compressing objects: 100% (8/8), done. remote: Total 103996 (delta 3), reused 8 (delta 2), pack-reused 103986 Receiving objects: 100% (103996/103996), 90.54 MiB | 22.82 MiB/s, done. Resolving deltas: 100% (67391/67391), done. Checking out files: 100% (6432/6432), done. ➜ workspace cd terraform-provider-aws ➜ terraform-provider-aws git:(master) gopls -rpc.trace -v check aws/resource_aws_rds_cluster.go 2019/08/07 22:13:22 Info:Build info ---------- golang.org/x/tools/gopls v0.1.2 golang.org/x/tools/gopls@v0.1.3 h1:CB5ECiPysqZrwxcyRjN+exyZpY0gODTZvNiqQi3lpeo= golang.org/x/sync@v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU= golang.org/x/tools@v0.0.0-20190710153321-831012c29e42 h1:4IOeC7p+OItq3+O5BWkcmVu2uBe3jekXau5S4QZX9DU= Go info ------- go version go1.12.7 darwin/amd64 GOARCH="amd64" GOBIN="/Users/svanharmelen/GoCode/bin" GOCACHE="/Users/svanharmelen/Library/Caches/go-build" GOEXE="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOOS="darwin" GOPATH="/Users/svanharmelen/GoCode" GOPROXY="" GORACE="" GOROOT="/usr/local/Cellar/go/1.12.7/libexec" GOTMPDIR="" GOTOOLDIR="/usr/local/Cellar/go/1.12.7/libexec/pkg/tool/darwin_amd64" GCCGO="gccgo" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="/Users/svanharmelen/workspace/terraform-provider-aws/go.mod" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/bz/cgd8_ttn1tx_6f8mt_ys06qjgxjxn1/T/go-build545681017=/tmp/go-build -gno-record-gcc-switches -f no-common" 2019/08/07 22:13:35 Error:unable to check package for file:///Users/svanharmelen/workspace/terraform-provider-aws/aws/resource_aws_rds_cluster.go: failed to cache file file:///Users/svanharmelen/workspace/terraform-provider-aws/aws/datasync.go: no AST for file:///Users/svanharmelen/workspace/terraform-provider-aws/aws/datasync.go 2019/08/07 22:13:38 Error:unable to check package for file:///Users/svanharmelen/workspace/terraform-provider-aws/aws/resource_aws_rds_cluster.go: failed to cache file file:///Users/svanharmelen/workspace/terraform-provider-aws/aws/datasync.go: no AST for file:///Users/svanharmelen/workspace/terraform-provider-aws/aws/datasync.go /Users/svanharmelen/workspace/terraform-provider-aws/aws/resource_aws_rds_cluster.go: file:///Users/svanharmelen/workspace/terraform-provider-aws/aws/resource_aws_rds_cluster.go is not part of a packageThe text was updated successfully, but these errors were encountered: