From d3d5e2af0e79408b93ee3bedf0c66c8f6a64f3e5 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Tue, 27 Aug 2024 10:51:27 -0700 Subject: [PATCH] use new unique package to reduce memory duplication of interned labels when multiple remote writes are configured Signed-off-by: Callum Styan --- go.mod | 2 +- storage/remote/intern.go | 30 +++++++++++++++--------------- storage/remote/intern_test.go | 15 ++++++++------- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/go.mod b/go.mod index 50d560bc3..8496227ab 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/prometheus/prometheus -go 1.21.0 +go 1.23.0 toolchain go1.22.5 diff --git a/storage/remote/intern.go b/storage/remote/intern.go index 23047acd9..36067c671 100644 --- a/storage/remote/intern.go +++ b/storage/remote/intern.go @@ -20,6 +20,7 @@ package remote import ( "sync" + "unique" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" @@ -35,22 +36,20 @@ var noReferenceReleases = promauto.NewCounter(prometheus.CounterOpts{ type pool struct { mtx sync.RWMutex - pool map[string]*entry + pool map[unique.Handle[string]]*entry } type entry struct { refs atomic.Int64 - - s string } -func newEntry(s string) *entry { - return &entry{s: s} +func newEntry() *entry { + return &entry{} } func newPool() *pool { return &pool{ - pool: map[string]*entry{}, + pool: map[unique.Handle[string]]*entry{}, } } @@ -60,27 +59,28 @@ func (p *pool) intern(s string) string { } p.mtx.RLock() - interned, ok := p.pool[s] + h := unique.Make(s) + interned, ok := p.pool[h] p.mtx.RUnlock() if ok { interned.refs.Inc() - return interned.s + return s } p.mtx.Lock() defer p.mtx.Unlock() - if interned, ok := p.pool[s]; ok { + if interned, ok := p.pool[h]; ok { interned.refs.Inc() - return interned.s + return s } - - p.pool[s] = newEntry(s) - p.pool[s].refs.Store(1) + p.pool[h] = newEntry() + p.pool[h].refs.Store(1) return s } func (p *pool) release(s string) { p.mtx.RLock() - interned, ok := p.pool[s] + h := unique.Make(s) + interned, ok := p.pool[h] p.mtx.RUnlock() if !ok { @@ -98,5 +98,5 @@ func (p *pool) release(s string) { if interned.refs.Load() != 0 { return } - delete(p.pool, s) + delete(p.pool, h) } diff --git a/storage/remote/intern_test.go b/storage/remote/intern_test.go index 917c42e91..5d94b058b 100644 --- a/storage/remote/intern_test.go +++ b/storage/remote/intern_test.go @@ -22,6 +22,7 @@ import ( "fmt" "testing" "time" + "unique" "github.com/stretchr/testify/require" ) @@ -30,7 +31,7 @@ func TestIntern(t *testing.T) { interner := newPool() testString := "TestIntern" interner.intern(testString) - interned, ok := interner.pool[testString] + interned, ok := interner.pool[unique.Make(testString)] require.True(t, ok) require.Equal(t, int64(1), interned.refs.Load(), fmt.Sprintf("expected refs to be 1 but it was %d", interned.refs.Load())) @@ -41,13 +42,13 @@ func TestIntern_MultiRef(t *testing.T) { testString := "TestIntern_MultiRef" interner.intern(testString) - interned, ok := interner.pool[testString] + interned, ok := interner.pool[unique.Make(testString)] require.True(t, ok) require.Equal(t, int64(1), interned.refs.Load(), fmt.Sprintf("expected refs to be 1 but it was %d", interned.refs.Load())) interner.intern(testString) - interned, ok = interner.pool[testString] + interned, ok = interner.pool[unique.Make(testString)] require.True(t, ok) require.Equal(t, int64(2), interned.refs.Load(), fmt.Sprintf("expected refs to be 2 but it was %d", interned.refs.Load())) @@ -58,13 +59,13 @@ func TestIntern_DeleteRef(t *testing.T) { testString := "TestIntern_DeleteRef" interner.intern(testString) - interned, ok := interner.pool[testString] + interned, ok := interner.pool[unique.Make(testString)] require.True(t, ok) require.Equal(t, int64(1), interned.refs.Load(), fmt.Sprintf("expected refs to be 1 but it was %d", interned.refs.Load())) interner.release(testString) - _, ok = interner.pool[testString] + _, ok = interner.pool[unique.Make(testString)] require.False(t, ok) } @@ -73,7 +74,7 @@ func TestIntern_MultiRef_Concurrent(t *testing.T) { testString := "TestIntern_MultiRef_Concurrent" interner.intern(testString) - interned, ok := interner.pool[testString] + interned, ok := interner.pool[unique.Make(testString)] require.True(t, ok) require.Equal(t, int64(1), interned.refs.Load(), fmt.Sprintf("expected refs to be 1 but it was %d", interned.refs.Load())) @@ -84,7 +85,7 @@ func TestIntern_MultiRef_Concurrent(t *testing.T) { time.Sleep(time.Millisecond) interner.mtx.RLock() - interned, ok = interner.pool[testString] + interned, ok = interner.pool[unique.Make(testString)] interner.mtx.RUnlock() require.True(t, ok) require.Equal(t, int64(1), interned.refs.Load(), fmt.Sprintf("expected refs to be 1 but it was %d", interned.refs.Load()))