Streamline retrieval of context property names and IDs

Most of this can be inline, and we never need to copy the actual
identifier hash.

Change-Id: I6468b6b1a571e4854c00c865a2aa57c3b2f0ca8c
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

7 files changed, 51 insertions, 45 deletions

diff --git a/src/qml/jsruntime/qv4executablecompilationunit.cpp b/src/qml/jsruntime/qv4executablecompilationunit.cpp
index 8ef8ae2221..01c8f0a7c9 100644
--- a/src/qml/jsruntime/qv4executablecompilationunit.cpp
+++ b/src/qml/jsruntime/qv4executablecompilationunit.cpp
@@ -401,6 +401,7 @@ IdentifierHash ExecutableCompilationUnit::createNamedObjectsPerComponent(int com
         const CompiledData::Object *namedObject = objectAt(*namedObjectIndexPtr);
         namedObjectCache.add(runtimeStrings[namedObject->idNameIndex], namedObject->id);
     }
+    Q_ASSERT(!namedObjectCache.isEmpty());
     return *namedObjectsPerComponentCache.insert(componentObjectIndex, namedObjectCache);
 }
 
diff --git a/src/qml/jsruntime/qv4executablecompilationunit_p.h b/src/qml/jsruntime/qv4executablecompilationunit_p.h
index 1b1b9add5c..3dc0e58ca5 100644
--- a/src/qml/jsruntime/qv4executablecompilationunit_p.h
+++ b/src/qml/jsruntime/qv4executablecompilationunit_p.h
@@ -319,6 +319,7 @@ IdentifierHash ExecutableCompilationUnit::namedObjectsPerComponent(int component
     auto it = namedObjectsPerComponentCache.find(componentObjectIndex);
     if (Q_UNLIKELY(it == namedObjectsPerComponentCache.end()))
         return createNamedObjectsPerComponent(componentObjectIndex);
+    Q_ASSERT(!it->isEmpty());
     return *it;
 }
 
diff --git a/src/qml/jsruntime/qv4identifierhash.cpp b/src/qml/jsruntime/qv4identifierhash.cpp
index 7d2d3143b2..d349b7c4fe 100644
--- a/src/qml/jsruntime/qv4identifierhash.cpp
+++ b/src/qml/jsruntime/qv4identifierhash.cpp
@@ -50,6 +50,7 @@ namespace QV4 {
 IdentifierHash::IdentifierHash(ExecutionEngine *engine)
 {
     d = new IdentifierHashData(engine->identifierTable, 3);
+    Q_ASSERT(!isEmpty());
 }
 
 void IdentifierHash::detach()
diff --git a/src/qml/jsruntime/qv4qmlcontext.cpp b/src/qml/jsruntime/qv4qmlcontext.cpp
index 18d611f2c3..f845f1d6e5 100644
--- a/src/qml/jsruntime/qv4qmlcontext.cpp
+++ b/src/qml/jsruntime/qv4qmlcontext.cpp
@@ -88,11 +88,7 @@ static OptionalReturnedValue searchContextProperties(
         bool *hasProperty, Value *base, QV4::Lookup *lookup, QV4::Lookup *originalLookup,
         QQmlEnginePrivate *ep)
 {
-    const QV4::IdentifierHash &properties = context->propertyNames();
-    if (properties.count() == 0)
-        return OptionalReturnedValue();
-
-    const int propertyIdx = properties.value(name);
+    const int propertyIdx = context->propertyIndex(name);
 
     if (propertyIdx == -1)
         return OptionalReturnedValue();
@@ -429,17 +425,13 @@ bool QQmlContextWrapper::virtualPut(Managed *m, PropertyKey id, const Value &val
     ScopedString name(scope, id.asStringOrSymbol());
 
     while (context) {
-        const QV4::IdentifierHash &properties = context->propertyNames();
         // Search context properties
-        if (properties.count()) {
-            const int propertyIndex = properties.value(name);
-            if (propertyIndex != -1) {
-                if (propertyIndex < context->numIdValues()) {
-                    v4->throwError(QLatin1String("left-hand side of assignment operator is not an lvalue"));
-                    return false;
-                }
+        if (const int propertyIndex = context->propertyIndex(name); propertyIndex != -1) {
+            if (propertyIndex < context->numIdValues()) {
+                v4->throwError(QLatin1String("left-hand side of assignment operator is not an lvalue"));
                 return false;
             }
+            return false;
         }
 
         // Search scope object
diff --git a/src/qml/qml/qqmlcontext.cpp b/src/qml/qml/qqmlcontext.cpp
index e884b28532..e25dda1c75 100644
--- a/src/qml/qml/qqmlcontext.cpp
+++ b/src/qml/qml/qqmlcontext.cpp
@@ -287,10 +287,9 @@ void QQmlContext::setContextProperty(const QString &name, const QVariant &value)
         return;
     }
 
-    QV4::IdentifierHash *properties = data->detachedPropertyNames();
-    int idx = properties->value(name);
+    int idx = data->propertyIndex(name);
     if (idx == -1) {
-        properties->add(name, data->numIdValues() + d->numPropertyValues());
+        data->addPropertyNameAndIndex(name, data->numIdValues() + d->numPropertyValues());
         d->appendPropertyValue(value);
         data->refreshExpressions();
     } else {
@@ -362,14 +361,10 @@ QVariant QQmlContext::contextProperty(const QString &name) const
 {
     Q_D(const QQmlContext);
     QVariant value;
-    int idx = -1;
 
     QQmlRefPointer<QQmlContextData> data = d->m_data;
 
-    const QV4::IdentifierHash properties = data->propertyNames();
-    if (properties.count())
-        idx = properties.value(name);
-
+    const int idx = data->propertyIndex(name);
     if (idx == -1) {
         if (QObject *obj = data->contextObject()) {
             QQmlPropertyData local;
@@ -484,7 +479,7 @@ void QQmlContextPrivate::dropDestroyedQObject(const QString &name, QObject *dest
     if (!m_data->isValid())
         return;
 
-    const int idx = m_data->propertyNames().value(name);
+    const int idx = m_data->propertyIndex(name);
     Q_ASSERT(idx >= 0);
     if (qvariant_cast<QObject *>(propertyValue(idx)) != destroyed)
         return;
diff --git a/src/qml/qml/qqmlcontextdata.cpp b/src/qml/qml/qqmlcontextdata.cpp
index 34a45c6d2d..1e8c57bf18 100644
--- a/src/qml/qml/qqmlcontextdata.cpp
+++ b/src/qml/qml/qqmlcontextdata.cpp
@@ -281,20 +281,16 @@ void QQmlContextData::setIdValue(int idx, QObject *obj)
 
 QString QQmlContextData::findObjectId(const QObject *obj) const
 {
-    const QV4::IdentifierHash &properties = propertyNames();
-    if (m_propertyNameCache.isEmpty())
-        return QString();
-
     for (int ii = 0; ii < m_idValueCount; ii++) {
         if (m_idValues[ii] == obj)
-            return properties.findId(ii);
+            return propertyName(ii);
     }
 
     if (m_publicContext) {
         QQmlContextPrivate *p = QQmlContextPrivate::get(m_publicContext);
         for (int ii = 0; ii < p->numPropertyValues(); ++ii)
             if (p->propertyValue(ii) == QVariant::fromValue(const_cast<QObject *>(obj)))
-                return properties.findId(ii);
+                return propertyName(ii);
     }
 
     if (m_linkedContext)
@@ -322,22 +318,13 @@ void QQmlContextData::addExpression(QQmlJavaScriptExpression *expression)
     expression->insertIntoList(&m_expressions);
 }
 
-QV4::IdentifierHash QQmlContextData::propertyNames() const
-{
-    if (m_propertyNameCache.isEmpty()) {
-        if (m_typeCompilationUnit)
-            m_propertyNameCache = m_typeCompilationUnit->namedObjectsPerComponent(m_componentObjectIndex);
-        else
-            m_propertyNameCache = QV4::IdentifierHash(m_engine->handle());
-    }
-    return m_propertyNameCache;
-}
-
-QV4::IdentifierHash *QQmlContextData::detachedPropertyNames()
+void QQmlContextData::initPropertyNames() const
 {
-    propertyNames();
-    m_propertyNameCache.detach();
-    return &m_propertyNameCache;
+    if (m_typeCompilationUnit)
+        m_propertyNameCache = m_typeCompilationUnit->namedObjectsPerComponent(m_componentObjectIndex);
+    else
+        m_propertyNameCache = QV4::IdentifierHash(m_engine->handle());
+    Q_ASSERT(!m_propertyNameCache.isEmpty());
 }
 
 QUrl QQmlContextData::url() const
diff --git a/src/qml/qml/qqmlcontextdata_p.h b/src/qml/qml/qqmlcontextdata_p.h
index acfcb93867..146a1c97dd 100644
--- a/src/qml/qml/qqmlcontextdata_p.h
+++ b/src/qml/qml/qqmlcontextdata_p.h
@@ -173,8 +173,29 @@ public:
         }
     }
 
-    QV4::IdentifierHash propertyNames() const;
-    QV4::IdentifierHash *detachedPropertyNames();
+    int propertyIndex(const QString &name) const
+    {
+        ensurePropertyNames();
+        return m_propertyNameCache.value(name);
+    }
+
+    int propertyIndex(QV4::String *name) const
+    {
+        ensurePropertyNames();
+        return m_propertyNameCache.value(name);
+    }
+
+    QString propertyName(int index) const
+    {
+        ensurePropertyNames();
+        return m_propertyNameCache.findId(index);
+    }
+
+    void addPropertyNameAndIndex(const QString &name, int index)
+    {
+        Q_ASSERT(!m_propertyNameCache.isEmpty());
+        m_propertyNameCache.add(name, index);
+    }
 
     void setExpressions(QQmlJavaScriptExpression *expressions) { m_expressions = expressions; }
     QQmlJavaScriptExpression *takeExpressions()
@@ -343,6 +364,14 @@ private:
 
     void refreshExpressionsRecursive(bool isGlobal);
     void refreshExpressionsRecursive(QQmlJavaScriptExpression *);
+    void initPropertyNames() const;
+
+    void ensurePropertyNames() const
+    {
+        if (m_propertyNameCache.isEmpty())
+            initPropertyNames();
+        Q_ASSERT(!m_propertyNameCache.isEmpty());
+    }
 
     // My parent context and engine
     QQmlContextData *m_parent = nullptr;