scudo: Remove positional template arguments for secondary cache. NFCI.

Make these arguments named constants in the Config class instead
of being positional arguments to MapAllocatorCache. This makes the
configuration easier to follow.

Eventually we should follow suit with the other classes but this is
a start.

Differential Revision: https://reviews.llvm.org/D93251

diff --git a/compiler-rt/lib/scudo/standalone/allocator_config.h b/compiler-rt/lib/scudo/standalone/allocator_config.h
index cf362da..8f1757d 100644 (file)
--- a/compiler-rt/lib/scudo/standalone/allocator_config.h
+++ b/compiler-rt/lib/scudo/standalone/allocator_config.h
@@ -32,7 +32,13 @@ struct DefaultConfig {
   // 512KB regions
   typedef SizeClassAllocator32<SizeClassMap, 19U> Primary;
 #endif
-  typedef MapAllocator<MapAllocatorCache<>> Secondary;
+  typedef MapAllocatorCache<DefaultConfig> SecondaryCache;
+  static const u32 SecondaryCacheEntriesArraySize = 32U;
+  static const u32 SecondaryCacheDefaultMaxEntriesCount = 32U;
+  static const uptr SecondaryCacheDefaultMaxEntrySize = 1UL << 19;
+  static const s32 SecondaryCacheMinReleaseToOsIntervalMs = INT32_MIN;
+  static const s32 SecondaryCacheMaxReleaseToOsIntervalMs = INT32_MAX;
+
   template <class A> using TSDRegistryT = TSDRegistryExT<A>; // Exclusive
 };
 
@@ -47,9 +53,13 @@ struct AndroidConfig {
   // 256KB regions
   typedef SizeClassAllocator32<SizeClassMap, 18U, 1000, 1000> Primary;
 #endif
-  // Cache blocks up to 2MB
-  typedef MapAllocator<MapAllocatorCache<256U, 32U, 2UL << 20, 0, 1000>>
-      Secondary;
+  typedef MapAllocatorCache<AndroidConfig> SecondaryCache;
+  static const u32 SecondaryCacheEntriesArraySize = 256U;
+  static const u32 SecondaryCacheDefaultMaxEntriesCount = 32U;
+  static const uptr SecondaryCacheDefaultMaxEntrySize = 2UL << 20;
+  static const s32 SecondaryCacheMinReleaseToOsIntervalMs = 0;
+  static const s32 SecondaryCacheMaxReleaseToOsIntervalMs = 1000;
+
   template <class A>
   using TSDRegistryT = TSDRegistrySharedT<A, 8U, 2U>; // Shared, max 8 TSDs.
 };
@@ -63,7 +73,13 @@ struct AndroidSvelteConfig {
   // 64KB regions
   typedef SizeClassAllocator32<SizeClassMap, 16U, 1000, 1000> Primary;
 #endif
-  typedef MapAllocator<MapAllocatorCache<16U, 4U, 1UL << 18, 0, 0>> Secondary;
+  typedef MapAllocatorCache<AndroidSvelteConfig> SecondaryCache;
+  static const u32 SecondaryCacheEntriesArraySize = 16U;
+  static const u32 SecondaryCacheDefaultMaxEntriesCount = 4U;
+  static const uptr SecondaryCacheDefaultMaxEntrySize = 1UL << 18;
+  static const s32 SecondaryCacheMinReleaseToOsIntervalMs = 0;
+  static const s32 SecondaryCacheMaxReleaseToOsIntervalMs = 0;
+
   template <class A>
   using TSDRegistryT = TSDRegistrySharedT<A, 2U, 1U>; // Shared, max 2 TSDs.
 };
@@ -72,7 +88,7 @@ struct AndroidSvelteConfig {
 struct FuchsiaConfig {
   // 1GB Regions
   typedef SizeClassAllocator64<DefaultSizeClassMap, 30U> Primary;
-  typedef MapAllocator<MapAllocatorNoCache> Secondary;
+  typedef MapAllocatorNoCache SecondaryCache;
   template <class A>
   using TSDRegistryT = TSDRegistrySharedT<A, 8U, 4U>; // Shared, max 8 TSDs.
 };

diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index e214b01..7bf108e 100644 (file)
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -930,7 +930,7 @@ public:
   }
 
 private:
-  using SecondaryT = typename Params::Secondary;
+  using SecondaryT = MapAllocator<typename Params::SecondaryCache>;
   typedef typename PrimaryT::SizeClassMap SizeClassMap;
 
   static const uptr MinAlignmentLog = SCUDO_MIN_ALIGNMENT_LOG;

diff --git a/compiler-rt/lib/scudo/standalone/secondary.h b/compiler-rt/lib/scudo/standalone/secondary.h
index ff41bd3..cccbeb2 100644 (file)
--- a/compiler-rt/lib/scudo/standalone/secondary.h
+++ b/compiler-rt/lib/scudo/standalone/secondary.h
@@ -70,20 +70,18 @@ public:
   }
 };
 
-template <u32 EntriesArraySize = 32U, u32 DefaultMaxEntriesCount = 32U,
-          uptr DefaultMaxEntrySize = 1UL << 19,
-          s32 MinReleaseToOsIntervalMs = INT32_MIN,
-          s32 MaxReleaseToOsIntervalMs = INT32_MAX>
-class MapAllocatorCache {
+template <typename Config> class MapAllocatorCache {
 public:
   // Ensure the default maximum specified fits the array.
-  static_assert(DefaultMaxEntriesCount <= EntriesArraySize, "");
+  static_assert(Config::SecondaryCacheDefaultMaxEntriesCount <=
+                    Config::SecondaryCacheEntriesArraySize,
+                "");
 
   void initLinkerInitialized(s32 ReleaseToOsInterval) {
     setOption(Option::MaxCacheEntriesCount,
-              static_cast<sptr>(DefaultMaxEntriesCount));
+              static_cast<sptr>(Config::SecondaryCacheDefaultMaxEntriesCount));
     setOption(Option::MaxCacheEntrySize,
-              static_cast<sptr>(DefaultMaxEntrySize));
+              static_cast<sptr>(Config::SecondaryCacheDefaultMaxEntrySize));
     setOption(Option::ReleaseInterval, static_cast<sptr>(ReleaseToOsInterval));
   }
   void init(s32 ReleaseToOsInterval) {
@@ -162,13 +160,14 @@ public:
   bool setOption(Option O, sptr Value) {
     if (O == Option::ReleaseInterval) {
       const s32 Interval =
-          Max(Min(static_cast<s32>(Value), MaxReleaseToOsIntervalMs),
-              MinReleaseToOsIntervalMs);
+          Max(Min(static_cast<s32>(Value),
+                  Config::SecondaryCacheMaxReleaseToOsIntervalMs),
+              Config::SecondaryCacheMinReleaseToOsIntervalMs);
       atomic_store_relaxed(&ReleaseToOsIntervalMs, Interval);
       return true;
     } else if (O == Option::MaxCacheEntriesCount) {
       const u32 MaxCount = static_cast<u32>(Value);
-      if (MaxCount > EntriesArraySize)
+      if (MaxCount > Config::SecondaryCacheEntriesArraySize)
         return false;
       atomic_store_relaxed(&MaxEntriesCount, MaxCount);
       return true;
@@ -192,11 +191,11 @@ private:
       void *MapBase;
       uptr MapSize;
       MapPlatformData Data;
-    } MapInfo[EntriesArraySize];
+    } MapInfo[Config::SecondaryCacheEntriesArraySize];
     uptr N = 0;
     {
       ScopedLock L(Mutex);
-      for (uptr I = 0; I < EntriesArraySize; I++) {
+      for (uptr I = 0; I < Config::SecondaryCacheEntriesArraySize; I++) {
         if (!Entries[I].Block)
           continue;
         MapInfo[N].MapBase = reinterpret_cast<void *>(Entries[I].MapBase);
@@ -217,7 +216,7 @@ private:
     ScopedLock L(Mutex);
     if (!EntriesCount)
       return;
-    for (uptr I = 0; I < EntriesArraySize; I++) {
+    for (uptr I = 0; I < Config::SecondaryCacheEntriesArraySize; I++) {
       if (!Entries[I].Block || !Entries[I].Time || Entries[I].Time > Time)
         continue;
       releasePagesToOS(Entries[I].Block, 0,
@@ -237,7 +236,7 @@ private:
   };
 
   HybridMutex Mutex;
-  CachedBlock Entries[EntriesArraySize];
+  CachedBlock Entries[Config::SecondaryCacheEntriesArraySize];
   u32 EntriesCount;
   atomic_u32 MaxEntriesCount;
   atomic_uptr MaxEntrySize;

diff --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
index 481dfd8..7df4594 100644 (file)
--- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
@@ -401,7 +401,7 @@ struct DeathConfig {
   using DeathSizeClassMap = scudo::FixedSizeClassMap<DeathSizeClassConfig>;
   typedef scudo::SizeClassAllocator64<DeathSizeClassMap, DeathRegionSizeLog>
       Primary;
-  typedef scudo::MapAllocator<scudo::MapAllocatorNoCache> Secondary;
+  typedef scudo::MapAllocatorNoCache SecondaryCache;
   template <class A> using TSDRegistryT = scudo::TSDRegistrySharedT<A, 1U, 1U>;
 };
 

diff --git a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
index d9f2d2f..3c1e779 100644 (file)
--- a/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/secondary_test.cpp
@@ -8,6 +8,7 @@
 
 #include "tests/scudo_unit_test.h"
 
+#include "allocator_config.h"
 #include "secondary.h"
 
 #include <stdio.h>
@@ -54,14 +55,24 @@ template <class SecondaryT> static void testSecondaryBasic(void) {
   Str.output();
 }
 
+struct TestConfig {
+  static const scudo::u32 SecondaryCacheEntriesArraySize = 128U;
+  static const scudo::u32 SecondaryCacheDefaultMaxEntriesCount = 64U;
+  static const scudo::uptr SecondaryCacheDefaultMaxEntrySize = 1UL << 20;
+  static const scudo::s32 SecondaryCacheMinReleaseToOsIntervalMs = INT32_MIN;
+  static const scudo::s32 SecondaryCacheMaxReleaseToOsIntervalMs = INT32_MAX;
+};
+
 TEST(ScudoSecondaryTest, SecondaryBasic) {
   testSecondaryBasic<scudo::MapAllocator<scudo::MapAllocatorNoCache>>();
-  testSecondaryBasic<scudo::MapAllocator<scudo::MapAllocatorCache<>>>();
   testSecondaryBasic<
-      scudo::MapAllocator<scudo::MapAllocatorCache<128U, 64U, 1UL << 20>>>();
+      scudo::MapAllocator<scudo::MapAllocatorCache<scudo::DefaultConfig>>>();
+  testSecondaryBasic<
+      scudo::MapAllocator<scudo::MapAllocatorCache<TestConfig>>>();
 }
 
-using LargeAllocator = scudo::MapAllocator<scudo::MapAllocatorCache<>>;
+using LargeAllocator =
+    scudo::MapAllocator<scudo::MapAllocatorCache<scudo::DefaultConfig>>;
 
 // This exercises a variety of combinations of size and alignment for the
 // MapAllocator. The size computation done here mimic the ones done by the