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: enable semantic tokens by default #45313

Open
hyangah opened this issue Mar 31, 2021 · 11 comments
Open

x/tools/gopls: enable semantic tokens by default #45313

hyangah opened this issue Mar 31, 2021 · 11 comments
Assignees

Comments

@hyangah
Copy link
Contributor

@hyangah hyangah commented Mar 31, 2021

Is there any concern or prerequisite to meet before turning on the feature by default?

cc @pjweinb @stamblerre

@gopherbot gopherbot added this to the Unreleased milestone Mar 31, 2021
@pjweinb
Copy link

@pjweinb pjweinb commented Mar 31, 2021

as far as i know, the primary question is whether people would be unpleasantly surprised. The LSP response message for a full file is pretty large, but clients could just ask for ranges if it matters.

@OneOfOne
Copy link
Contributor

@OneOfOne OneOfOne commented Apr 2, 2021

As long as there's a way to turn it off.

I have it on by default but I have 2 projects that I have to turn off for.

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 2, 2021

@OneOfOne: Do you mind sharing the reason that you turn off semantic tokens for these projects? If there is a problem or bug, we'd be happy to look into it.

@OneOfOne
Copy link
Contributor

@OneOfOne OneOfOne commented Apr 2, 2021

@stamblerre it has massive files that vscode lags badly with, probably related to golang/vscode-go#1387

@pjweinb
Copy link

@pjweinb pjweinb commented Apr 2, 2021

how big are your files? and when you turn off semantic tokens, does vscode do syntax coloring? (or does it say the files are too large for syntax coloring?)

@OneOfOne
Copy link
Contributor

@OneOfOne OneOfOne commented Apr 2, 2021

~4000-5000 loc, it does syntax coloring and quite fast compared to gopls's semantic tokens.

@pjweinb
Copy link

@pjweinb pjweinb commented Apr 2, 2021

Thanks. I suspect we need a limit on how big a file we generate semantic tokens for. How many bytes are your files?

@OneOfOne
Copy link
Contributor

@OneOfOne OneOfOne commented Apr 2, 2021

this is my full config:

"[go]": {
	"editor.insertSpaces": false,
	"editor.formatOnSave": true,
	"editor.formatOnSaveMode": "file",
	"editor.codeActionsOnSave": {
		"source.organizeImports": true,
		"source.fixAll": true
	}
},
"go.lintOnSave": "off",
"go.testFlags": ["-v", "-count=1", "-benchtime=5s", "-timeout=5m"],
"go.coverMode": "atomic",
"go.coverShowCounts": true,
"go.editorContextMenuCommands": {
	"toggleTestFile": true,
	"addTags": true,
	"removeTags": false,
	"testAtCursor": false,
	"testFile": false,
	"testPackage": true,
	"generateTestForFunction": true,
	"generateTestForFile": false,
	"generateTestForPackage": false,
	"addImport": false,
	"testCoverage": false,
	"playground": false,
	"debugTestAtCursor": true
},
"go.addTags": {
	"tags": "json",
	"options": "json=omitempty",
	"promptForTags": false,
	"transform": "camelcase"
},
"go.coverOnSingleTest": false,
"go.toolsManagement.checkForUpdates": "off",
"go.enableCodeLens": {
	"references": false,
	"runtest": false
},
"go.toolsEnvVars": {
	"GO111MODULE": "auto",
	"GOFLAGS": "-tags=go2"
},
"go.delveConfig": {
	"dlvLoadConfig": {
		"followPointers": true,
		"maxVariableRecurse": 1,
		"maxStringLen": 128,
		"maxArrayValues": 64,
		"maxStructFields": -1
	},
	"apiVersion": 2,
	"showGlobalVariables": false
},
"go.languageServerExperimentalFeatures": {
	"diagnostics": true
},
//"go.languageServerFlags": ["-rpc.trace"],
"gopls": {
	"formatting.gofumpt": true,
	"analyses": {
		"ST1000": false,
		"ST1003": false,
		"SA5001": false,
		"nilness": true,
		"unusedwrite": true,

		// too much noise
		"fieldalignment": false,
		"shadow": false,
		"composites": false
	},
	"ui.codelenses": {
		"generate": true,
		"regenerate_cgo": true,
		"test": true,
		"vendor": true,
		"tidy": true,
		"upgrade_dependency": true,
		"gc_details": true
	},
	"ui.diagnostic.annotations": {
		"bounds": true,
		"inline": true,
		"escape": false,
		"nil": false
	},
	"ui.diagnostic.staticcheck": true,
	"ui.completion.usePlaceholders": true,
	"ui.navigation.importShortcut": "Definition",
	"ui.completion.matcher": "Fuzzy",
	"ui.navigation.symbolMatcher": "Fuzzy",
	"ui.navigation.symbolStyle": "Dynamic",
	"ui.completion.completionBudget": "500ms",
	"ui.semanticTokens": true,
	"ui.diagnostic.experimentalDiagnosticsDelay": "300ms",

	"build.experimentalPackageCacheKey": true,
	"build.directoryFilters": [
		"-node_modules",
		"-data"
	]
}

@pjweinb
Copy link

@pjweinb pjweinb commented Apr 4, 2021

@OneOfOne: thank you for the report. I agree that very large files are a problem. Presently gopls tells vscode that it supports both full-file semantic token requests and range-limited semantic token requests, and vscode requests both, although having gotten the full one the range-limited one adds no information.

Changing gopls to say that it only supports range-limited semantic token requests ought to help, but vscode continues to send both sorts of requests. Responding only to the range-limited ones speeds things up a lot, but if one scrolls quickly through a file, there's a slight delay in displaying the semantic token information. (scrolling earlier in the file is ok; vscode seems to remember the semantic tokens.)

An alternate approach would be to respond to full requests only for files that aren't too large. This would rely on clients (particularly vscode) not getting confused by errors on some semantic token requests. It's not clear what to use for 'large'.

@pjweinb pjweinb self-assigned this Apr 5, 2021
@pjweinb
Copy link

@pjweinb pjweinb commented Apr 6, 2021

Here's a tiny bit of data (from my laptop, a MacBookPro from a few years ago). These are end-end numbers (from request to receiving the LSP response), the 10 files with the most semantic tokens from the Go source distribution. The columns are

of semantic tokens, byte size of file, elapsed time in nanoseconds, tokens per millisecond

47916 387707 80140074 598 /Users/pjw/goroot/src/vendor/golang.org/x/text/unicode/norm/tables13.0.0.go
53045 953604 120104915 442 /Users/pjw/goroot/src/cmd/compile/internal/ssa/opGen.go
55544 429798 108746014 511 /Users/pjw/goroot/src/cmd/compile/internal/ssa/rewritePPC64.go
56431 443746 102833507 549 /Users/pjw/goroot/src/cmd/compile/internal/ssa/rewriteS390X.go
60301 330745 97937471 616 /Users/pjw/goroot/src/cmd/compile/internal/test/constFold_test.go
63675 499705 170635479 374 /Users/pjw/goroot/src/cmd/compile/internal/ssa/rewriteARM.go
73028 649009 193933596 377 /Users/pjw/goroot/src/cmd/compile/internal/test/testdata/arithConst_test.go
82454 609723 153604334 537 /Users/pjw/goroot/src/cmd/compile/internal/ssa/rewritegeneric.go
95731 746331 174103583 550 /Users/pjw/goroot/src/cmd/compile/internal/ssa/rewriteARM64.go
108801 865209 238304719 457 /Users/pjw/goroot/src/cmd/compile/internal/ssa/rewriteAMD64.go

@pjweinb
Copy link

@pjweinb pjweinb commented Apr 6, 2021

memo to self: always Preview comments. sorry about the big font.

@stamblerre stamblerre removed this from the Unreleased milestone Apr 6, 2021
@stamblerre stamblerre added this to the gopls/unplanned milestone Apr 6, 2021
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
5 participants