From 6ff8aa4b83e801acadd45cc32759ea77cf4532c8 Mon Sep 17 00:00:00 2001 From: Aaron Kennedy Date: Wed, 23 May 2012 11:38:46 +0100 Subject: [PATCH] Reduce size of QQmlAbstractBinding The doubly-linked list is unnecessary. Most bindings are created at startup, and removed only when the object is destroyed. In these cases the double-link buys nothing other than wasted space. Even when the bindings are removed earlier, searching the list is not that slow. Change-Id: I22e1376b78ba712dafd171c7447fbc6ed212b891 Reviewed-by: Roberto Raggi --- src/qml/qml/qqmlabstractbinding.cpp | 67 ++++++++++++++++++++++++------- src/qml/qml/qqmlabstractbinding_p.h | 19 +++++++-- src/qml/qml/qqmlengine.cpp | 2 +- src/qml/qml/qqmlvaluetypeproxybinding.cpp | 23 ++++++++--- 4 files changed, 86 insertions(+), 25 deletions(-) diff --git a/src/qml/qml/qqmlabstractbinding.cpp b/src/qml/qml/qqmlabstractbinding.cpp index acc207e..672e3f5 100644 --- a/src/qml/qml/qqmlabstractbinding.cpp +++ b/src/qml/qml/qqmlabstractbinding.cpp @@ -48,13 +48,14 @@ QT_BEGIN_NAMESPACE QQmlAbstractBinding::QQmlAbstractBinding() -: m_prevBinding(0), m_nextBinding(0) +: m_nextBinding(0) { } QQmlAbstractBinding::~QQmlAbstractBinding() { - Q_ASSERT(m_prevBinding == 0); + Q_ASSERT(isAddedToObject() == false); + Q_ASSERT(m_nextBinding == 0); Q_ASSERT(*m_mePtr == 0); } @@ -83,7 +84,8 @@ However, it does not enable the binding itself or call update() on it. */ void QQmlAbstractBinding::addToObject() { - Q_ASSERT(!m_prevBinding); + Q_ASSERT(!m_nextBinding); + Q_ASSERT(isAddedToObject() == false); QObject *obj = object(); Q_ASSERT(obj); @@ -117,18 +119,16 @@ void QQmlAbstractBinding::addToObject() } m_nextBinding = proxy->m_bindings; - if (m_nextBinding) m_nextBinding->m_prevBinding = &m_nextBinding; - m_prevBinding = &proxy->m_bindings; proxy->m_bindings = this; } else { m_nextBinding = data->bindings; - if (m_nextBinding) m_nextBinding->m_prevBinding = &m_nextBinding; - m_prevBinding = &data->bindings; data->bindings = this; data->setBindingBit(obj, index); } + + setAddedToObject(true); } /*! @@ -136,22 +136,59 @@ Remove the binding from the object. */ void QQmlAbstractBinding::removeFromObject() { - if (m_prevBinding) { + if (isAddedToObject()) { + QObject *obj = object(); int index = propertyIndex(); - *m_prevBinding = m_nextBinding; - if (m_nextBinding) m_nextBinding->m_prevBinding = m_prevBinding; - m_prevBinding = 0; - m_nextBinding = 0; + QQmlData *data = QQmlData::get(obj, false); + Q_ASSERT(data); if (index & 0xFF000000) { + + // Find the value type binding + QQmlAbstractBinding *vtbinding = data->bindings; + while (vtbinding->propertyIndex() != (index & 0xFFFFFF)) { + vtbinding = vtbinding->m_nextBinding; + Q_ASSERT(vtbinding); + } + Q_ASSERT(vtbinding->bindingType() == QQmlAbstractBinding::ValueTypeProxy); + + QQmlValueTypeProxyBinding *vtproxybinding = + static_cast(vtbinding); + + QQmlAbstractBinding *binding = vtproxybinding->m_bindings; + if (binding == this) { + vtproxybinding->m_bindings = m_nextBinding; + } else { + while (binding->m_nextBinding != this) { + binding = binding->m_nextBinding; + Q_ASSERT(binding); + } + binding->m_nextBinding = m_nextBinding; + } + // Value type - we don't remove the proxy from the object. It will sit their happily // doing nothing until it is removed by a write, a binding change or it is reused // to hold more sub-bindings. - } else if (QObject *obj = object()) { - QQmlData *data = QQmlData::get(obj, false); - if (data) data->clearBindingBit(index); + + } else { + + if (data->bindings == this) { + data->bindings = m_nextBinding; + } else { + QQmlAbstractBinding *binding = data->bindings; + while (binding->m_nextBinding != this) { + binding = binding->m_nextBinding; + Q_ASSERT(binding); + } + binding->m_nextBinding = m_nextBinding; + } + + data->clearBindingBit(index); } + + m_nextBinding = 0; + setAddedToObject(false); } } diff --git a/src/qml/qml/qqmlabstractbinding_p.h b/src/qml/qml/qqmlabstractbinding_p.h index ed41496..f3071cf 100644 --- a/src/qml/qml/qqmlabstractbinding_p.h +++ b/src/qml/qml/qqmlabstractbinding_p.h @@ -65,8 +65,6 @@ class Q_QML_PRIVATE_EXPORT QQmlAbstractBinding public: typedef QWeakPointer Pointer; - QQmlAbstractBinding(); - virtual void destroy(); virtual QString expression() const; @@ -95,6 +93,7 @@ public: static void printBindingLoopError(QQmlProperty &prop); protected: + QQmlAbstractBinding(); virtual ~QQmlAbstractBinding(); void clear(); @@ -114,9 +113,13 @@ private: typedef QSharedPointer SharedPointer; // To save memory, we also store the rarely used weakPointer() instance in here + // We also use the flag bits: + // m_mePtr.flag1: added to object QPointerValuePair m_mePtr; - QQmlAbstractBinding **m_prevBinding; + inline void setAddedToObject(bool v); + inline bool isAddedToObject() const; + QQmlAbstractBinding *m_nextBinding; }; @@ -126,6 +129,16 @@ QQmlAbstractBinding::getPointer(QQmlAbstractBinding *p) return p ? p->weakPointer() : Pointer(); } +void QQmlAbstractBinding::setAddedToObject(bool v) +{ + m_mePtr.setFlagValue(v); +} + +bool QQmlAbstractBinding::isAddedToObject() const +{ + return m_mePtr.flag(); +} + QT_END_NAMESPACE #endif // QQMLABSTRACTBINDING_P_H diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp index 4a3e307..08dd845 100644 --- a/src/qml/qml/qqmlengine.cpp +++ b/src/qml/qml/qqmlengine.cpp @@ -1293,7 +1293,7 @@ void QQmlData::destroyed(QObject *object) QQmlAbstractBinding *binding = bindings; while (binding) { QQmlAbstractBinding *next = binding->m_nextBinding; - binding->m_prevBinding = 0; + binding->setAddedToObject(false); binding->m_nextBinding = 0; binding->destroy(); binding = next; diff --git a/src/qml/qml/qqmlvaluetypeproxybinding.cpp b/src/qml/qml/qqmlvaluetypeproxybinding.cpp index 2cc15a5..2b891dc 100644 --- a/src/qml/qml/qqmlvaluetypeproxybinding.cpp +++ b/src/qml/qml/qqmlvaluetypeproxybinding.cpp @@ -50,10 +50,14 @@ QQmlValueTypeProxyBinding::QQmlValueTypeProxyBinding(QObject *o, int index) QQmlValueTypeProxyBinding::~QQmlValueTypeProxyBinding() { - while (m_bindings) { - QQmlAbstractBinding *binding = m_bindings; - binding->setEnabled(false, 0); + QQmlAbstractBinding *binding = m_bindings; + // This must be identical to the logic in QQmlData::destroyed() + while (binding) { + QQmlAbstractBinding *next = binding->m_nextBinding; + binding->setAddedToObject(false); + binding->m_nextBinding = 0; binding->destroy(); + binding = next; } } @@ -110,16 +114,23 @@ Removes a collection of bindings, corresponding to the set bits in \a mask. void QQmlValueTypeProxyBinding::removeBindings(quint32 mask) { QQmlAbstractBinding *binding = m_bindings; + QQmlAbstractBinding *lastBinding = 0; + while (binding) { if (mask & (1 << (binding->propertyIndex() >> 24))) { QQmlAbstractBinding *remove = binding; binding = remove->m_nextBinding; - *remove->m_prevBinding = remove->m_nextBinding; - if (remove->m_nextBinding) remove->m_nextBinding->m_prevBinding = remove->m_prevBinding; - remove->m_prevBinding = 0; + + if (lastBinding == 0) + m_bindings = remove->m_nextBinding; + else + lastBinding->m_nextBinding = remove->m_nextBinding; + + remove->setAddedToObject(false); remove->m_nextBinding = 0; remove->destroy(); } else { + lastBinding = binding; binding = binding->m_nextBinding; } } -- 2.7.4