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 · 13 comments
Open

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

hyangah opened this issue Mar 31, 2021 · 13 comments
Assignees
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.

Comments

@hyangah
Copy link
Contributor

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 Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Mar 31, 2021
@gopherbot gopherbot added this to the Unreleased milestone Mar 31, 2021
@pjweinb
Copy link

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 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

@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 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 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 commented Apr 2, 2021

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

@pjweinb
Copy link

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 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 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 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 commented Apr 6, 2021

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

@stamblerre stamblerre modified the milestones: Unreleased, gopls/unplanned Apr 6, 2021
@hyangah
Copy link
Contributor Author

hyangah commented Apr 4, 2022

From off-line chat: @pjweinb @findleyr
Another aspect to check is whether gopls can produces semantic tokens without type checking. For semantic token, we need just parse result.

vscode's behavior around range-limited semantic token requests described in #45313 (comment) seems strange, I will take a look.

@pjweinb
Copy link

pjweinb commented Apr 4, 2022

In gopls, semantic tokens use type information if is available. I see no reason not to. Semantic token processing seems fast enough, once the initial workspace load is done. Small files take about 2 ms (not including the marshaling time), and large files (60-80KB) take about 13ms, all measured on my laptop. It won't do files over 100KB, forcing vscode (or other clients) to fall back to range requests (because of the Apr 6, 2021 results shown above).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants