From 658872faa2b76987c7b2abef95c9711cf4caa1c7 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Tue, 4 Jun 2013 14:28:13 +0200 Subject: [PATCH] Move handling of multiply wrapped QObjects from QV8QObjectWrapper into QV4::ExecutionEngine The bookkeeping is annoying to do, but it should be a rare case. The common case of a QObject being wrapped only once is still lightweight as-is. Change-Id: I12e9b67270ca2afbd77b4395246eef0dfc324f8f Reviewed-by: Lars Knoll --- src/qml/qml/v4/qv4engine.cpp | 4 ++ src/qml/qml/v4/qv4engine_p.h | 7 ++- src/qml/qml/v4/qv4mm.cpp | 9 ++++ src/qml/qml/v8/qv8qobjectwrapper.cpp | 88 ++++++++++++++++-------------------- src/qml/qml/v8/qv8qobjectwrapper_p.h | 26 +++++++++-- 5 files changed, 80 insertions(+), 54 deletions(-) diff --git a/src/qml/qml/v4/qv4engine.cpp b/src/qml/qml/v4/qv4engine.cpp index fbe55d1..c96d60a 100644 --- a/src/qml/qml/v4/qv4engine.cpp +++ b/src/qml/qml/v4/qv4engine.cpp @@ -63,6 +63,7 @@ #include "qv4debugging_p.h" #include "qv4executableallocator_p.h" #include "qv4sequenceobject_p.h" +#include #if defined(Q_OS_LINUX) || defined(Q_OS_MAC) #include @@ -93,6 +94,7 @@ ExecutionEngine::ExecutionEngine(QQmlJS::EvalISelFactory *factory) , functionsNeedSort(false) , m_engineId(engineSerial.fetchAndAddOrdered(1)) , regExpCache(0) + , m_multiplyWrappedQObjects(0) { MemoryManager::GCBlocker gcBlocker(memoryManager); @@ -271,6 +273,8 @@ ExecutionEngine::ExecutionEngine(QQmlJS::EvalISelFactory *factory) ExecutionEngine::~ExecutionEngine() { + delete m_multiplyWrappedQObjects; + m_multiplyWrappedQObjects = 0; delete memoryManager; emptyClass->destroy(); delete identifierCache; diff --git a/src/qml/qml/v4/qv4engine_p.h b/src/qml/qml/v4/qv4engine_p.h index 6db8ebc..60e3eee 100644 --- a/src/qml/qml/v4/qv4engine_p.h +++ b/src/qml/qml/v4/qv4engine_p.h @@ -103,7 +103,7 @@ struct SequencePrototype; struct EvalFunction; struct Identifiers; struct InternalClass; - +class MultiplyWrappedQObjectMap; class RegExp; class RegExpCache; @@ -221,6 +221,11 @@ struct Q_QML_EXPORT ExecutionEngine }; QIntrusiveList scarceResources; + // Normally the JS wrappers for QObjects are stored in the QQmlData/QObjectPrivate, + // but any time a QObject is wrapped a second time in another engine, we have to do + // bookkeeping. + MultiplyWrappedQObjectMap *m_multiplyWrappedQObjects; + ExecutionEngine(QQmlJS::EvalISelFactory *iselFactory = 0); ~ExecutionEngine(); diff --git a/src/qml/qml/v4/qv4mm.cpp b/src/qml/qml/v4/qv4mm.cpp index 4afef68..e278637 100644 --- a/src/qml/qml/v4/qv4mm.cpp +++ b/src/qml/qml/v4/qv4mm.cpp @@ -342,6 +342,15 @@ std::size_t MemoryManager::sweep() weak = weak->next; } + if (MultiplyWrappedQObjectMap *multiplyWrappedQObjects = m_d->engine->m_multiplyWrappedQObjects) { + for (MultiplyWrappedQObjectMap::Iterator it = multiplyWrappedQObjects->begin(); it != multiplyWrappedQObjects->end();) { + if (!it.value()->markBit) + it = multiplyWrappedQObjects->erase(it); + else + ++it; + } + } + std::size_t freedCount = 0; for (QVector::iterator i = m_d->heapChunks.begin(), ei = m_d->heapChunks.end(); i != ei; ++i) diff --git a/src/qml/qml/v8/qv8qobjectwrapper.cpp b/src/qml/qml/v8/qv8qobjectwrapper.cpp index 90c903b..091a952 100644 --- a/src/qml/qml/v8/qv8qobjectwrapper.cpp +++ b/src/qml/qml/v8/qv8qobjectwrapper.cpp @@ -268,29 +268,6 @@ DEFINE_MANAGED_VTABLE(QObjectWrapper); // XXX TODO: Need to review all calls to QQmlEngine *engine() to confirm QObjects work // correctly in a worker thread -class QV8QObjectInstance : public QQmlGuard -{ -public: - QV8QObjectInstance(QObject *o, QV8QObjectWrapper *w) - : QQmlGuard(o), wrapper(w) - { - } - - ~QV8QObjectInstance() - { - } - - virtual void objectDestroyed(QObject *o) - { - if (wrapper) - wrapper->m_taintedObjects.remove(o); - delete this; - } - - QV4::WeakValue jsWrapper; - QV8QObjectWrapper *wrapper; -}; - namespace { templatewrapper = 0; - } - m_taintedObjects.clear(); } void QV8QObjectWrapper::destroy() @@ -833,34 +804,28 @@ v8::Handle QV8QObjectWrapper::newQObject(QObject *object) } else { // If this object is tainted, we have to check to see if it is in our // tainted object list - TaintedHash::Iterator iter = - ddata->hasTaintedV8Object?m_taintedObjects.find(object):m_taintedObjects.end(); - bool found = iter != m_taintedObjects.end(); + Object *alternateWrapper = 0; + if (v4->m_multiplyWrappedQObjects && ddata->hasTaintedV8Object) + alternateWrapper = v4->m_multiplyWrappedQObjects->value(object); // If our tainted handle doesn't exist or has been collected, and there isn't // a handle in the ddata, we can assume ownership of the ddata->v8object - if ((!found || (*iter)->jsWrapper.isEmpty()) && ddata->jsWrapper.isEmpty()) { - QV4::Value rv = QV4::QObjectWrapper::wrap(v4, ddata, object); - ddata->jsWrapper = rv; + if (ddata->jsWrapper.isEmpty() && !alternateWrapper) { + QV4::Value result = QV4::QObjectWrapper::wrap(v4, ddata, object); + ddata->jsWrapper = result; ddata->jsEngineId = v4->m_engineId; - - if (found) { - delete (*iter); - m_taintedObjects.erase(iter); - } - - return rv; - } else if (!found) { - QV8QObjectInstance *instance = new QV8QObjectInstance(object, this); - iter = m_taintedObjects.insert(object, instance); - ddata->hasTaintedV8Object = true; + return result; } - if ((*iter)->jsWrapper.isEmpty()) { - (*iter)->jsWrapper = QV4::QObjectWrapper::wrap(v4, ddata, object); + if (!alternateWrapper) { + alternateWrapper = QV4::QObjectWrapper::wrap(v4, ddata, object).asObject(); + if (!v4->m_multiplyWrappedQObjects) + v4->m_multiplyWrappedQObjects = new MultiplyWrappedQObjectMap; + v4->m_multiplyWrappedQObjects->insert(object, alternateWrapper); + ddata->hasTaintedV8Object = true; } - return (*iter)->jsWrapper.value(); + return QV4::Value::fromObject(alternateWrapper); } } @@ -1957,5 +1922,30 @@ QmlSignalHandler::QmlSignalHandler(ExecutionEngine *engine, QObject *object, int DEFINE_MANAGED_VTABLE(QmlSignalHandler); +void MultiplyWrappedQObjectMap::insert(QObject *key, Object *value) +{ + QHash::insert(key, value); + connect(key, SIGNAL(destroyed(QObject*)), this, SLOT(removeDestroyedObject(QObject*))); +} + +MultiplyWrappedQObjectMap::Iterator MultiplyWrappedQObjectMap::erase(MultiplyWrappedQObjectMap::Iterator it) +{ + disconnect(it.key(), SIGNAL(destroyed(QObject*)), this, SLOT(removeDestroyedObject(QObject*))); + return QHash::erase(it); +} + +void MultiplyWrappedQObjectMap::remove(QObject *key) +{ + Iterator it = find(key); + if (it == end()) + return; + erase(it); +} + +void MultiplyWrappedQObjectMap::removeDestroyedObject(QObject *object) +{ + QHash::remove(object); +} + QT_END_NAMESPACE diff --git a/src/qml/qml/v8/qv8qobjectwrapper_p.h b/src/qml/qml/v8/qv8qobjectwrapper_p.h index 78db659..72bf750 100644 --- a/src/qml/qml/v8/qv8qobjectwrapper_p.h +++ b/src/qml/qml/v8/qv8qobjectwrapper_p.h @@ -73,7 +73,6 @@ class QObject; class QV8Engine; class QQmlData; class QV8ObjectResource; -class QV8QObjectInstance; class QV8QObjectConnectionList; class QQmlPropertyCache; @@ -167,6 +166,28 @@ private: } }; +class MultiplyWrappedQObjectMap : public QObject, + private QHash +{ + Q_OBJECT +public: + typedef QHash::ConstIterator ConstIterator; + typedef QHash::Iterator Iterator; + + ConstIterator begin() const { return QHash::constBegin(); } + Iterator begin() { return QHash::begin(); } + ConstIterator end() const { return QHash::constEnd(); } + Iterator end() { return QHash::end(); } + + void insert(QObject *key, Object *value); + Object *value(QObject *key) const { return QHash::value(key, 0); } + Iterator erase(Iterator it); + void remove(QObject *key); + +private slots: + void removeDestroyedObject(QObject*); +}; + } class Q_QML_PRIVATE_EXPORT QV8QObjectWrapper @@ -186,7 +207,6 @@ public: private: friend class QQmlPropertyCache; friend class QV8QObjectConnectionList; - friend class QV8QObjectInstance; friend struct QV4::QObjectWrapper; static QV4::Value GetProperty(QV8Engine *, QObject *, @@ -200,8 +220,6 @@ private: QV8Engine *m_engine; QHash m_connections; - typedef QHash TaintedHash; - TaintedHash m_taintedObjects; }; v8::Handle QV8QObjectWrapper::getProperty(QObject *object, const QHashedV4String &string, -- 2.7.4