From 57d08f6c0909ed0873f2fa35a0298d713fa98a9d Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Sun, 17 Oct 2021 12:59:34 +0200 Subject: Inline a few methods of QPropertyObserverPointer This is necessary to optimize QObjectCompatProperty::notify in a second step. Change-Id: I89aaf51d39e04f17285f7db27f9b40d145fd846d Reviewed-by: Fabian Kosmale --- src/corelib/kernel/qproperty.cpp | 104 +++----------------------------------- src/corelib/kernel/qproperty_p.h | 106 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 111 insertions(+), 99 deletions(-) (limited to 'src/corelib/kernel') diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp index ce22309bc2..eb554eaeeb 100644 --- a/src/corelib/kernel/qproperty.cpp +++ b/src/corelib/kernel/qproperty.cpp @@ -668,37 +668,20 @@ QPropertyObserver &QPropertyObserver::operator=(QPropertyObserver &&other) noexc return *this; } -#define UNLINK_COMMON \ - if (ptr->next) \ - ptr->next->prev = ptr->prev; \ - if (ptr->prev) \ - ptr->prev.setPointer(ptr->next.data()); \ - ptr->next = nullptr; \ - ptr->prev.clear(); - /*! + \fn QPropertyObserverPointer::unlink() \internal Unlinks */ -void QPropertyObserverPointer::unlink() -{ - UNLINK_COMMON - if (ptr->next.tag() == QPropertyObserver::ObserverIsAlias) - ptr->aliasData = nullptr; -} + /*! + \fn QPropertyObserverPointer::unlink_fast() \internal Like unlink, but does not handle ObserverIsAlias. Must only be called in places where we know that we are not dealing with such an observer. */ -void QPropertyObserverPointer::unlink_fast() -{ - Q_ASSERT(ptr->next.tag() != QPropertyObserver::ObserverIsAlias); - UNLINK_COMMON -} -#undef UNLINK_COMMON void QPropertyObserverPointer::setChangeHandler(QPropertyObserver::ChangeHandler changeHandler) { @@ -725,90 +708,17 @@ void QPropertyObserverPointer::setBindingToNotify_unsafe(QPropertyBindingPrivate } /*! + \class QPropertyObserverNodeProtector \internal QPropertyObserverNodeProtector is a RAII wrapper which takes care of the internal switching logic for QPropertyObserverPointer::notify (described ibidem) */ -struct [[nodiscard]] QPropertyObserverNodeProtector { - QPropertyObserverBase m_placeHolder; - QPropertyObserverNodeProtector(QPropertyObserver *observer) - { - // insert m_placeholder after observer into the linked list - QPropertyObserver *next = observer->next.data(); - m_placeHolder.next = next; - observer->next = static_cast(&m_placeHolder); - if (next) - next->prev = &m_placeHolder.next; - m_placeHolder.prev = &observer->next; - m_placeHolder.next.setTag(QPropertyObserver::ObserverIsPlaceholder); - } - QPropertyObserver *next() const { return m_placeHolder.next.data(); } - - ~QPropertyObserverNodeProtector() { - QPropertyObserverPointer d{static_cast(&m_placeHolder)}; - d.unlink_fast(); - } -}; - -/*! \internal +/*! + \fn QPropertyObserverNodeProtector::notify(QUntypedPropertyData *propertyDataPtr) + \internal \a propertyDataPtr is a pointer to the observed property's property data */ -void QPropertyObserverPointer::notify(QUntypedPropertyData *propertyDataPtr) -{ - auto observer = const_cast(ptr); - /* - * The basic idea of the loop is as follows: We iterate over all observers in the linked list, - * and execute the functionality corresponding to their tag. - * However, complication arise due to the fact that the triggered operations might modify the list, - * which includes deletion and move of the current and next nodes. - * Therefore, we take a few safety precautions: - * 1. Before executing any action which might modify the list, we insert a placeholder node after the current node. - * As that one is stack allocated and owned by us, we can rest assured that it is - * still there after the action has executed, and placeHolder->next points to the actual next node in the list. - * Note that taking next at the beginning of the loop does not work, as the executed action might either move - * or delete that node. - * 2. After the triggered action has finished, we can use the next pointer in the placeholder node as a safe way to - * retrieve the next node. - * 3. Some care needs to be taken to avoid infinite recursion with change handlers, so we add an extra test there, that - * checks whether we're already have the same change handler in our call stack. This can be done by checking whether - * the node after the current one is a placeholder node. - */ - while (observer) { - QPropertyObserver *next = observer->next.data(); - switch (QPropertyObserver::ObserverTag(observer->next.tag())) { - case QPropertyObserver::ObserverNotifiesChangeHandler: - { - auto handlerToCall = observer->changeHandler; - // prevent recursion - if (next && next->next.tag() == QPropertyObserver::ObserverIsPlaceholder) { - observer = next->next.data(); - continue; - } - // handlerToCall might modify the list - QPropertyObserverNodeProtector protector(observer); - handlerToCall(observer, propertyDataPtr); - next = protector.next(); - break; - } - case QPropertyObserver::ObserverNotifiesBinding: - { - auto bindingToNotify = observer->binding; - QPropertyObserverNodeProtector protector(observer); - bindingToNotify->notifyRecursive(); - next = protector.next(); - break; - } - case QPropertyObserver::ObserverIsPlaceholder: - // recursion is already properly handled somewhere else - break; - case QPropertyObserver::ObserverIsAlias: - break; - default: Q_UNREACHABLE(); - } - observer = next; - } -} #ifndef QT_NO_DEBUG void QPropertyObserverPointer::noSelfDependencies(QPropertyBindingPrivate *binding) diff --git a/src/corelib/kernel/qproperty_p.h b/src/corelib/kernel/qproperty_p.h index 111811134e..dee8b47874 100644 --- a/src/corelib/kernel/qproperty_p.h +++ b/src/corelib/kernel/qproperty_p.h @@ -97,13 +97,42 @@ struct Q_AUTOTEST_EXPORT QPropertyBindingDataPointer } }; +struct [[nodiscard]] QPropertyObserverNodeProtector { + QPropertyObserverBase m_placeHolder; + QPropertyObserverNodeProtector(QPropertyObserver *observer) + { + // insert m_placeholder after observer into the linked list + QPropertyObserver *next = observer->next.data(); + m_placeHolder.next = next; + observer->next = static_cast(&m_placeHolder); + if (next) + next->prev = &m_placeHolder.next; + m_placeHolder.prev = &observer->next; + m_placeHolder.next.setTag(QPropertyObserver::ObserverIsPlaceholder); + } + + QPropertyObserver *next() const { return m_placeHolder.next.data(); } + + ~QPropertyObserverNodeProtector(); +}; + // This is a helper "namespace" struct QPropertyObserverPointer { QPropertyObserver *ptr = nullptr; - void unlink(); - void unlink_fast(); + void unlink() + { + unlink_common(); + if (ptr->next.tag() == QPropertyObserver::ObserverIsAlias) + ptr->aliasData = nullptr; + } + + void unlink_fast() + { + Q_ASSERT(ptr->next.tag() != QPropertyObserver::ObserverIsAlias); + unlink_common(); + } void setBindingToNotify(QPropertyBindingPrivate *binding); void setBindingToNotify_unsafe(QPropertyBindingPrivate *binding); @@ -121,6 +150,17 @@ struct QPropertyObserverPointer explicit operator bool() const { return ptr != nullptr; } QPropertyObserverPointer nextObserver() const { return {ptr->next.data()}; } + +private: + void unlink_common() + { + if (ptr->next) + ptr->next->prev = ptr->prev; + if (ptr->prev) + ptr->prev.setPointer(ptr->next.data()); + ptr->next = nullptr; + ptr->prev.clear(); + } }; class QPropertyBindingErrorPrivate : public QSharedData @@ -740,6 +780,68 @@ inline void QPropertyBindingPrivate::evaluateRecursive_inline(QBindingStatus *st firstObserver.evaluateBindings(status); } +inline void QPropertyObserverPointer::notify(QUntypedPropertyData *propertyDataPtr) +{ + auto observer = const_cast(ptr); + /* + * The basic idea of the loop is as follows: We iterate over all observers in the linked list, + * and execute the functionality corresponding to their tag. + * However, complication arise due to the fact that the triggered operations might modify the list, + * which includes deletion and move of the current and next nodes. + * Therefore, we take a few safety precautions: + * 1. Before executing any action which might modify the list, we insert a placeholder node after the current node. + * As that one is stack allocated and owned by us, we can rest assured that it is + * still there after the action has executed, and placeHolder->next points to the actual next node in the list. + * Note that taking next at the beginning of the loop does not work, as the executed action might either move + * or delete that node. + * 2. After the triggered action has finished, we can use the next pointer in the placeholder node as a safe way to + * retrieve the next node. + * 3. Some care needs to be taken to avoid infinite recursion with change handlers, so we add an extra test there, that + * checks whether we're already have the same change handler in our call stack. This can be done by checking whether + * the node after the current one is a placeholder node. + */ + while (observer) { + QPropertyObserver *next = observer->next.data(); + switch (QPropertyObserver::ObserverTag(observer->next.tag())) { + case QPropertyObserver::ObserverNotifiesChangeHandler: + { + auto handlerToCall = observer->changeHandler; + // prevent recursion + if (next && next->next.tag() == QPropertyObserver::ObserverIsPlaceholder) { + observer = next->next.data(); + continue; + } + // handlerToCall might modify the list + QPropertyObserverNodeProtector protector(observer); + handlerToCall(observer, propertyDataPtr); + next = protector.next(); + break; + } + case QPropertyObserver::ObserverNotifiesBinding: + { + auto bindingToNotify = observer->binding; + QPropertyObserverNodeProtector protector(observer); + bindingToNotify->notifyRecursive(); + next = protector.next(); + break; + } + case QPropertyObserver::ObserverIsPlaceholder: + // recursion is already properly handled somewhere else + break; + case QPropertyObserver::ObserverIsAlias: + break; + default: Q_UNREACHABLE(); + } + observer = next; + } +} + +inline QPropertyObserverNodeProtector::~QPropertyObserverNodeProtector() +{ + QPropertyObserverPointer d{static_cast(&m_placeHolder)}; + d.unlink_fast(); +} + QT_END_NAMESPACE #endif // QPROPERTY_P_H -- cgit v1.2.3