On 2021-03-01 10:06, Thiago Macieira wrote:
On Monday, 1 March 2021 09:38:58 PST Thomas Rodgers wrote: And
_M_phase, despite being non-atomic, is never accessed without the
atomic_ref, aside from the constructor. Both arrive() and wait() start
off by
creating the atomic_ref.
If it's non-atomic, then how is wait() supposed to wait on it,
atomically?
Hey, it's your code :-)
It is using atomics to operate on the value. It's just that the type on
the
structure isn't an atomic by itself.
Why, I don't know. Olivier's original code did use atomics
<https://github.com/ogiroux/atomic_wait/blob/master/include/barrier#L55-L56>:
std::atomic<ptrdiff_t> expected_adjustment;
std::atomic<__phase_t> phase;
It is atomic, in that it is always an atomic_ref. This change was made
at
Jonathan's request.
And I am not disagreeing with that. I am, however saying, that I know
this particular implementation (well the upstream one it is based on)
has been extensively tested by the original author (ogiroux) including
time on Summit. If we are going to start changing his design decisions
(beyond the largely cosmetic, not algorithmic, ones that I have made as
per Jonathan's request), they should be motivated by more than a 'well
we feel int's would be better here because Futex' justification.
That's a reasonable request.
I'd like to see the benchmark results of using a directly-futexable
type vs
using unsigned char. But even the timing results need to be weighed
against
the increased memory use for std::barrier, which means it's not a
directly-
objective conclusion. And given we *may* get 8-bit futexes, it might be
worth
keeping them this way and just tell people with large contentions to
upgrade.
We may also want to introduce a central barrier type for memory
constrained environments. I specifically
removed that from this patch because it -
1) wasn't clear how we'd go about making that decision
2) this support in GCC11 is experimental
That of course rests on the contended atomic_wait being out-of-line.
