From 0f3f6367893e69d6986bef6ad19f47e8d834a13f Mon Sep 17 00:00:00 2001
From: Joshua Baehring <josh.baehring@gmail.com>
Date: Thu, 5 Sep 2024 22:33:16 -0400
Subject: [PATCH] [scudo] Update secondary cache time-based release logic.

Secondary cache entries are now released to the OS from least recent
to most recent entries. This helps to avoid unnecessary scans of the
cache since entries ready to be released (specifically, entries
that are considered old relative to the configurable release interval)
will always be at the tail of the list of committed entries by the LRU
ordering. For this same reason, the OldestTime variable is no longer
needed to indicate when releases are necessary so it has been removed.
---
 compiler-rt/lib/scudo/standalone/secondary.h | 52 ++++++++++++--------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h
index 1a232b9b9fb2d..766874ef25525 100644
--- a/compiler-rt/lib/scudo/standalone/secondary.h
+++ b/compiler-rt/lib/scudo/standalone/secondary.h
@@ -244,6 +244,7 @@ class MapAllocatorCache {
     // The cache is initially empty
     LRUHead = CachedBlock::InvalidEntry;
     LRUTail = CachedBlock::InvalidEntry;
+    LastUnreleasedEntry = CachedBlock::InvalidEntry;
 
     // Available entries will be retrieved starting from the beginning of the
     // Entries array
@@ -318,9 +319,10 @@ class MapAllocatorCache {
         }
         CachedBlock PrevEntry = Quarantine[QuarantinePos];
         Quarantine[QuarantinePos] = Entry;
-        if (OldestTime == 0)
-          OldestTime = Entry.Time;
         Entry = PrevEntry;
+        // Update the entry time to reflect the time that the
+        // quarantined memory is placed in the Entries array
+        Entry.Time = Time;
       }
 
       // All excess entries are evicted from the cache
@@ -331,9 +333,6 @@ class MapAllocatorCache {
       }
 
       insert(Entry);
-
-      if (OldestTime == 0)
-        OldestTime = Entry.Time;
     } while (0);
 
     for (MemMapT &EvictMemMap : EvictionMemMaps)
@@ -532,6 +531,9 @@ class MapAllocatorCache {
       Entries[LRUHead].Prev = static_cast<u16>(FreeIndex);
     }
 
+    if (LastUnreleasedEntry == CachedBlock::InvalidEntry)
+      LastUnreleasedEntry = static_cast<u16>(FreeIndex);
+
     Entries[FreeIndex] = Entry;
     Entries[FreeIndex].Next = LRUHead;
     Entries[FreeIndex].Prev = CachedBlock::InvalidEntry;
@@ -549,6 +551,9 @@ class MapAllocatorCache {
 
     Entries[I].invalidate();
 
+    if (I == LastUnreleasedEntry)
+      LastUnreleasedEntry = Entries[LastUnreleasedEntry].Prev;
+
     if (I == LRUHead)
       LRUHead = Entries[I].Next;
     else
@@ -590,35 +595,37 @@ class MapAllocatorCache {
     }
   }
 
-  void releaseIfOlderThan(CachedBlock &Entry, u64 Time) REQUIRES(Mutex) {
-    if (!Entry.isValid() || !Entry.Time)
-      return;
-    if (Entry.Time > Time) {
-      if (OldestTime == 0 || Entry.Time < OldestTime)
-        OldestTime = Entry.Time;
-      return;
-    }
+  inline void release(CachedBlock &Entry) {
+    DCHECK(Entry.Time != 0);
     Entry.MemMap.releaseAndZeroPagesToOS(Entry.CommitBase, Entry.CommitSize);
     Entry.Time = 0;
   }
 
   void releaseOlderThan(u64 Time) EXCLUDES(Mutex) {
     ScopedLock L(Mutex);
-    if (!EntriesCount || OldestTime == 0 || OldestTime > Time)
+    if (!EntriesCount)
       return;
-    OldestTime = 0;
-    for (uptr I = 0; I < Config::getQuarantineSize(); I++)
-      releaseIfOlderThan(Quarantine[I], Time);
-    for (uptr I = 0; I < Config::getEntriesArraySize(); I++)
-      releaseIfOlderThan(Entries[I], Time);
-  }
 
+    for (uptr I = 0; I < Config::getQuarantineSize(); I++) {
+      CachedBlock &ReleaseEntry = Quarantine[I];
+      if (!ReleaseEntry.isValid() || !ReleaseEntry.Time ||
+          ReleaseEntry.Time > Time)
+        continue;
+      release(ReleaseEntry);
+    }
+
+    // Release oldest entries first by releasing from decommit base
+    while (LastUnreleasedEntry != CachedBlock::InvalidEntry &&
+           Entries[LastUnreleasedEntry].Time <= Time) {
+      release(Entries[LastUnreleasedEntry]);
+      LastUnreleasedEntry = Entries[LastUnreleasedEntry].Prev;
+    }
+  }
   HybridMutex Mutex;
   u32 EntriesCount GUARDED_BY(Mutex) = 0;
   u32 QuarantinePos GUARDED_BY(Mutex) = 0;
   atomic_u32 MaxEntriesCount = {};
   atomic_uptr MaxEntrySize = {};
-  u64 OldestTime GUARDED_BY(Mutex) = 0;
   atomic_s32 ReleaseToOsIntervalMs = {};
   u32 CallsToRetrieve GUARDED_BY(Mutex) = 0;
   u32 SuccessfulRetrieves GUARDED_BY(Mutex) = 0;
@@ -633,6 +640,9 @@ class MapAllocatorCache {
   u16 LRUTail GUARDED_BY(Mutex) = 0;
   // The AvailableHead is the top of the stack of available entries
   u16 AvailableHead GUARDED_BY(Mutex) = 0;
+  // The LastUnreleasedEntry is the least recently used entry that has not
+  // been released
+  u16 LastUnreleasedEntry GUARDED_BY(Mutex) = 0;
 };
 
 template <typename Config> class MapAllocator {