dennis.luxen planned changes to this revision.
dennis.luxen added a comment.
In https://reviews.llvm.org/D37677#868851, @EricWF wrote:
> I agree with the general consensus that we should only make this change if
> it's significantly faster, and only after we have a test that demonstrates
>
EricWF requested changes to this revision.
EricWF added a comment.
This revision now requires changes to proceed.
I agree with the general consensus that we should only make this change if it's
significantly faster, and only after we have a test that demonstrates this.
Unfortunately I don't
bcraig added a comment.
In https://reviews.llvm.org/D37677#866743, @Quuxplusone wrote:
> 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,
Quuxplusone added a comment.
TODO.txt says "future should use for synchronization." I would have
interpreted this as meaning that the line
unsigned __state_;
should become
std::atomic __state_;
and appropriate loads and stores used so that `__is_ready()` can be checked
without having
dennis.luxen added a comment.
In https://reviews.llvm.org/D37677#866362, @bcraig wrote:
> Is there a benchmark where this demonstrates some performance improvement? I
> fear that the switch to condition_variable_any will swamp any performance
> gains from the switch to a spin lock.
>
> Also,
bcraig added a comment.
Is there a benchmark where this demonstrates some performance improvement? I
fear that the switch to condition_variable_any will swamp any performance gains
from the switch to a spin lock.
Also, the spin lock is being held during allocating operations (the exception
dennis.luxen created this revision.
This task is listed in TODO.txt. The implementation swaps mutex against a
spinlock based on atomic_flag. The spin lock itself is implemented as a nested
class in a protected context of the associated state.
https://reviews.llvm.org/D37677
Files:
