-
Notifications
You must be signed in to change notification settings - Fork 653
Portable atomics? #726
Comments
Yes, it's on the TODO list but I haven't gotten around to it yet. The idea is have a uv-atomic.h header file with static inline functions that follow C++11 in that you explicitly specify the memory model. As an example, you'd have something like this: enum uv_memory_model {
UV_ATOMIC_RELAXED,
UV_ATOMIC_CONSUME,
UV_ATOMIC_ACQUIRE,
UV_ATOMIC_RELEASE,
UV_ATOMIC_ACQ_REL,
UV_ATOMIC_SEQ_CST
};
typedef struct {
uint32_t val;
} uv_atomic_uint32;
inline static void uv_atomic_uint32_inc(uv_atomic_uint32* p, enum uv_memory_model model) {
switch (model) {
/* ... */
}
} If you have some spare cycles, pull requests are welcome. :-) |
I decided to take a slightly different approach after thinking it over. A (deeply incomplete) proof-of-concept in all its horrible beauty is here: https://gist.github.com/bnoordhuis/3fafd09f65a789dd98bc - warning: lots and lots of cpp abuse. Essentially, what it gives you are the store, load, exchange, compare_exchange_{strong,weak} and fetch_{add,sub,and,or,xor} operations from C++11, with special-casing for the memory model. That is: int uv_compare_exchange_uint32(uint32_t *ptr, uint32_t *expected, uint32_t newval)
{
return uv_compare_exchange_uint32_seq_cst(ptr, expected, newval);
}
int uv_compare_exchange_uint32_seq_cst(uint32_t *ptr, uint32_t *expected, uint32_t newval)
{
// CAS operation goes here
}
// and *_acquire, *_release, etc. Supported types are signed and unsigned 8/16/32/64 bits integers plus void*. One thing that I've skipped on for now is supporting separate memory models for the success and failure cases of compare_exchange_{weak,strong}. The C++11 spec mandates it but I really can't think of a plausible scenario where you would actually use that... Suggestions welcome. |
First of all, thanks for taking another look at this. I've been meaning to get back around to looking at this, but my knowledge of the C++11 memory model is "immature" at best, so I still have a lot of background reading to do. I mainly code in old C89, so I don't get to even look at fancy C11 features, and my experience and knowledge with threading and lock-free programming is Just Enough To Be Dangerous (tm). I can't complain much about the macro magic - implementing all of those operations across that set of integer types practically requires it (though TBH I'd be pretty happy if it were just platform-sized integers and pointers, since that covers my use cases entirely, I understand that might not be enough for a library though). I also prefer having the explicit function names rather than the enum to specify the memory model, even if it does lead to the macro madness. I wonder if the weak vs strong CAS operations distinction is necessary. Obviously we're good for x86 and "strong" CAS emulation is slowish on ARM since we have to spin a loop through failures, but new ARM(v8) is getting "strong" memory model ops (ldra/strl), and does libuv/Node.js care about any other platforms? Should we be aiming for maximum portability here? I agree entirely on the _explicit() methods - they don't seem useful to me, but they might be if libuv cares about the more obscure load/store architectures. Furthermore, are the _sync*() /InterlockedOp()/OSAtomicOp() family of functions not usable for implementing these ops? (Something I read said that at least __sync_synchronize() was or is pretty broken on x86, but that's the only case that I know I've heard of.) If not, should libuv have passthroughs for building with a C11/C++11 compilers and/or fallback code paths to regular locks? Thanks again for your work! |
MIPS. It's not a tier 1 architecture but I don't want to break it if I can avoid it, provided the cost is not too high. There is some interest from large companies in porting node.js to AIX so keeping the door open for PPC would be nice too.
They are, but only when the compiler supports them. The _sync*() family of functions was originally added in gcc 4.1 but not fully implemented on most architectures until (IIRC) 4.2 or 4.3 so it won't do to rely on them too much. Besides, the actual assembly code is pretty trivial compared to everything else in that file. :-)
I wouldn't call it broken but it's inefficient: it always emits a full barrier when what you usually need is just a read or a write barrier. |
I'll take a poke at this, if nobody else is. @bnoordhuis, I assume your code is OK, but needs msvc variants that use |
It's more of a proof-of-concept right now. It doesn't currently insert memory barriers, for example (which mostly works out okay on x86 but only by accident. ARM has a significantly weaker memory model.)
Yep. Also, it would be nice to have some kind of fallback for unsupported architectures but I haven't made up my mind yet on how to do that. Suggestions welcome. |
Closing old stalled issues, please reopen at https://github.com/libuv/libuv if still needed. |
It's nice that libuv gives us portable threading support, but often a lock is more than you need, a simple atomic operation will suffice. The three my own code need most are inc/dec and cmpxchg. I've already noticed in the codebase itself the desire to use atomic ops (see refcounting, uv__async_make_pending and the other uses of the ACCESS_ONCE macro).
Any chance we could get these as uv_atomic_inc() etc, instead of having to roll our own?
The text was updated successfully, but these errors were encountered: