[PATCH] D64358: [scudo][standalone] Merge Spin & Blocking mutex into a Hybrid one
Kostya Kortchinsky via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 10 09:18:31 PDT 2019
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
More information about the llvm-commits
mailing list