[tools] gopls/internal/lsp/filecache: increase idle GC period to 6 hours

19 views
Skip to first unread message

Gopher Robot (Gerrit)

unread,
Jul 6, 2023, 4:45:02 PM7/6/23
to Alan Donovan, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Robert Findley, kokoro, Bryan Mills, golang-co...@googlegroups.com

Gopher Robot submitted this change.

View Change



1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: gopls/internal/lsp/filecache/filecache.go
Insertions: 2, Deletions: 2.

@@ -186,7 +186,7 @@
return nil
}

-// The active 1-channel is selectable resettable event
+// The active 1-channel is a selectable resettable event
// indicating recent cache activity.
var active = make(chan struct{}, 1)

@@ -541,7 +541,7 @@
}

// Wait up to the max period,
- // or for cache activity in this process.
+ // or for Set activity in this process.
select {
case <-active:
case <-time.After(maxPeriod):
```

Approvals: Robert Findley: Looks good to me, approved Alan Donovan: Run TryBots; Automatically submit change Gopher Robot: TryBots succeeded
gopls/internal/lsp/filecache: increase idle GC period to 6 hours

This change causes the GC thread to wait for at least the minimum
period (5m), and then up to a maximum period of 6 hours, but
the second wait is interrupted by any Set activity in this process.

This should reduce the GC CPU load of an idle gopls process by 72x,
while still ensuring that the complete cache is scanned more often
than daily.

Fixes golang/go#61049

Change-Id: Ib9defee854130217384024eacdb2876b7c587014
Reviewed-on: https://go-review.googlesource.com/c/tools/+/506816
Reviewed-by: Robert Findley <rfin...@google.com>
Run-TryBot: Alan Donovan <adon...@google.com>
Auto-Submit: Alan Donovan <adon...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M gopls/internal/lsp/filecache/filecache.go
1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/gopls/internal/lsp/filecache/filecache.go b/gopls/internal/lsp/filecache/filecache.go
index 286b458..132fb70 100644
--- a/gopls/internal/lsp/filecache/filecache.go
+++ b/gopls/internal/lsp/filecache/filecache.go
@@ -140,6 +140,12 @@
func Set(kind string, key [32]byte, value []byte) error {
memCache.Set(memKey{kind, key}, value, len(value))

+ // Set the active event to wake up the GC.
+ select {
+ case active <- struct{}{}:
+ default:
+ }
+
iolimit <- struct{}{} // acquire a token
defer func() { <-iolimit }() // release a token

@@ -180,6 +186,10 @@
return nil
}

+// The active 1-channel is a selectable resettable event
+// indicating recent cache activity.
+var active = make(chan struct{}, 1)
+
// writeFileNoTrunc is like os.WriteFile but doesn't truncate until
// after the write, so that racing writes of the same data are idempotent.
func writeFileNoTrunc(filename string, data []byte, perm os.FileMode) error {
@@ -207,17 +217,18 @@

var budget int64 = 1e9 // 1GB

-// SetBudget sets a soft limit on disk usage of files in the cache (in
-// bytes) and returns the previous value. Supplying a negative value
-// queries the current value without changing it.
+// SetBudget sets a soft limit on disk usage of regular files in the
+// cache (in bytes) and returns the previous value. Supplying a
+// negative value queries the current value without changing it.
//
// If two gopls processes have different budgets, the one with the
// lower budget will collect garbage more actively, but both will
// observe the effect.
//
// Even in the steady state, the storage usage reported by the 'du'
-// command may exceed the budget by as much as 50-70% due to the
-// overheads of directories and the effects of block quantization.
+// command may exceed the budget by as much as a factor of 3 due to
+// the overheads of directories and the effects of block quantization,
+// which are especially pronounced for the small index files.
func SetBudget(new int64) (old int64) {
if new < 0 {
return atomic.LoadInt64(&budget)
@@ -387,20 +398,23 @@
func gc(goplsDir string) {
// period between collections
//
- // This was increased from 1 minute as an immediate measure to
- // reduce the CPU cost of gopls when idle, which was around
- // 15% of a core (#61049). A better solution might be to avoid
- // walking the entire tree every period. e.g. walk only the
- // subtree corresponding to this gopls executable every period,
- // and the subtrees for other gopls instances every hour.
- const period = 5 * time.Minute
+ // Originally the period was always 1 minute, but this
+ // consumed 15% of a CPU core when idle (#61049).
+ //
+ // The reason for running collections even when idle is so
+ // that long lived gopls sessions eventually clean up the
+ // caches created by defunct executables.
+ const (
+ minPeriod = 5 * time.Minute // when active
+ maxPeriod = 6 * time.Hour // when idle
+ )

// Sleep statDelay*batchSize between stats to smooth out I/O.
//
// The constants below were chosen using the following heuristics:
// - 1GB of filecache is on the order of ~100-200k files, in which case
- // 100μs delay per file introduces 10-20s of additional walk time, less
- // than the 1m gc period.
+ // 100μs delay per file introduces 10-20s of additional walk time,
+ // less than the minPeriod.
// - Processing batches of stats at once is much more efficient than
// sleeping after every stat (due to OS optimizations).
const statDelay = 100 * time.Microsecond // average delay between stats, to smooth out I/O
@@ -488,7 +502,8 @@
}
files = nil // release memory before sleep

- time.Sleep(period)
+ // Wait unconditionally for the minimum period.
+ time.Sleep(minPeriod)

// Once only, delete all directories.
// This will succeed only for the empty ones,
@@ -524,6 +539,13 @@
log.Printf("deleted %d empty directories", deleted)
}
}
+
+ // Wait up to the max period,
+ // or for Set activity in this process.
+ select {
+ case <-active:
+ case <-time.After(maxPeriod):
+ }
}
}


To view, visit change 506816. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: merged
Gerrit-Project: tools
Gerrit-Branch: master
Gerrit-Change-Id: Ib9defee854130217384024eacdb2876b7c587014
Gerrit-Change-Number: 506816
Gerrit-PatchSet: 3
Gerrit-Owner: Alan Donovan <adon...@google.com>
Gerrit-Reviewer: Alan Donovan <adon...@google.com>
Gerrit-Reviewer: Bryan Mills <bcm...@google.com>
Gerrit-Reviewer: Gopher Robot <go...@golang.org>
Gerrit-Reviewer: Robert Findley <rfin...@google.com>
Gerrit-Reviewer: kokoro <noreply...@google.com>
Gerrit-CC: kokoro <noreply...@google.com>
Reply all
Reply to author
Forward
0 new messages