IdentifierTable: Optimize property key lookup by QString

We only have to create QV4::String if the entry doesn't exist, yet.

Also, make sure the identifier of a new QV4::String inserted for an
array index is actually populated.

Change-Id: I223d191905baea5e537f483a9b3aa3db26f891b2
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>

3 files changed, 29 insertions, 18 deletions

diff --git a/src/qml/jsruntime/qv4identifiertable.cpp b/src/qml/jsruntime/qv4identifiertable.cpp
index c4ce837360..7c5bc3188c 100644
--- a/src/qml/jsruntime/qv4identifiertable.cpp
+++ b/src/qml/jsruntime/qv4identifiertable.cpp
@@ -141,8 +141,14 @@ Heap::String *IdentifierTable::insertString(const QString &s)
         Heap::String *str = engine->newString(s);
         str->stringHash = hash;
         str->subtype = subtype;
+        str->identifier = PropertyKey::fromArrayIndex(hash);
         return str;
     }
+    return resolveStringEntry(s, hash, subtype);
+}
+
+Heap::String *IdentifierTable::resolveStringEntry(const QString &s, uint hash, uint subtype)
+{
     uint idx = hash % alloc;
     while (Heap::StringOrSymbol *e = entriesByHash[idx]) {
         if (e->stringHash == hash && e->toQString() == s)
@@ -281,30 +287,20 @@ void IdentifierTable::sweep()
 
 PropertyKey IdentifierTable::asPropertyKey(const QString &s)
 {
-    return insertString(s)->identifier;
+    uint subtype;
+    const uint hash = String::createHashValue(s.constData(), s.length(), &subtype);
+    if (subtype == Heap::String::StringType_ArrayIndex)
+        return PropertyKey::fromArrayIndex(hash);
+    return resolveStringEntry(s, hash, subtype)->identifier;
 }
 
 PropertyKey IdentifierTable::asPropertyKey(const char *s, int len)
 {
     uint subtype;
     uint hash = String::createHashValue(s, len, &subtype);
-    if (hash == UINT_MAX)
-        return asPropertyKey(QString::fromUtf8(s, len));
-
-    QLatin1String latin(s, len);
-    uint idx = hash % alloc;
-    while (Heap::StringOrSymbol *e = entriesByHash[idx]) {
-        if (e->stringHash == hash && e->toQString() == latin)
-            return e->identifier;
-        ++idx;
-        idx %= alloc;
-    }
-
-    Heap::String *str = engine->newString(QString::fromLatin1(s, len));
-    str->stringHash = hash;
-    str->subtype = subtype;
-    addEntry(str);
-    return str->identifier;
+    if (subtype == Heap::String::StringType_ArrayIndex)
+        return PropertyKey::fromArrayIndex(hash);
+    return resolveStringEntry(QString::fromLatin1(s, len), hash, subtype)->identifier;
 }
 
 }
diff --git a/src/qml/jsruntime/qv4identifiertable_p.h b/src/qml/jsruntime/qv4identifiertable_p.h
index 6553ad6103..b2a9bc1195 100644
--- a/src/qml/jsruntime/qv4identifiertable_p.h
+++ b/src/qml/jsruntime/qv4identifiertable_p.h
@@ -109,6 +109,9 @@ public:
     void removeIdentifierHash(IdentifierHashData *h) {
         idHashes.remove(h);
     }
+
+private:
+    Heap::String *resolveStringEntry(const QString &s, uint hash, uint subtype);
 };
 
 }
diff --git a/tests/auto/qml/qv4identifiertable/tst_qv4identifiertable.cpp b/tests/auto/qml/qv4identifiertable/tst_qv4identifiertable.cpp
index 157d0f2a62..e51cca9be4 100644
--- a/tests/auto/qml/qv4identifiertable/tst_qv4identifiertable.cpp
+++ b/tests/auto/qml/qv4identifiertable/tst_qv4identifiertable.cpp
@@ -43,6 +43,7 @@ private slots:
     void dontSweepAcrossBucketBoundaries();
     void sweepAcrossBucketBoundariesIfFirstBucketFull();
     void sweepBucketGap();
+    void insertNumericStringPopulatesIdentifier();
 };
 
 void tst_qv4identifiertable::sweepFirstEntryInBucket()
@@ -359,6 +360,17 @@ void tst_qv4identifiertable::sweepBucketGap()
     QCOMPARE(table.entriesByHash[3], nullptr);
 }
 
+void tst_qv4identifiertable::insertNumericStringPopulatesIdentifier()
+{
+    QV4::ExecutionEngine engine;
+    const QString numeric = QStringLiteral("1");
+    uint subtype;
+    const uint hash = QV4::String::createHashValue(numeric.constData(), numeric.length(), &subtype);
+    QCOMPARE(subtype, QV4::Heap::String::StringType_ArrayIndex);
+    QCOMPARE(engine.identifierTable->insertString(numeric)->identifier,
+             QV4::PropertyKey::fromArrayIndex(hash));
+}
+
 QTEST_MAIN(tst_qv4identifiertable)
 
 #include "tst_qv4identifiertable.moc"