DIP 1024--Shared Atomics--Community Review Round 1

[Manu]

On Tue, Oct 1, 2019 at 10:57 PM Manu <turkeyman at gmail.com> wrote:
>
> On Tue, Oct 1, 2019 at 3:45 AM Mike Parker via Digitalmars-d
> <digitalmars-d at puremagic.com> wrote:
> >
> > [...]
>
> At best, this DIP is premature; we need to make shared correctly
> restrict read/write access before trying to do fancy stuff.
> That flag is now available, but the implementation has bugs, and it
> needs to be enabled by default.
>
> Assuming that the supporting material I mention above is available and
> works properly, then I think it might be reasonable to open some
> discussion of this kind, and on that note... I think this is a really
> bad idea, and almost the opposite of what you want in virtually every
> case.
> I've written a whole lot of lock-free concurrent code, which involves
> a lot of atomics, and the semantics in this DIP are not anything I
> have ever seen or wanted (as I've argued here in the past). I think
> the semantics described are misleading, and would only serve to
> improperly educate an amateur user how to incorrectly write lock-free
> algorithms, and increase their confidence that they're doing it right
> (they're probably not).
> 1. Most atomic operations are NOT seq-cst. Seq-cst operations are *super rare*.
>   a. I just searched our entire engine codebase; there are 971 calls
> to atomic functions; there are *four* instances of seq-cst, and I
> think at least some of them are actually wrong (over-synchronised),
> possibly all of them, I would need to consider closer.
> 2. Many atomic operations appear in a series, in which case, it's
> typical that exactly one operation in the series may be the
> synchronisation point.
> 3. In reality, probably at least half of operations are 'relaxed'.
> 4. xchg and cas operations are fundamental to any atomic code, and
> there are no syntax or operators for those operations; so, you're
> undoubtedly using core.atomic aggressively anyway, and I think it's
> just weird to litter naked arithmetic in between calls to core.atomic
> functions. I feel that's misleading by making the naked arithmetic
> look like NOT-atomic operations by contrast to a bunch of atomic
> library calls.
> 5. In almost all cases when working with atomics, you tend to do
> quirky stuff not captured by regular usage of data types, and not
> conveniently facilitated by this DIP.
>
> For instance; this is a common implementation for releasing a ref count:
>   if (atomicLoad(rc, Relaxed) == 1 || atomicSub(rc, 1, Relaxed))
>   {
>     // maybe acquire if necessary
>     // do release logic...
>   }
>
> How do I write that code with this DIP?
>
> We need to make shared correct before we do any stuff like this, and
> then when it's correct, we can talk... but I will probably continue to
> resist this, because the only result I see is that it will train
> amateurs to write bad atomic algorithms with an improper sense of
> confidence gained by being sanctioned by the language.
>
> This is a bad idea.

Sorry, that should have read:

  if (atomicLoad(rc, Relaxed) == 1 || atomicSub(rc, 1, Relaxed) == 1)
  {
    // maybe acquire if necessary
    // do release logic...
  }