Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: add ability to set deep completion Budget via LSP Configuration #36142

Closed
myitcv opened this issue Dec 14, 2019 · 8 comments
Closed

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Dec 14, 2019

What version of Go are you using (go version)?

$ go version
go version devel +da4d58587e Sat Dec 7 15:57:30 2019 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20191213221258-04c2e8eff935
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.1.8-0.20191213221258-04c2e8eff935

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/govim/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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build191688178=/tmp/go-build -gno-record-gcc-switches"

What did you do?

See below for background. We very quickly established this is related to the deep completion Budget being exceeded. Hence #36142 (comment) and the updated title.

Background

In govim test complete_deep_fuzzy we verify that deep fuzzy completions work.

The test is as simple as ensuring that the input file:

package main

func main() {
	var a struct {
		b struct {
			crikey int
		}
		d int
	}
	var x int
	x = a.ck
}

when completed after a.ck results in:

package main

func main() {
	var a struct {
		b struct {
			crikey int
		}
		d int
	}
	var x int
	x = a.b.crikey
}

However, over 50% of the time we're seeing this test fail because no completion items are returned.

Here are the gopls logs from a failing and passing run:

Much like #35638 this is a timing/race related issue and hard to reproduce standalone. Generally I find this fails when my machine is under high stress.

The test can be run on a machine that uses Docker via:

go test -exec="dockexec govim/govim:go1.13.4_vim_v8.1.2253_v1 -t -e VIM_FLAVOR=vim" -count=1 -v -run "TestScripts/scenario_default/complete_deep_fuzzy" ./cmd/govim

(download of Docker image may initially take some time)

What did you expect to see?

A consistently passing test.

What did you see instead?

Intermittent failures.


cc @stamblerre @muirdm

@myitcv myitcv added this to the gopls v1.0 milestone Dec 14, 2019
@gopherbot gopherbot added the Tools label Dec 14, 2019
@muirdm

This comment has been minimized.

Copy link

@muirdm muirdm commented Dec 14, 2019

It might be the completion budget. If it runs out of the (default) 100ms budget, it will give up on deep completions. You can try disabling the "Budget" completion option by setting it to 0, but it isn't currently exposed over LSP.

@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Dec 14, 2019

The log files attached to the initial comment were incorrect: now fixed.

Also worth noting is that if the initial go.mod is "tidy" then I've currently been unable to reproduce this.

More explicitly, if the initial go.mod is:

module mod.com

Then the test fails as reported above.

However if it is:

module mod.com

go 1.13

it has not yet failed.

@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Dec 14, 2019

You can try disabling the "Budget" completion option by setting it to 0

Thanks @muirdm - hadn't thought about that. Will try now.

@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Dec 14, 2019

@muirdm - brilliant observation, thank you. Setting the Budget to 0 has indeed "fixed" the test failures. Hence for integration tests we will need the ability to do this.

I'll change the title of this description to instead be a request to expose gopls configuration to set this value.

@myitcv myitcv changed the title x/tools/gopls: (fuzzy) completion not deterministic x/tools/gopls: add ability to set deep completion Budget via LSP Configuration Dec 14, 2019
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Dec 17, 2019

Change https://golang.org/cl/211584 mentions this issue: internal/lsp: expose option to disable timeouts for completion

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Dec 17, 2019

@myitcv: does the above CL work for you?

@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Dec 18, 2019

@myitcv myitcv reopened this Dec 18, 2019
@myitcv

This comment has been minimized.

Copy link
Member Author

@myitcv myitcv commented Dec 20, 2019

All fixes for this have now landed.

@myitcv myitcv closed this Dec 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.