[PATCH] D61385: [scudo][standalone] Introduce the Quarantine

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added inline comments.


================
Comment at: lib/scudo/standalone/quarantine.h:138
+    uptr TotalQuarantineChunks = 0;
+    for (auto It = List.begin(); It != List.end(); ++It) {
+      BatchCount++;
----------------
Does range-based loop work here?


================
Comment at: lib/scudo/standalone/quarantine.h:157
+        "Global quarantine stats: batches: %zd; bytes: %zd (user: %zd); "
+        "chunks: %zd (capacity: %zd); %d%% chunks used; %d%% memory overhead\n",
+        BatchCount, TotalBytes, TotalQuarantinedBytes, TotalQuarantineChunks,
----------------
Since `uptr`, probably want `%zd` for all.


================
Comment at: lib/scudo/standalone/quarantine.h:181
+    // is zero (it allows us to perform just one atomic read per put() call).
+    CHECK((Size == 0 && CacheSize == 0) || CacheSize != 0);
+
----------------
What range of `MaxCacheSize` will Scudo allow?  I notice you removed handling for `MaxCacheSize == 0` in `put` below.


================
Comment at: lib/scudo/standalone/quarantine.h:188
+    CacheMutex.init();
+    RecyleMutex.init();
+  }
----------------
I think these can be `initLinkerInitialized`.


================
Comment at: lib/scudo/standalone/quarantine.h:188
+    CacheMutex.init();
+    RecyleMutex.init();
+  }
----------------
morehouse wrote:
> I think these can be `initLinkerInitialized`.
Right now it's a no-op, but I think we should call `Cache.initLinkerInitialized` here too in case a future change makes it not a no-op.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D61385