perf(lsp): filter servers before searching PATH#3370
Open
TheJhyeFactor wants to merge 1 commit into
Open
Conversation
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the Contributor License Agreement (CLA) and hereby sign the CLA. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #3072.
What I looked at
The reported PATH scan maps to
Manager.startServer: for every bundled LSP, Crush calledexec.LookPathbefore checking whether that server handled the file being viewed or edited. With roughly 300 server definitions and a wide PATH, a single file event could turn into thousands of filesystem checks for unrelated language servers.The CPU profile confirmed that this is filesystem work rather than LSP startup work. In the lookup-first benchmark, 90.5% of CPU samples were in raw syscalls reached through
os.Stat, and 99.3% of allocated space flowed throughexec.LookPath.What changed
This doesn't add a long-lived PATH cache, so installing an LSP during a session keeps the existing retry behaviour.
Benchmark
Apple M4, darwin/arm64, Go 1.26.5. The benchmark used 300 synthetic server definitions, 70 empty PATH directories, and one server matching a Go file. Five runs per order:
That isolates a roughly 165x reduction in wall time, 238x less allocated memory, and 96x fewer allocations for the discovery scan. This is a controlled worst-case benchmark of LSP discovery, not an end-to-end application benchmark.
Validation
go test -race -failfast ./...go build -race ./...golangci-lint run --path-mode=abs --config=.golangci.yml --timeout=5mgo mod tidywith no diffExisting work
#3096 makes the unavailable-server retry delay configurable. This takes a different route: it avoids probing irrelevant servers in the first place, while leaving the current retry semantics unchanged.