lib/bytesutil: optimize internStringMap cleanup

- Make it in a separate goroutine, so it doesn't slow down regular intern() calls.

- Do not lock internStringMap.mutableLock during the cleanup routine, since now
  it is called from a single goroutine and reads only the readonly part of the internStringMap.
  This should prevent from locking regular intern() calls for new strings during cleanups.

- Add jitter to the cleanup interval in order to prevent from synchornous increase in resource usage
  during cleanups.

- Run the cleanup twice per -internStringCacheExpireDuration . This should save 30% CPU time spent
  on cleanup comparing to the previous code, which was running the cleanup 3 times per -internStringCacheExpireDuration .
This commit is contained in:
Aliaksandr Valialkin 2024-06-13 15:02:55 +02:00
parent ac16d1dc1b
commit faf07fbc67
No known key found for this signature in database
GPG key ID: 52C003EE2BCDB9EB

View file

@ -8,6 +8,7 @@ import (
"time" "time"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/fasttime" "github.com/VictoriaMetrics/VictoriaMetrics/lib/fasttime"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/timeutil"
) )
var ( var (
@ -20,14 +21,11 @@ var (
) )
type internStringMap struct { type internStringMap struct {
mu sync.Mutex mutableLock sync.Mutex
mutable map[string]string mutable map[string]string
mutableReads uint64 mutableReads uint64
readonly atomic.Pointer[map[string]internStringMapEntry] readonly atomic.Pointer[map[string]internStringMapEntry]
cleanupInterval uint64
nextCleanupTime atomic.Uint64
} }
type internStringMapEntry struct { type internStringMapEntry struct {
@ -41,8 +39,15 @@ func newInternStringMap() *internStringMap {
} }
readonly := make(map[string]internStringMapEntry) readonly := make(map[string]internStringMapEntry)
m.readonly.Store(&readonly) m.readonly.Store(&readonly)
m.cleanupInterval = uint64(cacheExpireDuration.Seconds() / 3)
m.nextCleanupTime.Store(fasttime.UnixTimestamp() + m.cleanupInterval) go func() {
cleanupInterval := timeutil.AddJitterToDuration(*cacheExpireDuration) / 2
ticker := time.NewTicker(cleanupInterval)
for range ticker.C {
m.cleanup()
}
}()
return m return m
} }
@ -54,21 +59,16 @@ func (m *internStringMap) intern(s string) string {
if isSkipCache(s) { if isSkipCache(s) {
return strings.Clone(s) return strings.Clone(s)
} }
currentTime := fasttime.UnixTimestamp()
if currentTime >= m.nextCleanupTime.Load() {
m.nextCleanupTime.Store(currentTime + m.cleanupInterval)
m.cleanup()
}
readonly := m.getReadonly() readonly := m.getReadonly()
e, ok := readonly[s] e, ok := readonly[s]
if ok { if ok {
// Fast path - the string has been found in readonly map // Fast path - the string has been found in readonly map.
return e.s return e.s
} }
// Slower path - search for the string in mutable map under the lock. // Slower path - search for the string in mutable map under the lock.
m.mu.Lock() m.mutableLock.Lock()
sInterned, ok := m.mutable[s] sInterned, ok := m.mutable[s]
if !ok { if !ok {
// Verify whether s has been already registered by concurrent goroutines in m.readonly // Verify whether s has been already registered by concurrent goroutines in m.readonly
@ -88,7 +88,7 @@ func (m *internStringMap) intern(s string) string {
m.migrateMutableToReadonlyLocked() m.migrateMutableToReadonlyLocked()
m.mutableReads = 0 m.mutableReads = 0
} }
m.mu.Unlock() m.mutableLock.Unlock()
return sInterned return sInterned
} }
@ -111,9 +111,6 @@ func (m *internStringMap) migrateMutableToReadonlyLocked() {
} }
func (m *internStringMap) cleanup() { func (m *internStringMap) cleanup() {
m.mu.Lock()
defer m.mu.Unlock()
readonly := m.getReadonly() readonly := m.getReadonly()
currentTime := fasttime.UnixTimestamp() currentTime := fasttime.UnixTimestamp()
needCleanup := false needCleanup := false