Simplify object/id lookup in QQmlDebugService

By tracking object destruction we can avoid looping over all cached
objects to find out which ones are still OK and we don't have to
manually clear the cache anymore. Looking up objects by source location
is specific to the engine debug service and should be done there.

Change-Id: I7dab73a7bf9c17087784f1bd9c5aef513b31e2c1
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>

4 files changed, 52 insertions, 113 deletions

diff --git a/src/qml/debugger/qqmldebugservice.cpp b/src/qml/debugger/qqmldebugservice.cpp
index dda7fbe435..769560bfa0 100644
--- a/src/qml/debugger/qqmldebugservice.cpp
+++ b/src/qml/debugger/qqmldebugservice.cpp
@@ -104,26 +104,31 @@ QQmlDebugService::State QQmlDebugService::state() const
 }
 
 namespace {
-
-struct ObjectReference
-{
-    QPointer<QObject> object;
-    int id;
-};
-
-struct ObjectReferenceHash
+class ObjectReferenceHash : public QObject
 {
+    Q_OBJECT
+public:
     ObjectReferenceHash() : nextId(0) {}
 
-    QHash<QObject *, ObjectReference> objects;
+    QHash<QObject *, int> objects;
     QHash<int, QObject *> ids;
 
     int nextId;
-};
 
+private slots:
+    void remove(QObject *obj);
+};
 }
 Q_GLOBAL_STATIC(ObjectReferenceHash, objectReferenceHash)
 
+void ObjectReferenceHash::remove(QObject *obj)
+{
+    QHash<QObject *, int>::Iterator iter = objects.find(obj);
+    if (iter != objects.end()) {
+        ids.remove(iter.value());
+        objects.erase(iter);
+    }
+}
 
 /*!
     Returns a unique id for \a object.  Calling this method multiple times
@@ -135,112 +140,23 @@ int QQmlDebugService::idForObject(QObject *object)
         return -1;
 
     ObjectReferenceHash *hash = objectReferenceHash();
-    QHash<QObject *, ObjectReference>::Iterator iter =
-            hash->objects.find(object);
+    QHash<QObject *, int>::Iterator iter = hash->objects.find(object);
 
     if (iter == hash->objects.end()) {
         int id = hash->nextId++;
-
         hash->ids.insert(id, object);
-        iter = hash->objects.insert(object, ObjectReference());
-        iter->object = object;
-        iter->id = id;
-    } else if (iter->object != object) {
-        int id = hash->nextId++;
-
-        hash->ids.remove(iter->id);
-
-        hash->ids.insert(id, object);
-        iter->object = object;
-        iter->id = id;
-    }
-    return iter->id;
-}
-
-/*!
-    Returns the object for unique \a id.  If the object has not previously been
-    assigned an id, through idForObject(), then 0 is returned.  If the object
-    has been destroyed, 0 is returned.
-*/
-QObject *QQmlDebugService::objectForId(int id)
-{
-    ObjectReferenceHash *hash = objectReferenceHash();
-
-    QHash<int, QObject *>::Iterator iter = hash->ids.find(id);
-    if (iter == hash->ids.end())
-        return 0;
-
-
-    QHash<QObject *, ObjectReference>::Iterator objIter =
-            hash->objects.find(*iter);
-    Q_ASSERT(objIter != hash->objects.end());
-
-    if (objIter->object == 0) {
-        hash->ids.erase(iter);
-        hash->objects.erase(objIter);
-        // run a loop to remove other invalid objects
-        removeInvalidObjectsFromHash();
-        return 0;
-    } else {
-        return *iter;
+        iter = hash->objects.insert(object, id);
+        connect(object, SIGNAL(destroyed(QObject*)), hash, SLOT(remove(QObject*)));
     }
+    return iter.value();
 }
 
 /*!
-    Returns a list of objects matching the given filename, line and column.
+    Returns the mapping of objects to unique \a ids, created through calls to idForObject().
 */
-QList<QObject*> QQmlDebugService::objectForLocationInfo(const QString &filename,
-                                                 int lineNumber, int columnNumber)
+const QHash<int, QObject *> &QQmlDebugService::objectsForIds()
 {
-    ObjectReferenceHash *hash = objectReferenceHash();
-    QList<QObject*> objects;
-    QHash<int, QObject *>::Iterator iter = hash->ids.begin();
-    while (iter != hash->ids.end()) {
-        QHash<QObject *, ObjectReference>::Iterator objIter =
-                hash->objects.find(*iter);
-        Q_ASSERT(objIter != hash->objects.end());
-
-        if (objIter->object == 0) {
-            iter = hash->ids.erase(iter);
-            hash->objects.erase(objIter);
-        } else {
-            QQmlData *ddata = QQmlData::get(iter.value());
-            if (ddata && ddata->outerContext) {
-                if (QFileInfo(ddata->outerContext->urlString()).fileName() == filename &&
-                    ddata->lineNumber == lineNumber &&
-                    ddata->columnNumber >= columnNumber) {
-                    objects << *iter;
-                }
-            }
-            ++iter;
-        }
-    }
-    return objects;
-}
-
-void QQmlDebugService::removeInvalidObjectsFromHash()
-{
-    ObjectReferenceHash *hash = objectReferenceHash();
-    QHash<int, QObject *>::Iterator iter = hash->ids.begin();
-    while (iter != hash->ids.end()) {
-        QHash<QObject *, ObjectReference>::Iterator objIter =
-                hash->objects.find(*iter);
-        Q_ASSERT(objIter != hash->objects.end());
-
-        if (objIter->object == 0) {
-            iter = hash->ids.erase(iter);
-            hash->objects.erase(objIter);
-        } else {
-            ++iter;
-        }
-    }
-}
-
-void QQmlDebugService::clearObjectsFromHash()
-{
-    ObjectReferenceHash *hash = objectReferenceHash();
-    hash->ids.clear();
-    hash->objects.clear();
+    return objectReferenceHash()->ids;
 }
 
 bool QQmlDebugService::isDebuggingEnabled()
@@ -344,3 +260,5 @@ QQmlDebugStream::QQmlDebugStream(const QByteArray &ba)
 }
 
 QT_END_NAMESPACE
+
+#include "qqmldebugservice.moc"
diff --git a/src/qml/debugger/qqmldebugservice_p.h b/src/qml/debugger/qqmldebugservice_p.h
index a84237c109..af58168fff 100644
--- a/src/qml/debugger/qqmldebugservice_p.h
+++ b/src/qml/debugger/qqmldebugservice_p.h
@@ -36,6 +36,7 @@
 
 #include <QtCore/qobject.h>
 #include <QtCore/qdatastream.h>
+#include <QtCore/qhash.h>
 
 #include <private/qtqmlglobal_p.h>
 
@@ -73,12 +74,9 @@ public:
     void sendMessage(const QByteArray &);
     void sendMessages(const QList<QByteArray> &);
 
+    static const QHash<int, QObject *> &objectsForIds();
     static int idForObject(QObject *);
-    static QObject *objectForId(int);
-    static QList<QObject*> objectForLocationInfo(const QString &filename,
-                                          int lineNumber, int columnNumber);
-    static void removeInvalidObjectsFromHash();
-    static void clearObjectsFromHash();
+    static QObject *objectForId(int id) { return objectsForIds().value(id); }
 
     static QString objectToString(QObject *obj);
 
diff --git a/src/qml/debugger/qqmlenginedebugservice.cpp b/src/qml/debugger/qqmlenginedebugservice.cpp
index 0a3ddc6db5..ac49d99f09 100644
--- a/src/qml/debugger/qqmlenginedebugservice.cpp
+++ b/src/qml/debugger/qqmlenginedebugservice.cpp
@@ -48,6 +48,7 @@
 
 #include <QtCore/qdebug.h>
 #include <QtCore/qmetaobject.h>
+#include <QtCore/qfileinfo.h>
 #include <private/qmetaobject_p.h>
 
 QT_BEGIN_NAMESPACE
@@ -427,6 +428,27 @@ void QQmlEngineDebugService::messageReceived(const QByteArray &message)
     QMetaObject::invokeMethod(this, "processMessage", Qt::QueuedConnection, Q_ARG(QByteArray, message));
 }
 
+/*!
+    Returns a list of objects matching the given filename, line and column.
+*/
+QList<QObject*> QQmlEngineDebugService::objectForLocationInfo(const QString &filename,
+                                                              int lineNumber, int columnNumber)
+{
+    QList<QObject *> objects;
+    const QHash<int, QObject *> &hash = objectsForIds();
+    for (QHash<int, QObject *>::ConstIterator i = hash.constBegin(); i != hash.constEnd(); ++i) {
+        QQmlData *ddata = QQmlData::get(i.value());
+        if (ddata && ddata->outerContext) {
+            if (QFileInfo(ddata->outerContext->urlString()).fileName() == filename &&
+                ddata->lineNumber == lineNumber &&
+                ddata->columnNumber >= columnNumber) {
+                objects << i.value();
+            }
+        }
+    }
+    return objects;
+}
+
 void QQmlEngineDebugService::processMessage(const QByteArray &message)
 {
     QQmlDebugStream ds(message);
@@ -500,8 +522,7 @@ void QQmlEngineDebugService::processMessage(const QByteArray &message)
 
         ds >> file >> lineNumber >> columnNumber >> recurse >> dumpProperties;
 
-        QList<QObject*> objects = QQmlDebugService::objectForLocationInfo(
-                                file, lineNumber, columnNumber);
+        QList<QObject*> objects = objectForLocationInfo(file, lineNumber, columnNumber);
 
         rs << QByteArray("FETCH_OBJECTS_FOR_LOCATION_R") << queryId
            << objects.count();
diff --git a/src/qml/debugger/qqmlenginedebugservice_p.h b/src/qml/debugger/qqmlenginedebugservice_p.h
index 0a824b132f..a87d352d1c 100644
--- a/src/qml/debugger/qqmlenginedebugservice_p.h
+++ b/src/qml/debugger/qqmlenginedebugservice_p.h
@@ -116,6 +116,8 @@ private:
     bool resetBinding(int objectId, const QString &propertyName);
     bool setMethodBody(int objectId, const QString &method, const QString &body);
     void storeObjectIds(QObject *co);
+    QList<QObject *> objectForLocationInfo(const QString &filename, int lineNumber,
+                                           int columnNumber);
 
     QList<QQmlEngine *> m_engines;
     QQmlWatcher *m_watch;