[PATCH] D37677: [libc++] implement future synchronization using atomic_flag

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

TODO.txt says "future should use <atomic> for synchronization." I would have interpreted this as meaning that the line

  unsigned __state_;

should become

  std::atomic<unsigned> __state_;

and appropriate loads and stores used so that `__is_ready()` can be checked without having to take the mutex first. OTOH, I don't actually see how that would help: if it's not ready, you probably want to take a unique_lock so you can wait, and if it *is* ready, you probably want to take a lock so that you can get the value out.
Atomics might allow functions like `__set_future_attached()` to stop taking the lock as well. But again I'm not sure what the benefit would be; the cost is obviously "risk of subtle bugs and maintenance nightmare."

This current patch just swaps out std::mutex for a std::mutex-alike class that claims to be faster for uncontested accesses. Definitely safer than my interpretation. :) If this patch actually helps, then I would offer that the class could be provided as a reusable class `std::__spin_lock` in the <mutex> header instead of being hidden inside `__assoc_shared_state`.


https://reviews.llvm.org/D37677