[PATCH] D64358: [scudo][standalone] Merge Spin & Blocking mutex into a Hybrid one

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad marked 5 inline comments as done.
cryptoad added inline comments.


================
Comment at: lib/scudo/standalone/mutex.h:21
 public:
-  void init() { atomic_store_relaxed(&State, 0); }
-
-  void lock() {
+  void init() { memset(this, 0, sizeof(*this)); }
+  bool tryLock();
----------------
hctim wrote:
> cryptoad wrote:
> > hctim wrote:
> > > Instead of memset-init here, can we simply have a `constexpr` constructor to guarantee zero-initialisation? Would potentially prevent forgotten `init()` in future.
> > The benefit here is to not do unnecessary zeroing of memory when we are dealing with TLS or static variable that are already zero'd out.
> > That allows us to only have to use `init` in tests and for stack variables.
> > I'd rather keep it that way since it's conformant with other similar code constructs.
> Constant initialization should generally be done at compile-time for statics, and non-IE TLS will have to zero-init either in the c'tor or `init()` anyway, right?
> 
> Currently, it's quite hard to keep track of whether the mutex is initialised or not. For example, `SizeClassInfo::Mutex` is only safe as it's used in `Allocator::Primary` which is (as far as I can tell) statically allocated. There shouldn't be a perf overhead for this, and it would make it a strong guarantee that the mutex is always initialised.
> 
> Up to you though - I personally find it very hard to keep track of what needs to be initialised. Also, if there is ever a change that makes the class unusable if zero-initialised, the author may mistakenly assume `init()` is called every time.
I'd like to keep it that way. I might revisit that at some point.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D64358