[Development] atomic reference counting implementation
marc at kdab.com
Thu Aug 8 10:55:13 CEST 2019
On 2019-08-08 09:36, Lars Knoll wrote:
>> On 7 Aug 2019, at 20:00, Mutz, Marc via Development
> Changing ref() from seq_cst to relaxed and documenting that should be
> ok for Qt 6, as it’ll still to the right thing for its indented use
> case. At least it is very much preferable over duplicating all our
>> I think I'm on record saying such impl details as ref-counting for
>> Qt implicitly shared classes should not be public API. This is a
>> perfect example of why I believe that to be fundamentally true.
> helped our users implement their own ref-counted
> classes. Having such a class is a good thing. And if some of its
> semantics are stricter than they should have been, let’s fix that.
Careful not to mix things here: What we need to change is not QSDP or
QESDP. At least not for this particular problem. Q(E)SDP's problems lie
elsewhere and it's just a lucky coincidence that QISP allows to fix
both: the a) Q(E)SDP problems and b) taking Qt ref-counting private to
allow the proposed optimisation.
What needs to change for (b) is ref() (and deref()), and I think
QSharedData, but Peppe already doubted that and he may have a point. I'm
saying that we can have both: old ref() with the same semantics and
something Qt-private to use in our own ref-counting. QSharedData isn't
rocket science, and it's easy to do the change while porting to QISP.
That leaves the question of whether "QSharedDataV2" should be public or
private API. I'm not sure. I'd very much prefer to have this as a
private implementation detail of Qt, and keep QSharedData as the public
Let's look at the possible users:
- applications programmers can continue to use QSD and "suffer" from the
performance drag of ordered ref(). They can also use QISP + std::atomic
as ref-count and implement their own qIntrusiveRef/Deref to match what
Herb and Hans recommend.
- (Qt-using) library programmers can use qshareddata_p.h and selectively
opt into the "fast" ref-counting. For most Qt-centric libraries, this
will probably not bring any new dependencies.
I can also spell QSharedDataV2 "QIntrusivelyRefCounted" and make that
public. I'm confident that with QISP and QSDv2, we won't need to have
yet another pimpl class in a few years. I wasn't so confident with
- Qt libraries should all be ported to QISP and QSDv2.
> This goes back to what the classes/methods intended and documented use
> is. ref() is there to increase a refcount
But that's the whole point Olivier was making two years ago: ref() is
_documented_ to be a memory barrier:
> This function uses ordered memory ordering semantics,
> which ensures that memory access before and after the
> atomic operation (in program order) may not be re-ordered.
> , and for that relaxed
> semantics are enough. If you need seq_cst, we have a method that
> explicitly does that in QAtomic as well.
Experience shows that even "obviously correct" changes in documented
behaviour can backfire (QString(QByteArray) comes to mind). Users may
have used ref() as a synchronisation point, because it was documented to
be. I find it hard to believe that users would be able to find such uses
in their code bases even if they knew ref() has changed.
This is the worst of silent breakages: no known (easy) pattern to search
for, so high chance of being missed even when audited for. While
probability of the API being used in this particular way is low, the
severity is fatal: UB. It's like having a mutex which in some random
cases doesn't lock.
If you want to change ref(), call it something else and deprecate or
remove ref() to make its use a compile error. But there's absolutely
zero need to touch QAtomic or QSharedData: they can live on (deprecated
or not, in QtCore or Qt5Support, I don't care), untouched, while the
world migrates to the improved successors (std::atomic and QISP).
More information about the Development