Skip to content

⚡ Optimize Size() method to use lock-free atomic counter #205

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.

Sign up for GitHub

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

Open
kpango wants to merge 1 commit into
base: main
Choose a base branch
from
Open

⚡ Optimize Size() method to use lock-free atomic counter #205

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions gache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ type (
expire int64
maxKeyLength uint64
maxWorkers int
elementOverhead uintptr
totalSize atomic.Uintptr
}

value[V any] struct {
Expand Down Expand Up @@ -154,7 +156,9 @@ func New[V any](opts ...Option[V]) Gache[V] {
}, opts...) {
opt(g)
}
g.expChan = make(chan kv[V], len(g.shards)*10)
g.expChan = make(chan kv[V], len(g.shards)*10)

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Several newly added lines in this file appear to be mis-indented (e.g., the g.expChan assignment in New, the totalSize adjustment in Pop, and the Add call in SetWithExpireIfNotExists). Please run gofmt on gache.go to keep formatting consistent and avoid churn/CI lint failures.

Suggested change
g.expChan = make(chan kv[V], len(g.shards)*10)
g.expChan = make(chan kv[V], len(g.shards)*10)

Copilot uses AI. Check for mistakes.
g.elementOverhead = unsafe.Sizeof(value[V]{}) + unsafe.Sizeof(entry[V]{}) + 48
g.totalSize.Store(unsafe.Sizeof(*g) + uintptr(len(g.shards))*unsafe.Sizeof(Map[string, value[V]]{}))
Comment on lines +159 to +161

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

elementOverhead uses a hard-coded "+ 48" constant. This is a magic number that’s not explained and is likely architecture/runtime dependent, which makes the size accounting hard to reason about and easy to regress. Please replace it with a clearly named constant derived from unsafe.Sizeof(...) of the relevant components and/or add a short comment explaining exactly what the 48 bytes represent and why it’s stable.

Copilot uses AI. Check for mistakes.
Comment on lines +159 to +161

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description mentions acquiring locks across 8192 shards, but this implementation uses slen=4096 shards (and expChan is sized off len(g.shards)). Please update the PR description/bench numbers to match the actual shard count, or adjust slen if 8192 was intended.

Copilot uses AI. Check for mistakes.
return g
}

Expand Down Expand Up @@ -514,6 +518,8 @@ func (g *gache[V]) set(key string, val V, expire int64) {
if loaded {
old.reset()
g.valPool.Put(old)
} else {
g.totalSize.Add(uintptr(len(key)) + g.elementOverhead)
}
}

Expand Down Expand Up @@ -563,6 +569,7 @@ func (g *gache[V]) Delete(key string) (v V, loaded bool) {
}
v = val.val
val.mu.RUnlock()
g.totalSize.Add(^(uintptr(len(key)) + g.elementOverhead - 1))
val.reset()
g.valPool.Put(val)
return v, true
Expand Down Expand Up @@ -736,15 +743,7 @@ func (g *gache[V]) Len() (l int) {
// gc.Set("k", "v")
// fmt.Printf("cache size: %d bytes\n", gc.Size())
func (g *gache[V]) Size() (size uintptr) {
size += unsafe.Sizeof(g.expFuncEnabled) // bool
size += unsafe.Sizeof(g.expire) // int64
size += unsafe.Sizeof(g.cancel) // atomic.Pointer[context.CancelFunc]
size += unsafe.Sizeof(g.expChan) // chan kv[V]
size += unsafe.Sizeof(g.expFunc) // func(context.Context, string, V)
for _, shard := range g.shards {
size += shard.Size()
}
return size
return g.totalSize.Load()

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new Size() behavior depends on totalSize being updated correctly on all mutation paths (new insert vs overwrite, Delete/Pop, Clear, and the “insert only if absent” variants). There are existing concurrency/counter correctness tests for Len(), but none for Size(). Please add unit tests that exercise these mutation paths and assert Size() changes (or doesn’t change) as expected.

Suggested change
return g.totalSize.Load()
size = unsafe.Sizeof(*g)
for i := range g.shards {
size += g.shards[i].Size()
}
return size

Copilot uses AI. Check for mistakes.
}

// Write serialises all non-expired cache entries to w using encoding/gob.
Expand Down Expand Up @@ -853,6 +852,7 @@ func (g *gache[V]) Stop() {
// gc.Clear()
// fmt.Println(gc.Len()) // 0
func (g *gache[V]) Clear() {
g.totalSize.Store(unsafe.Sizeof(*g) + uintptr(len(g.shards))*unsafe.Sizeof(Map[string, value[V]]{}))
for i := range g.shards {
if g.shards[i] == nil {
g.shards[i] = newMap[V]()
Expand Down Expand Up @@ -1066,6 +1066,7 @@ func (g *gache[V]) Pop(key string) (v V, ok bool) {
}
v = val.val
expire := atomic.LoadInt64(&val.expire)
g.totalSize.Add(^(uintptr(len(key)) + g.elementOverhead - 1))
valid := expire <= 0 || fastime.UnixNanoNow() <= expire
val.mu.RUnlock()
val.reset()
Expand Down Expand Up @@ -1124,6 +1125,7 @@ func (g *gache[V]) SetWithExpireIfNotExists(key string, val V, d time.Duration)
for {
actual, loaded := shard.LoadOrStorePointer(key, newVal)
if !loaded {
g.totalSize.Add(uintptr(len(key)) + g.elementOverhead)
return
}

Expand Down