[PATCH] D73507: [scudo][standalone] Secondary & general other improvements

[Mitch Phillips via Phabricator via llvm-commits]

hctim added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/flags.inc:48
 
-SCUDO_FLAG(int, release_to_os_interval_ms, 5000,
+SCUDO_FLAG(int, release_to_os_interval_ms, SCUDO_ANDROID ? 1000 : 5000,
            "Interval (in milliseconds) at which to attempt release of unused "
----------------
Given that this might get very complicated if you wanted to introduce different defaults for different platforms - it may be worth adding a TODO to initialise Scudo as part of `libc_init_malloc` rather than rely on lazy-initialisation.


================
Comment at: compiler-rt/lib/scudo/standalone/primary64.h:88
       // limit is mostly arbitrary and based on empirical observations.
       // TODO(kostyak): make the lower limit a runtime option
       Region->CanRelease = (ReleaseToOsInterval >= 0) &&
----------------
Might be worth considering


================
Comment at: compiler-rt/lib/scudo/standalone/secondary.h:59
+  bool store(UNUSED LargeBlock::Header *H) { return false; }
+  static bool canCache(uptr Size) { return false; }
+  void disable() {}
----------------
`UNUSED uptr Size`


================
Comment at: compiler-rt/lib/scudo/standalone/secondary.h:146
+private:
+  void empty() {
+    struct {
----------------
Can you reuse code via. `empty() { releaseOlderThan(UINT64_MAX) }`?


================
Comment at: compiler-rt/lib/scudo/standalone/secondary.h:193
+        N++;
+      }
+    }
----------------
Missing `EntriesCount -= N`?


================
Comment at: compiler-rt/lib/scudo/standalone/secondary.h:294
 
-  if (MaxFreeListSize && AlignmentHint < PageSize) {
-    ScopedLock L(Mutex);
-    for (auto &H : FreeBlocks) {
-      const uptr FreeBlockSize = H.BlockEnd - reinterpret_cast<uptr>(&H);
-      if (FreeBlockSize < RoundedSize)
-        continue;
-      // Candidate free block should only be at most 4 pages larger.
-      if (FreeBlockSize > RoundedSize + 4 * PageSize)
-        break;
-      FreeBlocks.remove(&H);
-      InUseBlocks.push_back(&H);
-      AllocatedBytes += FreeBlockSize;
-      NumberOfAllocs++;
-      Stats.add(StatAllocated, FreeBlockSize);
+  if (AlignmentHint < PageSize && CacheT::canCache(RoundedSize)) {
+    LargeBlock::Header *H;
----------------
Nit: Just `canCache(RoundedSize)`


================
Comment at: compiler-rt/lib/scudo/standalone/secondary.h:387
   }
+  if (CacheT::canCache(CommitSize) && Cache.store(H))
+    return;
----------------
Nit: just `canCache(CommitSize)`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73507/new/

https://reviews.llvm.org/D73507