[scudo] Calling initCache() in init() of SizeClassAllocatorLocalCache

[ChiaHungDuan]

initCacheMaybe() will init all the size class arrays at once and it doesn't have much work to do even if it supports partial initialization. This avoids the call to initCacheMaybe in each allocate()/deallocate().

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (ChiaHungDuan)

Changes

initCacheMaybe() will init all the size class arrays at once and it doesn't have much work to do even if it supports partial initialization. This avoids the call to initCacheMaybe in each allocate()/deallocate().

---

Full diff: https://github.com/llvm/llvm-project/pull/71427.diff

1 Files Affected:

• (modified) compiler-rt/lib/scudo/standalone/local_cache.h (+1-12)

diff --git a/compiler-rt/lib/scudo/standalone/local_cache.h b/compiler-rt/lib/scudo/standalone/local_cache.h
index 6898840eb2bcba4..46d6affdc033b1f 100644
--- a/compiler-rt/lib/scudo/standalone/local_cache.h
+++ b/compiler-rt/lib/scudo/standalone/local_cache.h
@@ -28,6 +28,7 @@ template <class SizeClassAllocator> struct SizeClassAllocatorLocalCache {
     if (LIKELY(S))
       S->link(&Stats);
     Allocator = A;
+    initCache();
   }
 
   void destroy(GlobalStats *S) {
@@ -40,8 +41,6 @@ template <class SizeClassAllocator> struct SizeClassAllocatorLocalCache {
     DCHECK_LT(ClassId, NumClasses);
     PerClass *C = &PerClassArray[ClassId];
     if (C->Count == 0) {
-      initCacheMaybe(C);
-
       // Refill half of the number of max cached.
       DCHECK_GT(C->MaxCount / 2, 0U);
       if (UNLIKELY(!refill(C, ClassId, C->MaxCount / 2)))
@@ -61,9 +60,6 @@ template <class SizeClassAllocator> struct SizeClassAllocatorLocalCache {
   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 the cache is full, drain half of blocks back to the main allocator.
     const bool NeedToDrainCache = C->Count == C->MaxCount;
@@ -150,13 +146,6 @@ template <class SizeClassAllocator> struct SizeClassAllocatorLocalCache {
   LocalStats Stats;
   SizeClassAllocator *Allocator = nullptr;
 
-  ALWAYS_INLINE void initCacheMaybe(PerClass *C) {
-    if (LIKELY(C->MaxCount))
-      return;
-    initCache();
-    DCHECK_NE(C->MaxCount, 0U);
-  }
-
   NOINLINE void initCache() {
     for (uptr I = 0; I < NumClasses; I++) {
       PerClass *P = &PerClassArray[I];

[ChiaHungDuan]

Some clean up for the support of zero-size cache

[cryptoad]

IIRC the only reason we had it this way was to avoid dirtying the Cache for new threads that don't do allocations, which might be only really relevant for the Exclusive TSD model on heavy threaded applications. I don't think we ever gathered data about this, but might want to run a few tests (I usually ran the RPC benchmarks in g3 with Scudo compiled in).

[ChiaHungDuan]

IIRC the only reason we had it this way was to avoid dirtying the Cache for new threads that don't do allocations, which might be only really relevant for the Exclusive TSD model on heavy threaded applications. I don't think we ever gathered data about this, but might want to run a few tests (I usually ran the RPC benchmarks in g3 with Scudo compiled in).

Thanks for the context! Let me try that benchmark first and if it does have some impact, I'll try to adopt a different layout to minimize the size of unused dirty memory

[ChiaHungDuan]

After reviewing the logic of initialization of cache, I notice that we ensure the TSDs are initialized by initThreadMaybe() and iff the application accesses Scudo. For threads that never allocate, it'll not initialize the TSDs. With Exclusive TSDs, even if it only does deallocation, it'll only init the FallbackTSD by setting MinimalInit=true to avoid dirtying the space of ThreadTSD.

Therefore, I think we are safe to do this change without introducing potential unwanted dirty pages

[cferris1000]

LGTM