Hi Philippe,
This was discussed in
https://codereview.qt-project.org/c/qt/qtbase/+/66118. See, in
particular, Olivier's comment.
TL;DR: ref() is documented to be ordered, so cannot be changed.
Consequently, I didn't merge it even with Thiago's +2.
To fix, we'd need to port all of the Qt classes away from the public
classes (QSharedData, QAtomicInt) and implement ref-counting completely
from scratch (well, copy QSharedData → QSharedDataV2, QAtomicInt →
QAtomicIntV2, and do the change in V2). That, or we introduce silent
breakages into user code by applying 66118, something our Chief Whip has
just announced publicly should not happen:
https://blog.qt.io/blog/2019/08/07/technical-vision-qt-6/
If we must break compatibility, a compile-time error is preferable
over a silent breakage at runtime (as those are much harder to detect).
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.
Thanks,
Marc
On 2019-08-07 19:36, Philippe wrote:
I recently found that in Qt, reference counting to guard a resource,
is using
ref() / deref()
But ref() is using memory_order_seq_cst
while memory_order_relaxed, should be sufficient
What is important is to guarantee that destruction is not subject to a
race condition, to prevent double
destruction. Hence deref() with memory_order_seq_cst is enough to
guarantee
that.
It does not matter how much the counter increase, but what is important
is to control how it is decreased. Hence deref(with
memory_order_seq_cst)
is just enough.
I have verified the implementaiton of reference counting for shared_ptr
in clang, and it does what I describe above
(it even just use memory_order_acq_rel to decrement, and not
memory_order_seq_cst)
https://github.com/llvm-mirror/libcxx/blob/master/include/memory#L3344
Is there a reason why Qt is not optimized in the same way? (since ref()
is
used a lot, and atomic operations are a bit expensive).
Is there a requirement at some stage that the reference counter must be
ordered for increments?
Philippe
_______________________________________________
Development mailing list
[email protected]
https://lists.qt-project.org/listinfo/development
_______________________________________________
Development mailing list
[email protected]
https://lists.qt-project.org/listinfo/development
