[scudo] Try to release pages after unlocking the TSDs

[Chia-hungDuan]

Summary

This increases the parallelism and the usage of TSDs

[cferris]

compiler-rt/lib/scudo/standalone/primary32.h

236–237

It's not clear why that this is equivalent to the new os release in combined.h. Is this call completely replaced by that call in all circumstances?

[Chia-hungDuan]

Add more comment

compiler-rt/lib/scudo/standalone/primary32.h

236–237

The call stacks before:

• deallocate() /* combined.h */
  • deallocate() /* local_cache.h */
    • drain() /* local_cache.h */ /* call pushBlocks() if NeedToDrainCache */
      • pushBlocks() /* primary32/64.h */
        • releaseToOSMaybe() /* primary32/64.h */

The call stack after:

• deallocate() /* combined.h */
  • deallocate() /* local_cache.h */
    • drain() /* local_cache.h */ /* call pushBlocks() if NeedToDrainCache */
      • pushBlocks() /* primary32/64.h */
  • releaseToOSMaybe() /* primary32/64.h */ /* call this if NeedToDrainCache */

pushBlocks is the only place we will do proactive page release and we still have that call except that it's detached from pushBlocks

[cferris]

That makes sense to me.

LGTM.

compiler-rt/lib/scudo/standalone/combined.h

void quarantineOrDeallocateChunk(Options Options, void *TaggedPtr,
     if (BypassQuarantine) {
       if (allocatorSupportsMemoryTagging<Config>())
         Ptr = untagPointer(Ptr);
       void *BlockBegin = getBlockBegin(Ptr, &NewHeader);
       const uptr ClassId = NewHeader.ClassId;
       if (LIKELY(ClassId)) {
         bool UnlockRequired;
         auto *TSD = TSDRegistry.getTSDAndLock(&UnlockRequired);
+        const bool CacheDrained =
             TSD->getCache().deallocate(ClassId, BlockBegin);
         if (UnlockRequired)
           TSD->unlock();
+        // When we have drained some blocks back to the Primary from TSD, that
+        // implies that we may have the chance to release some pages as well.
+        // Note that in order not to block other thread's accessing the TSD,
+        // release the TSD first then try the page release.
+        if (CacheDrained)
+          Primary.tryReleaseToOS(ClassId, ReleaseToOS::Normal);
       } else {
         if (UNLIKELY(useMemoryTagging<Config>(Options)))
           storeTags(reinterpret_cast<uptr>(BlockBegin),
                     reinterpret_cast<uptr>(Ptr));
         Secondary.deallocate(Options, BlockBegin);
       }
     } else {
       bool UnlockRequired;

compiler-rt/lib/scudo/standalone/local_cache.h

void *allocate(uptr ClassId) {
     // the memory accesses in close quarters.
     const uptr ClassSize = C->ClassSize;
     CompactPtrT CompactP = C->Chunks[--C->Count];
     Stats.add(StatAllocated, ClassSize);
     Stats.sub(StatFree, ClassSize);
     return Allocator->decompactPtr(ClassId, CompactP);
   }
 
-  void deallocate(uptr ClassId, void *P) {
+  bool deallocate(uptr ClassId, void *P) {
     CHECK_LT(ClassId, NumClasses);
     PerClass *C = &PerClassArray[ClassId];
     // We still have to initialize the cache in the event that the first heap
     // operation in a thread is a deallocation.
     initCacheMaybe(C);
-    if (C->Count == C->MaxCount)
+
+    // If the cache is full, drain half of blocks back to the main allocator.
+    const bool NeedToDrainCache = C->Count == C->MaxCount;
+    if (NeedToDrainCache)
       drain(C, ClassId);
     // See comment in allocate() about memory accesses.
     const uptr ClassSize = C->ClassSize;
     C->Chunks[C->Count++] =
         Allocator->compactPtr(ClassId, reinterpret_cast<uptr>(P));
     Stats.sub(StatAllocated, ClassSize);
     Stats.add(StatFree, ClassSize);
+
+    return NeedToDrainCache;
   }
 
   bool isEmpty() const {
     for (uptr I = 0; I < NumClasses; ++I)
       if (PerClassArray[I].Count)
         return false;
     return true;
   }

compiler-rt/lib/scudo/standalone/primary32.h

for (u32 I = 1; I < Size; ++I) {
         Array[J] = Array[J - 1];
         --J;
       }
       Array[J] = Cur;
     }
 
     ScopedLock L(Sci->Mutex);
     pushBlocksImpl(C, ClassId, Sci, Array, Size, SameGroup);
-
-    if (ClassId != SizeClassMap::BatchClassId)
-      releaseToOSMaybe(Sci, ClassId);
   }
 
   void disable() NO_THREAD_SAFETY_ANALYSIS {
     // The BatchClassId must be locked last since other classes can use it.
     for (sptr I = static_cast<sptr>(NumClasses) - 1; I >= 0; I--) {
       if (static_cast<uptr>(I) == SizeClassMap::BatchClassId)
         continue;
       getSizeClassInfo(static_cast<uptr>(I))->Mutex.lock();
if (O == Option::ReleaseInterval) {
                                Config::Primary::MinReleaseToOsIntervalMs);
       atomic_store_relaxed(&ReleaseToOsIntervalMs, Interval);
       return true;
     }
     // Not supported by the Primary, but not an error either.
     return true;
   }
 
+  uptr tryReleaseToOS(uptr ClassId, ReleaseToOS ReleaseType) {
+    SizeClassInfo *Sci = getSizeClassInfo(ClassId);
+    // TODO: Once we have separate locks like primary64, we may consider using
+    // tryLock() as well.
+    ScopedLock L(Sci->Mutex);
+    return releaseToOSMaybe(Sci, ClassId, ReleaseType);
+  }
+
   uptr releaseToOS(ReleaseToOS ReleaseType) {
     uptr TotalReleasedBytes = 0;
     for (uptr I = 0; I < NumClasses; I++) {
       if (I == SizeClassMap::BatchClassId)
         continue;
       SizeClassInfo *Sci = getSizeClassInfo(I);
       ScopedLock L(Sci->Mutex);
       TotalReleasedBytes += releaseToOSMaybe(Sci, I, ReleaseType);

compiler-rt/lib/scudo/standalone/primary64.h

for (u32 I = 1; I < Size; ++I) {
       }
       Array[J] = Cur;
     }
 
     {
       ScopedLock L(Region->FLLock);
       pushBlocksImpl(C, ClassId, Region, Array, Size, SameGroup);
     }
-
-    // Only non-BatchClass will be here, try to release the pages in the region.
-
-    // Note that the tryLock() may fail spuriously, given that it should rarely
-    // happen and page releasing is fine to skip, we don't take certain
-    // approaches to ensure one page release is done.
-    if (Region->MMLock.tryLock()) {
-      releaseToOSMaybe(Region, ClassId);
-      Region->MMLock.unlock();
-    }
   }
 
   void disable() NO_THREAD_SAFETY_ANALYSIS {
     // The BatchClassId must be locked last since other classes can use it.
     for (sptr I = static_cast<sptr>(NumClasses) - 1; I >= 0; I--) {
       if (static_cast<uptr>(I) == SizeClassMap::BatchClassId)
         continue;
       getRegionInfo(static_cast<uptr>(I))->MMLock.lock();
if (O == Option::ReleaseInterval) {
                                Config::Primary::MinReleaseToOsIntervalMs);
       atomic_store_relaxed(&ReleaseToOsIntervalMs, Interval);
       return true;
     }
     // Not supported by the Primary, but not an error either.
     return true;
   }
 
+  uptr tryReleaseToOS(uptr ClassId, ReleaseToOS ReleaseType) {
+    RegionInfo *Region = getRegionInfo(ClassId);
+    // Note that the tryLock() may fail spuriously, given that it should rarely
+    // happen and page releasing is fine to skip, we don't take certain
+    // approaches to ensure one page release is done.
+    if (Region->MMLock.tryLock()) {
+      uptr BytesReleased = releaseToOSMaybe(Region, ClassId, ReleaseType);
+      Region->MMLock.unlock();
+      return BytesReleased;
+    }
+    return 0;
+  }
+
   uptr releaseToOS(ReleaseToOS ReleaseType) {
     uptr TotalReleasedBytes = 0;
     for (uptr I = 0; I < NumClasses; I++) {
       if (I == SizeClassMap::BatchClassId)
         continue;
       RegionInfo *Region = getRegionInfo(I);
       ScopedLock L(Region->MMLock);
       TotalReleasedBytes += releaseToOSMaybe(Region, I, ReleaseType);