From f3ce9e9332820a8b5084fb4d75994e8eb19ddfd3 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Mon, 23 Mar 2020 21:30:21 +0100 Subject: Make QPropertyBindingPrivate accessible to QtQml QtQml needs the private just for one detail which nobody else should need it for: Tracking additional dependencies and marking the binding as dirty. Exporting the private requires hiding some variables and providing accessors, to compile with MSVC - including the removal of QVarLengthArray usage. Upside: The binding structure shrinks by 8 bytes and the encapsulation makes it a little easier to change things without breaking declarative, ... in the unlikely event ;-) Also remove setDirty() from the public API as it's not needed by QtQml and using it is dangerous, because it means that there's a risk of somebody keeping a reference (count) to the untyped binding from within the binding closure, which introduces a memory leak. Change-Id: I43bd56f4bdf218efb54fa23e2d627ad3acfafeb5 Reviewed-by: Fabian Kosmale --- src/corelib/kernel/qproperty.cpp | 24 +++++++---------- src/corelib/kernel/qproperty.h | 14 +++++----- src/corelib/kernel/qproperty_p.h | 2 -- src/corelib/kernel/qpropertybinding.cpp | 11 ++------ src/corelib/kernel/qpropertybinding_p.h | 46 ++++++++++++++++++++++++++++++--- src/corelib/kernel/qpropertyprivate.h | 2 +- 6 files changed, 61 insertions(+), 38 deletions(-) (limited to 'src/corelib') diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp index 2dbe8b6310..18580fa756 100644 --- a/src/corelib/kernel/qproperty.cpp +++ b/src/corelib/kernel/qproperty.cpp @@ -53,7 +53,7 @@ QPropertyBase::QPropertyBase(QPropertyBase &&other, void *propertyDataPtr) QPropertyBasePointer d{this}; d.setFirstObserver(nullptr); if (auto binding = d.bindingPtr()) - binding->propertyDataPtr = propertyDataPtr; + binding->setProperty(propertyDataPtr); } void QPropertyBase::moveAssign(QPropertyBase &&other, void *propertyDataPtr) @@ -73,7 +73,7 @@ void QPropertyBase::moveAssign(QPropertyBase &&other, void *propertyDataPtr) std::swap(d_ptr, other.d_ptr); if (auto binding = d.bindingPtr()) - binding->propertyDataPtr = propertyDataPtr; + binding->setProperty(propertyDataPtr); d.setFirstObserver(observer.ptr); @@ -111,10 +111,10 @@ QUntypedPropertyBinding QPropertyBase::setBinding(const QUntypedPropertyBinding newBinding.data()->ref.ref(); d_ptr = (d_ptr & FlagMask) | reinterpret_cast(newBinding.data()); d_ptr |= BindingBit; - newBinding->dirty = true; - newBinding->propertyDataPtr = propertyDataPtr; + newBinding->setDirty(true); + newBinding->setProperty(propertyDataPtr); if (observer) - observer.prependToBinding(newBinding.data()); + newBinding->prependObserver(observer); } else { d_ptr &= ~BindingBit; } @@ -175,11 +175,10 @@ static thread_local BindingEvaluationState *currentBindingEvaluationState = null BindingEvaluationState::BindingEvaluationState(QPropertyBindingPrivate *binding) : binding(binding) - , dependencyObservers(&binding->dependencyObservers) { previousState = currentBindingEvaluationState; currentBindingEvaluationState = this; - dependencyObservers->clear(); + binding->clearDependencyObservers(); } BindingEvaluationState::~BindingEvaluationState() @@ -222,8 +221,7 @@ void QPropertyBase::registerWithCurrentlyEvaluatingBinding() const QPropertyBasePointer d{this}; - currentState->dependencyObservers->append(QPropertyObserver()); - QPropertyObserverPointer dependencyObserver{&(*currentState->dependencyObservers)[currentState->dependencyObservers->size() - 1]}; + QPropertyObserverPointer dependencyObserver = currentState->binding->allocateDependencyObserver(); dependencyObserver.setBindingToMarkDirty(currentState->binding); dependencyObserver.observeProperty(d); } @@ -263,6 +261,8 @@ QPropertyObserver::~QPropertyObserver() d.unlink(); } +QPropertyObserver::QPropertyObserver() = default; + QPropertyObserver::QPropertyObserver(QPropertyObserver &&other) { std::swap(bindingToMarkDirty, other.bindingToMarkDirty); @@ -351,12 +351,6 @@ void QPropertyObserverPointer::observeProperty(QPropertyBasePointer property) property.addObserver(ptr); } -void QPropertyObserverPointer::prependToBinding(QPropertyBindingPrivate *binding) -{ - ptr->prev = const_cast(&binding->firstObserver.ptr); - binding->firstObserver = *this; -} - QPropertyBindingError::QPropertyBindingError(Type type) { if (type != NoError) { diff --git a/src/corelib/kernel/qproperty.h b/src/corelib/kernel/qproperty.h index 21b2ed0839..338c7bbeec 100644 --- a/src/corelib/kernel/qproperty.h +++ b/src/corelib/kernel/qproperty.h @@ -137,12 +137,10 @@ public: QMetaType valueMetaType() const; - void setDirty(bool dirty = true); - -private: explicit QUntypedPropertyBinding(const QPropertyBindingPrivatePtr &priv); +private: friend class QtPrivate::QPropertyBase; - friend struct QPropertyBindingPrivate; + friend class QPropertyBindingPrivate; template friend class QPropertyBinding; QPropertyBindingPrivatePtr d; }; @@ -357,7 +355,7 @@ private: friend struct QPropertyBasePointer; friend class QPropertyBinding; - friend struct QPropertyObserver; + friend class QPropertyObserver; // Mutable because querying for the value may require evalating the binding expression, calling // non-const functions on QPropertyBase. mutable QtPrivate::QPropertyValueStorage d; @@ -385,8 +383,9 @@ namespace Qt { struct QPropertyObserverPrivate; struct QPropertyObserverPointer; -struct Q_CORE_EXPORT QPropertyObserver +class Q_CORE_EXPORT QPropertyObserver { +public: // Internal enum ObserverTag { ObserverNotifiesBinding = 0x0, @@ -394,7 +393,7 @@ struct Q_CORE_EXPORT QPropertyObserver }; Q_DECLARE_FLAGS(ObserverTags, ObserverTag) - QPropertyObserver() = default; + QPropertyObserver(); QPropertyObserver(QPropertyObserver &&other); QPropertyObserver &operator=(QPropertyObserver &&other); ~QPropertyObserver(); @@ -424,6 +423,7 @@ private: friend struct QPropertyObserverPointer; friend struct QPropertyBasePointer; + friend class QPropertyBindingPrivate; }; Q_DECLARE_OPERATORS_FOR_FLAGS(QPropertyObserver::ObserverTags) diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h index 1fbe5231fe..a50ae0dee1 100644 --- a/src/corelib/kernel/qproperty_p.h +++ b/src/corelib/kernel/qproperty_p.h @@ -90,7 +90,6 @@ struct QPropertyObserverPointer void notify(QPropertyBindingPrivate *triggeringBinding); void observeProperty(QPropertyBasePointer property); - void prependToBinding(QPropertyBindingPrivate *binding); explicit operator bool() const { return ptr != nullptr; } @@ -110,7 +109,6 @@ struct BindingEvaluationState BindingEvaluationState(QPropertyBindingPrivate *binding); ~BindingEvaluationState(); QPropertyBindingPrivate *binding; - QVarLengthArray *dependencyObservers = nullptr; BindingEvaluationState *previousState = nullptr; }; diff --git a/src/corelib/kernel/qpropertybinding.cpp b/src/corelib/kernel/qpropertybinding.cpp index 78d24cbdc1..aa7bf1d022 100644 --- a/src/corelib/kernel/qpropertybinding.cpp +++ b/src/corelib/kernel/qpropertybinding.cpp @@ -152,21 +152,14 @@ QPropertyBindingError QUntypedPropertyBinding::error() const { if (!d) return QPropertyBindingError(); - return d->error; + return d->bindingError(); } QMetaType QUntypedPropertyBinding::valueMetaType() const { if (!d) return QMetaType(); - return d->metaType; -} - -void QUntypedPropertyBinding::setDirty(bool dirty) -{ - if (!d) - return; - d->dirty = dirty; + return d->valueMetaType(); } QT_END_NAMESPACE diff --git a/src/corelib/kernel/qpropertybinding_p.h b/src/corelib/kernel/qpropertybinding_p.h index 92638c92cc..66e969547c 100644 --- a/src/corelib/kernel/qpropertybinding_p.h +++ b/src/corelib/kernel/qpropertybinding_p.h @@ -53,8 +53,7 @@ #include #include -#include -#include +#include #include #include @@ -62,12 +61,16 @@ QT_BEGIN_NAMESPACE -struct QPropertyBindingPrivate : public QSharedData +class Q_CORE_EXPORT QPropertyBindingPrivate : public QSharedData { +private: + friend struct QPropertyBasePointer; + QUntypedPropertyBinding::BindingEvaluationFunction evaluationFunction; QPropertyObserverPointer firstObserver; - QVarLengthArray dependencyObservers; + std::array inlineDependencyObservers; + QScopedPointer> heapObservers; void *propertyDataPtr = nullptr; @@ -79,6 +82,10 @@ struct QPropertyBindingPrivate : public QSharedData bool dirty = false; bool updating = false; +public: + // public because the auto-tests access it, too. + size_t dependencyObserverCount = 0; + QPropertyBindingPrivate(const QMetaType &metaType, QUntypedPropertyBinding::BindingEvaluationFunction evaluationFunction, const QPropertyBindingSourceLocation &location) : evaluationFunction(std::move(evaluationFunction)) @@ -87,6 +94,37 @@ struct QPropertyBindingPrivate : public QSharedData {} virtual ~QPropertyBindingPrivate(); + void setDirty(bool d) { dirty = d; } + void setProperty(void *propertyPtr) { propertyDataPtr = propertyPtr; } + void prependObserver(QPropertyObserverPointer observer) { + observer.ptr->prev = const_cast(&firstObserver.ptr); + firstObserver = observer; + } + + void clearDependencyObservers() { + for (size_t i = 0; i < inlineDependencyObservers.size(); ++i) { + QPropertyObserver empty; + qSwap(inlineDependencyObservers[i], empty); + } + if (heapObservers) + heapObservers->clear(); + dependencyObserverCount = 0; + } + QPropertyObserverPointer allocateDependencyObserver() { + if (dependencyObserverCount < inlineDependencyObservers.size()) { + ++dependencyObserverCount; + return {&inlineDependencyObservers[dependencyObserverCount - 1]}; + } + ++dependencyObserverCount; + if (!heapObservers) + heapObservers.reset(new std::vector()); + return {&heapObservers->emplace_back()}; + } + + QPropertyBindingSourceLocation sourceLocation() const { return location; } + QPropertyBindingError bindingError() const { return error; } + QMetaType valueMetaType() const { return metaType; } + void unlinkAndDeref(); void markDirtyAndNotifyObservers(); diff --git a/src/corelib/kernel/qpropertyprivate.h b/src/corelib/kernel/qpropertyprivate.h index 22c5416197..b4f2cd5154 100644 --- a/src/corelib/kernel/qpropertyprivate.h +++ b/src/corelib/kernel/qpropertyprivate.h @@ -58,7 +58,7 @@ QT_BEGIN_NAMESPACE class QUntypedPropertyBinding; -struct QPropertyBindingPrivate; +class QPropertyBindingPrivate; using QPropertyBindingPrivatePtr = QExplicitlySharedDataPointer; struct QPropertyBasePointer; -- cgit v1.2.3