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

internal/lsp/cache: use persistent map for storing gofiles in the snapshot #382

Closed

Conversation

euroelessar
Copy link
Contributor

@euroelessar euroelessar commented Jun 9, 2022

Use treap (https://en.wikipedia.org/wiki/Treap) as a persistent map to avoid copying s.goFiles across generations.
Maintain an additional s.parseKeysByURIMap to avoid scanning s.goFiles on individual file's content invalidation.

This on average reduces didChange latency on internal codebase from 160ms to 150ms.

In a followup the same approach can be used to avoid copying s.files, s.packages, and s.knownSubdirs.

Updates golang/go#45686

@gopherbot
Copy link
Contributor

This PR (HEAD: 159eb75) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/411554 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: 8c25760) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/411554 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@euroelessar
Copy link
Contributor Author

benchmark shows 40% reduction against ad756c7 on linux/amd64 for DidChange

$ go test -v ./gopls/internal/regtest/bench -run=TestBenchmarkDidChange -didchange_dir=$HOME/go/src/k8s.io/kubernetes -didchange_file=pkg/util/hash/hash.go
Before:
BenchmarkStatistics	     456	   3294187 ns/op	 1235424 B/op	    5611 allocs/op
After:
BenchmarkStatistics	     900	   2007147 ns/op	  829696 B/op	    5409 allocs/op

@gopherbot
Copy link
Contributor

Message from Alan Donovan:

Patch Set 3:

(5 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Alan Donovan:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: d6691bc) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/411554 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ruslan Nigmatullin:

Patch Set 4:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Ruslan Nigmatullin:

Patch Set 4:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@euroelessar euroelessar changed the title gopls: Use persistent map for storing gofiles in the snapshot internal/lsp/cache: use persistent map for storing gofiles in the snapshot Jun 13, 2022
@gopherbot
Copy link
Contributor

This PR (HEAD: fcf814c) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/411554 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: 599c2a3) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/411554 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ruslan Nigmatullin:

Patch Set 6:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Ruslan Nigmatullin:

Patch Set 6:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 9b6e3e1) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/411554 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Alan Donovan:

Patch Set 9:

(17 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Ruslan Nigmatullin:

Patch Set 9:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Ruslan Nigmatullin:

Patch Set 9:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: f0b31fd) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/411554 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: 0e5720f) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/411554 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ruslan Nigmatullin:

Patch Set 11:

(16 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Alan Donovan:

Patch Set 12: Code-Review+2

(5 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: b211d6c) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/411554 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ruslan Nigmatullin:

Patch Set 13:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

euroelessar pushed a commit to euroelessar/tools that referenced this pull request Jun 17, 2022
…pshot

Use treap (https://en.wikipedia.org/wiki/Treap) as a persistent map to avoid copying s.goFiles across generations.
Maintain an additional s.parseKeysByURIMap to avoid scanning s.goFiles on individual file's content invalidation.

This on average reduces didChange latency on internal codebase from 160ms to 150ms.

In a followup the same approach can be generically extended to avoid copying s.files and s.packages.

Updates golang/go#45686

Change-Id: Ic4a9b3c8fb2b66256f224adf9896ddcaaa6865b1
GitHub-Last-Rev: b211d6c
GitHub-Pull-Request: golang#382
@gopherbot
Copy link
Contributor

This PR (HEAD: a284a97) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/411554 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Hyang-Ah Hana Kim:

Patch Set 14:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Ruslan Nigmatullin:

Patch Set 14:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Robert Findley:

Patch Set 14: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 14:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from kokoro:

Patch Set 14:

Kokoro presubmit build starting for golang/tools/gopls-legacy/presubmit
Logs at:
https://source.cloud.google.com/results/invocations/f7b718d5-9141-4c18-9e86-5556cc94950b


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from kokoro:

Patch Set 14: gopls-CI+1

Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/f7b718d5-9141-4c18-9e86-5556cc94950b


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 14: TryBot-Result+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Robert Findley:

Patch Set 14:

(11 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

Ruslan Nigmatullin added 2 commits June 21, 2022 23:52
…pshot

Use treap (https://en.wikipedia.org/wiki/Treap) as a persistent map to avoid copying s.goFiles across generations.
Maintain an additional s.parseKeysByURIMap to avoid scanning s.goFiles on individual file's content invalidation.

This on average reduces didChange latency on internal codebase from 160ms to 150ms.

In a followup the same approach can be generically extended to avoid copying s.files and s.packages.

Updates golang/go#45686

Change-Id: Ic4a9b3c8fb2b66256f224adf9896ddcaaa6865b1
GitHub-Last-Rev: b211d6c
GitHub-Pull-Request: golang#382
cr
Change-Id: If949bf9aac8f60f09d7969feaf6e9f67030400af
@gopherbot
Copy link
Contributor

This PR (HEAD: 0abd257) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/411554 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ruslan Nigmatullin:

Patch Set 16:

(10 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Robert Findley:

Patch Set 16: Run-TryBot+1 Code-Review+2

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 16:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from kokoro:

Patch Set 16:

Kokoro presubmit build starting for golang/tools/gopls-legacy/presubmit
Logs at:
https://source.cloud.google.com/results/invocations/f1285bdc-8ee8-4178-b32a-91b9e255db95


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from kokoro:

Patch Set 16: gopls-CI+1

Kokoro presubmit build finished with status: SUCCESS
Logs at: https://source.cloud.google.com/results/invocations/f1285bdc-8ee8-4178-b32a-91b9e255db95


Please don’t reply on this GitHub thread. Visit golang.org/cl/411554.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Jun 22, 2022
…pshot

Use treap (https://en.wikipedia.org/wiki/Treap) as a persistent map to avoid copying s.goFiles across generations.
Maintain an additional s.parseKeysByURIMap to avoid scanning s.goFiles on individual file's content invalidation.

This on average reduces didChange latency on internal codebase from 160ms to 150ms.

In a followup the same approach can be used to avoid copying s.files, s.packages, and s.knownSubdirs.

Updates golang/go#45686

Change-Id: Ic4a9b3c8fb2b66256f224adf9896ddcaaa6865b1
GitHub-Last-Rev: 0abd257
GitHub-Pull-Request: #382
Reviewed-on: https://go-review.googlesource.com/c/tools/+/411554
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/411554 has been merged.

@gopherbot gopherbot closed this Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants