From 89b694ea15e46461aef5e1695e820582152c67c5 Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Thu, 4 Aug 2011 14:19:26 +0200 Subject: Replace QScriptBagContainer by QIntrusiveList MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QIntrusiveList effectively does the same as QScriptBagContainer, without the need to inherit a Node class. Let's avoid the code duplication. However, QIntrusiveList::remove() and insert() are less strict than QScriptBagContainer; introduce QScriptIntrusiveList that performs the same sanity checks (no duplicate insertion, etc). Ideally these checks would be merged into QIntrusiveList. Also rename QJSValuePrivate::reinitialize() to invalidate(), and make it not call engine->unregisterValue(this) any more. Values are only invalidated at engine destruction time, and it's the engine's responsibility to erase the list. Change-Id: I60fc61ee8f90a716a285b1dd1bf4d6a08a9349df Reviewed-on: http://codereview.qt.nokia.com/2628 Reviewed-by: Qt Sanity Bot Reviewed-by: Jędrzej Nowacki --- src/declarative/qml/v8/qjsvalue_impl_p.h | 8 +- src/declarative/qml/v8/qjsvalue_p.h | 8 +- src/declarative/qml/v8/qjsvalueiterator_p.h | 12 +- src/declarative/qml/v8/qscripttools_p.h | 182 +++------------------------- src/declarative/qml/v8/qv8engine_impl_p.h | 14 ++- src/declarative/qml/v8/qv8engine_p.h | 9 +- 6 files changed, 49 insertions(+), 184 deletions(-) diff --git a/src/declarative/qml/v8/qjsvalue_impl_p.h b/src/declarative/qml/v8/qjsvalue_impl_p.h index 39bfd898ee..b0ad7669d7 100644 --- a/src/declarative/qml/v8/qjsvalue_impl_p.h +++ b/src/declarative/qml/v8/qjsvalue_impl_p.h @@ -1038,12 +1038,14 @@ bool QJSValuePrivate::assignEngine(QV8Engine* engine) /*! \internal - reinitialize this value to an invalid value. + Invalidates this value. + + Does not remove the value from the engine's list of + registered values; that's the responsibility of the caller. */ -void QJSValuePrivate::reinitialize() +void QJSValuePrivate::invalidate() { if (isJSBased()) { - m_engine->unregisterValue(this); m_value.Dispose(); m_value.Clear(); } else if (isStringBased()) { diff --git a/src/declarative/qml/v8/qjsvalue_p.h b/src/declarative/qml/v8/qjsvalue_p.h index b50f573b5c..09cd38b0bf 100644 --- a/src/declarative/qml/v8/qjsvalue_p.h +++ b/src/declarative/qml/v8/qjsvalue_p.h @@ -43,7 +43,7 @@ #include #include -#include "qscripttools_p.h" +#include #include "qscriptshareddata_p.h" #include "qjsvalue.h" @@ -57,7 +57,6 @@ class QV8Engine; */ class QJSValuePrivate : public QSharedData - , public QScriptLinkedNode { public: inline QJSValuePrivate(); @@ -81,7 +80,7 @@ public: inline QJSValuePrivate(QV8Engine *engine, const QString& value); inline QJSValuePrivate(QV8Engine *engine, QJSValue::SpecialValue value); inline QJSValuePrivate(QV8Engine *engine, v8::Handle); - inline void reinitialize(); + inline void invalidate(); inline bool toBool() const; inline double toNumber() const; @@ -148,6 +147,7 @@ public: inline operator v8::Handle() const; inline v8::Handle asV8Value(QV8Engine *engine); private: + QIntrusiveListNode m_node; QV8Engine *m_engine; // Please, update class documentation when you change the enum. @@ -183,6 +183,8 @@ private: inline bool isNumberBased() const; inline bool isStringBased() const; inline bool prepareArgumentsForCall(v8::Handle argv[], const QJSValueList& arguments) const; + + friend class QV8Engine; }; QT_END_NAMESPACE diff --git a/src/declarative/qml/v8/qjsvalueiterator_p.h b/src/declarative/qml/v8/qjsvalueiterator_p.h index d3033d496b..6113d3dd4f 100644 --- a/src/declarative/qml/v8/qjsvalueiterator_p.h +++ b/src/declarative/qml/v8/qjsvalueiterator_p.h @@ -24,16 +24,17 @@ #ifndef QJSVALUEITERATOR_P_H #define QJSVALUEITERATOR_P_H -#include -#include +#include +#include "qjsvalue_p.h" -#include +#include QT_BEGIN_NAMESPACE class QV8Engine; -class QJSValueIteratorPrivate : public QScriptLinkedNode { +class QJSValueIteratorPrivate +{ public: inline QJSValueIteratorPrivate(const QJSValuePrivate* value); inline ~QJSValueIteratorPrivate(); @@ -52,10 +53,13 @@ public: private: Q_DISABLE_COPY(QJSValueIteratorPrivate) + QIntrusiveListNode m_node; QScriptSharedDataPointer m_object; v8::Persistent m_names; uint32_t m_index; uint32_t m_count; + + friend class QV8Engine; }; diff --git a/src/declarative/qml/v8/qscripttools_p.h b/src/declarative/qml/v8/qscripttools_p.h index f74fbab83f..c8dace0b9f 100644 --- a/src/declarative/qml/v8/qscripttools_p.h +++ b/src/declarative/qml/v8/qscripttools_p.h @@ -36,180 +36,32 @@ #ifndef QSCRIPTTOOLS_P_H #define QSCRIPTTOOLS_P_H -#include +#include QT_BEGIN_NAMESPACE -template -class QScriptBagContainer; - -/*! - \internal - \interface - Helper class for a container. The purpuse of it is to add two pointer properties to a class - inheriting this class without bloating an interface. - - This class exist only as a memory storage implementation. The only way to use it is to inherit it. -*/ -class QScriptLinkedNode +template +class QScriptIntrusiveList : public QIntrusiveList { -protected: - QScriptLinkedNode() - : m_next(0) - , m_prev(0) - {} - - ~QScriptLinkedNode() - { - Q_ASSERT_X(!isUsed(), Q_FUNC_INFO, "Destorying QScriptLinkedNode instance that still is in a container"); - } - -private: - bool isUsed() const - { - return m_next || m_prev; - } - -#if defined(Q_NO_TEMPLATE_FRIENDS) public: -#else - template - friend class QScriptBagContainer; -#endif - QScriptLinkedNode *m_next; - QScriptLinkedNode *m_prev; + inline void insert(N *n); + inline void remove(N *n); }; -/*! - \internal - The QScriptBagContainer is a simple, low level, set like container for a pointer type castable to - QScriptLinkedNode*. - Algorithms complexity: - put: O(1) - get: O(1) - forEach: O(n) - \note This container doesn't take ownership of pointed values. - \attention All values have to be unique. -*/ -template -class QScriptBagContainer +template +void QScriptIntrusiveList::insert(N *n) { -public: - QScriptBagContainer() - : m_first(0) - {} - - /*! - \internal - Add a this \a value to this container - */ - void insert(T* value) - { - //dump(Q_FUNC_INFO, value); - Q_ASSERT_X(!contains(value), Q_FUNC_INFO, "Can't insert a value which is in the bag already"); - QScriptLinkedNode* v = static_cast(value); - Q_ASSERT(v); - Q_ASSERT_X(!v->m_next && !v->m_prev, Q_FUNC_INFO, "Can't insert a value which is in an another bag"); - - if (m_first) - m_first->m_prev = v; - - v->m_next = m_first; - v->m_prev = 0; - m_first = v; - } - - /*! - \internal - Remove this \a value from this container - */ - void remove(T* value) - { - //dump(Q_FUNC_INFO, value); - QScriptLinkedNode* v = static_cast(value); - Q_ASSERT(v); + Q_ASSERT_X(!contains(n), Q_FUNC_INFO, "Can't insert a value which is in the list already"); + Q_ASSERT_X(!(n->*member).isInList(), Q_FUNC_INFO, "Can't insert a value which is in another list"); + QIntrusiveList::insert(n); +} - if (!v->m_next && !v->m_prev && m_first != v) { - // ignore that value as it is not registered at all - // FIXME: That may be optimized out if unregister call is removed from ~QtDataBase - return; - } - - Q_ASSERT_X(contains(value), Q_FUNC_INFO, "Can't remove a value which is not in the bag"); - Q_ASSERT(v->m_prev || (m_first == v && !v->m_prev)); - - if (v->m_next) - v->m_next->m_prev= v->m_prev; - - if (v->m_prev) - v->m_prev->m_next = v->m_next; - else - m_first = v->m_next; - // reset removed value - v->m_next = v->m_prev = 0; - } - - /*! - \internal - Call \a fun for each element in this container. Fun should accept T* as a parameter. - \note In general it is not allowed to change this container by calling put() or get() unless - given value is the same as currently procceded by forEach. - */ - template - void forEach(Functor fun) - { - //dump(Q_FUNC_INFO); - QScriptLinkedNode *i = m_first; - QScriptLinkedNode *tmp; - while (i) { - tmp = i; - i = i->m_next; - fun(static_cast(tmp)); - } - } - - /*! - \internal - Clear this container. - */ - void clear() - { - m_first = 0; - } - - /*! - \internal - Returns true if this container is empty; false otherwise. - */ - bool isEmpty() const - { - return !m_first; - } - -// void dump(const char* msg, T* obj = 0) const -// { -// qDebug() << msg << obj; -// qDebug() << m_first; -// QScriptLinkedNode *i = m_first; -// while (i) { -// qDebug() <<" - " << i << "(" << i->m_prev << ", " << i->m_next <<")"; -// i = i->m_next; -// } -// } - -private: - bool contains(T *value) const - { - QScriptLinkedNode *i = m_first; - while (i) { - if (static_cast(i) == value) - return true; - i = i->m_next; - } - return false; - } - QScriptLinkedNode *m_first; -}; +template +void QScriptIntrusiveList::remove(N *n) +{ + Q_ASSERT_X(contains(n), Q_FUNC_INFO, "Can't remove a value which is not in the list"); + QIntrusiveList::remove(n); +} QT_END_NAMESPACE diff --git a/src/declarative/qml/v8/qv8engine_impl_p.h b/src/declarative/qml/v8/qv8engine_impl_p.h index 382e221f5a..53ce2a5acd 100644 --- a/src/declarative/qml/v8/qv8engine_impl_p.h +++ b/src/declarative/qml/v8/qv8engine_impl_p.h @@ -99,9 +99,10 @@ inline void QV8Engine::unregisterValue(QJSValuePrivate *data) inline void QV8Engine::invalidateAllValues() { - QtScriptBagCleaner invalidator; - m_values.forEach(invalidator); - m_values.clear(); + ValueList::iterator it; + for (it = m_values.begin(); it != m_values.end(); it = it.erase()) + (*it)->invalidate(); + Q_ASSERT(m_values.isEmpty()); } inline void QV8Engine::registerValueIterator(QJSValueIteratorPrivate *data) @@ -116,9 +117,10 @@ inline void QV8Engine::unregisterValueIterator(QJSValueIteratorPrivate *data) inline void QV8Engine::invalidateAllIterators() { - QtScriptBagCleaner invalidator; - m_valueIterators.forEach(invalidator); - m_valueIterators.clear(); + ValueIteratorList::iterator it; + for (it = m_valueIterators.begin(); it != m_valueIterators.end(); it = it.erase()) + (*it)->invalidate(); + Q_ASSERT(m_valueIterators.isEmpty()); } /*! diff --git a/src/declarative/qml/v8/qv8engine_p.h b/src/declarative/qml/v8/qv8engine_p.h index b71e6dc55f..22c3504565 100644 --- a/src/declarative/qml/v8/qv8engine_p.h +++ b/src/declarative/qml/v8/qv8engine_p.h @@ -63,6 +63,8 @@ #include #include #include +#include "qjsvalue_p.h" +#include "qjsvalueiterator_p.h" #include "qscriptoriginalglobalobject_p.h" #include "qscripttools_p.h" @@ -214,7 +216,6 @@ class QDeclarativeEngine; class QDeclarativeValueType; class QNetworkAccessManager; class QDeclarativeContextData; -class QJSValueIteratorPrivate; class Q_DECLARATIVE_EXPORT QV8Engine { @@ -460,8 +461,10 @@ protected: double qtDateTimeToJsDate(const QDateTime &dt); QDateTime qtDateTimeFromJsDate(double jsDate); private: - QScriptBagContainer m_values; - QScriptBagContainer m_valueIterators; + typedef QScriptIntrusiveList ValueList; + ValueList m_values; + typedef QScriptIntrusiveList ValueIteratorList; + ValueIteratorList m_valueIterators; Q_DISABLE_COPY(QV8Engine) }; -- cgit v1.2.3