[PATCH] D84667: [scudo][standalone] mallopt runtime configuration options

[Christopher Ferris via Phabricator via llvm-commits]

cferris requested changes to this revision.
cferris added a comment.
This revision now requires changes to proceed.

Is there any major changes from the other mallopt change? In other words, should I rerun performance data?



================
Comment at: compiler-rt/lib/scudo/standalone/allocator_config.h:68
   template <class A>
-  using TSDRegistryT = TSDRegistrySharedT<A, 1U>; // Shared, only 1 TSD.
+  using TSDRegistryT = TSDRegistrySharedT<A, 2U, 1U>; // Shared, only 2 TSDs.
 };
----------------
Super small nit, but I think this means default 1 TSD, max of 2 TSDs. So the comment should be max 2 TSDs.


================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:710
+      bool Result = Primary.setOption(O, Value);
+      Result = Secondary.setOption(O, Value) && Result;
+      return TSDRegistry.setOption(O, Value) && Result;
----------------
Is the compiler required to call setOptions if Result is false? Since the whole statement would be false, I think there is a chance a good enough compiler might not even make these calls if it notices that the first result or second result is false.


================
Comment at: compiler-rt/lib/scudo/standalone/common.h:186
+enum class Option : u8 {
+  ReleaseInterval, // Release to OS interval in milliseconds.
+  MemtagTuning,
----------------
Small nit, should this align with the comments down below. Also, should MemtagTuning have a short explanation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84667