[tools] gopls/internal/cache: avoid unnecessary mod tidy invalidation
2 views
Skip to first unread message
Pontus Leitzler (Gerrit)
Dec 15, 2025, 3:53:29 PM (23 hours ago) Dec 15
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Pontus Leitzler has uploaded the change for review![]( "Open in Gerrit")
Commit message
gopls/internal/cache: avoid unnecessary mod tidy invalidation
When a file is closed (didClose) we get a transition from overlay
to disk which gopls always assumed was a "save" event that
invalidated mod tidy handle. That in hand triggered an unnecessary
"go mod tidy" invocation.
Browsing files in VSCode for example could cause a lot of didClose
event on unchanged files, and if the module is large each
invocation takes a while.
With this change we now check if the Identity changed when
determine if a file was saved, from the perspective of go mod tidy.
If the Identity hasn't changed, the content hasn't changed.
Change-Id: Ied0a89b4cb51a8cfec5c3ee04308944a30b36fa1
Change diff
diff --git a/gopls/internal/cache/snapshot.go b/gopls/internal/cache/snapshot.go
index 75ac34a..3e50bb5 100644
--- a/gopls/internal/cache/snapshot.go
+++ b/gopls/internal/cache/snapshot.go
@@ -1999,6 +1999,16 @@
// are both overlays, and if the current FileHandle is saved while the original
// FileHandle was not saved.
func fileWasSaved(originalFH, currentFH file.Handle) bool {
+ if originalFH == nil || currentFH == nil {
+ return true // should not happen for valid file handles
+ }
+
+ // If the file identity has not changed, the content has not changed.
+ // Therefore, the "saved" state (from the perspective of go mod tidy)
+ // has not changed.
+ if originalFH.Identity() == currentFH.Identity() {
+ return false
+ }
c, ok := currentFH.(*overlay)
if !ok || c == nil {
return true
Change information
Files:
- M gopls/internal/cache/snapshot.go
Change size: S
Delta: 1 file changed, 10 insertions(+), 0 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Pontus Leitzler (Gerrit)
Dec 15, 2025, 4:08:21 PM (22 hours ago) Dec 15
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Pontus Leitzler voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Pontus Leitzler (Gerrit)
Dec 15, 2025, 5:03:28 PM (22 hours ago) Dec 15
to goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
Pontus Leitzler voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Pontus Leitzler (Gerrit)
Dec 15, 2025, 5:34:46 PM (21 hours ago) Dec 15
to goph...@pubsubhelper.golang.org, Go LUCI, golang-co...@googlegroups.com
Pontus Leitzler voted and added 1 comment![]( "Open in Gerrit")
Votes added by Pontus Leitzler
| Commit-Queue | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Pontus Leitzler . resolved
Failed check was to due flaky unrelated test (reproduced on master with "go test . -run TestCheckGoModDeps -count=10000 -failfast").
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
10:50 AM (4 hours ago) 10:50 AM
to Pontus Leitzler, goph...@pubsubhelper.golang.org, Robert Findley, Go LUCI, golang-co...@googlegroups.com
Attention needed from Pontus Leitzler
Alan Donovan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alan Donovan
| Code-Review | +2 |
1 comment
Patchset-level comments
Alan Donovan . resolved
Thanks for this optimization. The change looks sound to me. It would be nice if we could have a test, but I don't see any precedent for an integration test that checks for internal server events (e.g. trace events, go command invocations, etc) whose presence or absence can be asserted as a side effect of a given sequence of client operations. cc'ing Rob in case he remembers one.
Related details
Attention is currently required from:
- Pontus Leitzler
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Knyszek (Gerrit)
11:16 AM (3 hours ago) 11:16 AM
to Pontus Leitzler, goph...@pubsubhelper.golang.org, Alan Donovan, Robert Findley, Go LUCI, golang-co...@googlegroups.com
Attention needed from Pontus Leitzler
Michael Knyszek voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Pontus Leitzler
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
1:42 PM (1 hour ago) 1:42 PM
to Pontus Leitzler, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Michael Knyszek, Robert Findley, Go LUCI, golang-co...@googlegroups.com
Alan Donovan submitted the change![]( "Open in Gerrit")
Change information
Commit message:
gopls/internal/cache: avoid unnecessary mod tidy invalidation
When a file is closed (didClose) we get a transition from overlay
to disk which gopls always assumed was a "save" event that
invalidated mod tidy handle. That in hand triggered an unnecessary
"go mod tidy" invocation.
Browsing files in VSCode for example could cause a lot of didClose
event on unchanged files, and if the module is large each
invocation takes a while.
With this change we now check if the Identity changed when
determine if a file was saved, from the perspective of go mod tidy.
If the Identity hasn't changed, the content hasn't changed.
Change-Id: Ied0a89b4cb51a8cfec5c3ee04308944a30b36fa1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/730220
Reviewed-by: Michael Knyszek <mkny...@google.com>
Reviewed-by: Alan Donovan <adon...@google.com>
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Files:
- M gopls/internal/cache/snapshot.go
Change size: S
Delta: 1 file changed, 10 insertions(+), 0 deletions(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +1 by Michael Knyszek, +2 by Alan Donovan
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages