compare_exchange may need stronger ordering about incr HOT 5 CLOSED

thanks for catching this! I had misread the docs for compare_exchange. Fixed in c7f284a. Also pushed v0.1.1 (with the change, and some other things) to crates.io.

I also checked SeqCst with the (limited) benchmarks out of curiosity and it did not seem to make a difference. Do you have an experience regarding when you would see a significant performance difference from the ordering, particularly on x86_64?

Thanks again for your help!

from incr.

On x86, there are really only two modes: SeqCst and everything else. So, your change is a NOP on x86. However, on ARM and Power, the other modes do make a difference. Also, the compiler may do optimizations based on the ordering you give and independent of the target platform, so the old code could still have produced incorrect results even on x86.

Generally speaking, the "lower" the mode the faster things should run. However, I have no experience at all in actually measuring these differences; I expect this would come up only in situations with significant contention (i.e., many cores trying to access the same thing at the same time).

However, SeqCst should not be needed. It is hardly ever needed. It really only comes up in situations where you need to make a case distinction "has this happened before or after that". Most of the time, including the situation here, all you really need is "if I read a value that the other thread has written, then everything I do now is guaranteed to be ordered after everything the other thread did before the write". This is exactly what you get with release-acquire. (However, note that this statement is my opinion and I've met people that disagree. ;)

I am now wondering if there are ways in which the Rust docs could be improved here, to avoid errors like this?

from incr.

Personally I found the std::sync::atomic api very challenging to learn:

• In my mind, an ideal rust api, if it compiles, it won't crash (unless a Result or Option is explicitly unwrapped, etc). Many of the atomic methods will panic if passed the wrong Ordering variant, which is well documented but still less than ideal.
• Similarly, an ideal rust api, if it compiles, won't appear to work but contain subtle, difficult to reproduce bugs. On this front, too, atomic fails, for example in the initial version of the code I wrote.
• Finally, regarding just the documentation: for someone who, like me, had no previous experience with atomics, there's a key concept missing that was relatively difficult for me to discover elsewhere. I can't remember what prompted the "aha!" but I don't think it was in the atomic docs. It's that typical usage of compare and swap will entail entering a loop until either 1) the swap was made successfully (e.g. the post-cas value was the same as expected), or 2) the conditions that prompted the attempt to set a new value are no longer valid. If you look at the code examples in the atomic mod, they're all fairly trivial and neither compare_and_swap nor compare_exchange includes this loop logic. (compare_exchange_weak seems to). Further, there's no introductory text that explains the basics of using atomics.
• In my opinion, an improved api would abandon the use of an Ordering enum and rely on traits that enforce the requirements of the different operations at the type level. This is easier said than done, however, here's a playground with a couple stabs at ranking zero-size structs with traits that won't compile.

from incr.

closing as original problem was resolved. hope subsequent conversation was of value to you!

from incr.

Similarly, an ideal rust api, if it compiles, won't appear to work but contain subtle, difficult to reproduce bugs. On this front, too, atomic fails, for example in the initial version of the code I wrote.

The purpose of the Atomic* types is to implement higher-level abstractions that do not need quite as much care. I'm afraid I have not seen any proposal showing how to provide access to these low-level primitives in a way that makes them hard to get wrong. In fact, one can write an entire PhD thesis about manually proving the correctness of algorithms that use these primitives directly (and that's what I am doing -- and actually I am simplifying and assuming every atomic access has SeqCst as even that is already hard to handle).

Either way, improving the documentation of a Rust libstd type is one of the easiest ways to get started contributing to the compiler :)

from incr.