diff --git a/internal/cache/clockpro.go b/internal/cache/clockpro.go index 94102a274b..b81e7141d7 100644 --- a/internal/cache/clockpro.go +++ b/internal/cache/clockpro.go @@ -71,9 +71,7 @@ func (h Handle) Get() []byte { // Release releases the reference to the cache entry. func (h Handle) Release() { - if h.value != nil { - h.value.release() - } + h.value.release() } type shard struct { @@ -176,7 +174,7 @@ func (c *shard) Set(id uint64, fileNum base.DiskFileNum, offset uint64, value *V // cache entry was a test page c.sizeTest -= e.size c.countTest-- - c.metaDel(e) + c.metaDel(e).release() c.metaCheck(e) e.size = int64(len(value.buf)) @@ -233,37 +231,71 @@ func (c *shard) Delete(id uint64, fileNum base.DiskFileNum, offset uint64) { return } - c.mu.Lock() - defer c.mu.Unlock() + var deletedValue *Value + func() { + c.mu.Lock() + defer c.mu.Unlock() - e := c.blocks.Get(k) - if e == nil { - return - } - c.metaEvict(e) - - c.checkConsistency() + e := c.blocks.Get(k) + if e == nil { + return + } + deletedValue = c.metaEvict(e) + c.checkConsistency() + }() + // Now that the mutex has been dropped, release the reference which will + // potentially free the memory associated with the previous cached value. + deletedValue.release() } // EvictFile evicts all of the cache values for the specified file. func (c *shard) EvictFile(id uint64, fileNum base.DiskFileNum) { + fkey := key{fileKey{id, fileNum}, 0} + for c.evictFileRun(fkey) { + // Sched switch to give another goroutine an opportunity to acquire the + // shard mutex. + runtime.Gosched() + } +} + +func (c *shard) evictFileRun(fkey key) (moreRemaining bool) { + // If most of the file's blocks are held in the block cache, evicting all + // the blocks may take a while. We don't want to block the entire cache + // shard, forcing concurrent readers until we're finished. We drop the mutex + // every [blocksPerMutexAcquisition] blocks to give other goroutines an + // opportunity to make progress. + const blocksPerMutexAcquisition = 5 c.mu.Lock() - defer c.mu.Unlock() - fkey := key{fileKey{id, fileNum}, 0} + // Releasing a value may result in free-ing it back to the memory allocator. + // This can have a nontrivial cost that we'd prefer to not pay while holding + // the shard mutex, so we collect the evicted values in a local slice and + // only release them in a defer after dropping the cache mutex. + var obsoleteValuesAlloc [blocksPerMutexAcquisition]*Value + obsoleteValues := obsoleteValuesAlloc[:0] + defer func() { + if !moreRemaining { + c.checkConsistency() + } + c.mu.Unlock() + for _, v := range obsoleteValues { + v.release() + } + }() + blocks := c.files.Get(fkey) if blocks == nil { - return + return false } - for b, n := blocks, (*entry)(nil); ; b = n { + for b, n := blocks, (*entry)(nil); len(obsoleteValues) < cap(obsoleteValues); b = n { n = b.fileLink.next - c.metaEvict(b) + obsoleteValues = append(obsoleteValues, c.metaEvict(b)) if b == n { - break + return false } } - - c.checkConsistency() + // Exhausted blocksPerMutexAcquisition. + return true } func (c *shard) Free() { @@ -274,7 +306,7 @@ func (c *shard) Free() { // metaCheck call when the "invariants" build tag is specified. for c.handHot != nil { e := c.handHot - c.metaDel(c.handHot) + c.metaDel(c.handHot).release() e.free() } @@ -359,12 +391,14 @@ func (c *shard) metaAdd(key key, e *entry) bool { // Remove the entry from the cache. This removes the entry from the blocks map, // the files map, and ensures that hand{Hot,Cold,Test} are not pointing at the -// entry. -func (c *shard) metaDel(e *entry) { +// entry. Returns the deleted value that must be released, if any. +func (c *shard) metaDel(e *entry) (deletedValue *Value) { if value := e.peekValue(); value != nil { value.ref.trace("metaDel") } - e.setValue(nil) + // Remove the pointer to the value. + deletedValue = e.val + e.val = nil c.blocks.Delete(e.key) if entriesGoAllocated { @@ -396,6 +430,7 @@ func (c *shard) metaDel(e *entry) { } else { c.files.Put(fkey, next) } + return deletedValue } // Check that the specified entry is not referenced by the cache. @@ -455,7 +490,7 @@ func (c *shard) metaCheck(e *entry) { } } -func (c *shard) metaEvict(e *entry) { +func (c *shard) metaEvict(e *entry) (evictedValue *Value) { switch e.ptype { case etHot: c.sizeHot -= e.size @@ -467,9 +502,10 @@ func (c *shard) metaEvict(e *entry) { c.sizeTest -= e.size c.countTest-- } - c.metaDel(e) + evictedValue = c.metaDel(e) c.metaCheck(e) e.free() + return evictedValue } func (c *shard) evict() { @@ -564,7 +600,7 @@ func (c *shard) runHandTest() { if c.coldTarget < 0 { c.coldTarget = 0 } - c.metaDel(e) + c.metaDel(e).release() c.metaCheck(e) e.free() } diff --git a/internal/cache/entry.go b/internal/cache/entry.go index 28a3a3f293..a49fde6b7e 100644 --- a/internal/cache/entry.go +++ b/internal/cache/entry.go @@ -139,9 +139,7 @@ func (e *entry) setValue(v *Value) { } old := e.val e.val = v - if old != nil { - old.release() - } + old.release() } func (e *entry) peekValue() *Value { diff --git a/internal/cache/value.go b/internal/cache/value.go index c5c967aff3..6d2cae15e5 100644 --- a/internal/cache/value.go +++ b/internal/cache/value.go @@ -40,7 +40,7 @@ func (v *Value) acquire() { } func (v *Value) release() { - if v.ref.release() { + if v != nil && v.ref.release() { v.free() } }