Speed up string handling from QML cache files

Currently when extracting a string from a compilation unit, we copy the
data. This happens for example when instantiating objects and setting
string properties and it also happens when creating the JS runtime
strings from the compilation unit.

Since QML cache files that are mapped into memory from disk, we can
avoid the copy by keeping the files mapped and making sure that the
in-memory representation is compatible with QStringData.

This optimization is limited to little-endian architectures.

Task-number: QTBUG-63068
Change-Id: I2450aacd3bf1eda3e5be4264149b23f0281d8b4e
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
Reviewed-by: Erik Verbruggen <erik.verbruggen@qt.io>

4 files changed, 53 insertions, 10 deletions

diff --git a/src/qml/compiler/qv4compilationunitmapper_unix.cpp b/src/qml/compiler/qv4compilationunitmapper_unix.cpp
index 8348613888..1fef4d38f4 100644
--- a/src/qml/compiler/qv4compilationunitmapper_unix.cpp
+++ b/src/qml/compiler/qv4compilationunitmapper_unix.cpp
@@ -92,8 +92,16 @@ CompiledData::Unit *CompilationUnitMapper::open(const QString &cacheFileName, co
 
 void CompilationUnitMapper::close()
 {
-    if (dataPtr != nullptr)
-        munmap(dataPtr, length);
+    // Do not unmap the data here.
+    if (dataPtr != nullptr) {
+        // Do not unmap cache files that are built with the StaticData flag. That's the majority of
+        // them and it's necessary to benefit from the QString literal optimization. There might
+        // still be QString instances around that point into that memory area. The memory is backed
+        // on the disk, so the kernel is free to release the pages and all that remains is the
+        // address space allocation.
+        if (!(reinterpret_cast<CompiledData::Unit*>(dataPtr)->flags & CompiledData::Unit::StaticData))
+            munmap(dataPtr, length);
+    }
     dataPtr = nullptr;
 }
 
diff --git a/src/qml/compiler/qv4compilationunitmapper_win.cpp b/src/qml/compiler/qv4compilationunitmapper_win.cpp
index 29e3e2ac01..3e44d045fc 100644
--- a/src/qml/compiler/qv4compilationunitmapper_win.cpp
+++ b/src/qml/compiler/qv4compilationunitmapper_win.cpp
@@ -113,8 +113,15 @@ CompiledData::Unit *CompilationUnitMapper::open(const QString &cacheFileName, co
 
 void CompilationUnitMapper::close()
 {
-    if (dataPtr != nullptr)
-        UnmapViewOfFile(dataPtr);
+    if (dataPtr != nullptr) {
+        // Do not unmap cache files that are built with the StaticData flag. That's the majority of
+        // them and it's necessary to benefit from the QString literal optimization. There might
+        // still be QString instances around that point into that memory area. The memory is backed
+        // on the disk, so the kernel is free to release the pages and all that remains is the
+        // address space allocation.
+        if (!(reinterpret_cast<CompiledData::Unit*>(dataPtr)->flags & CompiledData::Unit::StaticData))
+            UnmapViewOfFile(dataPtr);
+    }
     dataPtr = nullptr;
 }
 
diff --git a/src/qml/compiler/qv4compileddata_p.h b/src/qml/compiler/qv4compileddata_p.h
index 299c1a9f19..239c12acfd 100644
--- a/src/qml/compiler/qv4compileddata_p.h
+++ b/src/qml/compiler/qv4compileddata_p.h
@@ -186,16 +186,39 @@ struct JSClass
 };
 static_assert(sizeof(JSClass) == 4, "JSClass structure needs to have the expected size to be binary compatible on disk when generated by host compiler and loaded by target");
 
+// This data structure is intended to be binary compatible with QStringData/QStaticStringData on
+// 64-bit and 32-bit little-endian architectures, in all directions. So the same structure mapped
+// from a file must be castable to a QStringData regardless of the pointer size. With the first
+// few fields that's easy, they're always 32-bit. However the offset field of QArrayData is a
+// ptrdiff_t and thus variable in size.
+// On 64-bit systems compilers enforce an 8-byte alignment and thus place it at offset 16, while
+// on 32-bit systems offset 12 is sufficient. Therefore the two values don't overlap and contain
+// the same value.
 struct String
 {
+    qint32_le refcount; // -1
     qint32_le size;
+    quint32_le allocAndCapacityReservedFlag; // 0
+    quint32_le offsetOn32Bit;
+    quint64_le offsetOn64Bit;
     // uint16 strdata[]
 
     static int calculateSize(const QString &str) {
-        return (sizeof(String) + str.length() * sizeof(quint16) + 7) & ~0x7;
+        return (sizeof(String) + (str.length() + 1) * sizeof(quint16) + 7) & ~0x7;
     }
 };
-static_assert(sizeof(String) == 4, "String structure needs to have the expected size to be binary compatible on disk when generated by host compiler and loaded by target");
+static_assert(sizeof(String) == 24, "String structure needs to have the expected size to be binary compatible on disk when generated by host compiler and loaded by target");
+
+// Ensure compatibility with QString
+static_assert(offsetof(QArrayData, ref) == offsetof(String, refcount), "refcount must be at the same location");
+static_assert(offsetof(QArrayData, size) == offsetof(String, size), "size must be at the same location");
+static_assert(offsetof(String, offsetOn64Bit) == 16, "offset must be at 8-byte aligned location");
+static_assert(offsetof(String, offsetOn32Bit) == 12, "offset must be at 4-byte aligned location");
+#if QT_POINTER_SIZE == 8
+static_assert(offsetof(QArrayData, offset) == offsetof(String, offsetOn64Bit), "offset must be at the same location");
+#else
+static_assert(offsetof(QArrayData, offset) == offsetof(String, offsetOn32Bit), "offset must be at the same location");
+#endif
 
 struct CodeOffsetToLine {
     quint32_le codeOffset;
@@ -770,11 +793,11 @@ struct Unit
         if (str->size == 0)
             return QString();
 #if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
+        if (flags & StaticData) {
+            const QStringDataPtr holder = { const_cast<QStringData *>(reinterpret_cast<const QStringData*>(str)) };
+            return QString(holder);
+        }
         const QChar *characters = reinterpret_cast<const QChar *>(str + 1);
-        // Too risky to do this while we unmap disk backed compilation but keep pointers to string
-        // data in the identifier tables.
-        //        if (flags & StaticData)
-        //            return QString::fromRawData(characters, str->size);
         return QString(characters, str->size);
 #else
         const quint16_le *characters = reinterpret_cast<const quint16_le *>(str + 1);
diff --git a/src/qml/compiler/qv4compiler.cpp b/src/qml/compiler/qv4compiler.cpp
index bd9dd96d93..9b60ad14e6 100644
--- a/src/qml/compiler/qv4compiler.cpp
+++ b/src/qml/compiler/qv4compiler.cpp
@@ -90,7 +90,11 @@ void QV4::Compiler::StringTableGenerator::serialize(CompiledData::Unit *unit)
         const QString &qstr = strings.at(i);
 
         QV4::CompiledData::String *s = reinterpret_cast<QV4::CompiledData::String *>(stringData);
+        s->refcount = -1;
         s->size = qstr.length();
+        s->allocAndCapacityReservedFlag = 0;
+        s->offsetOn32Bit = sizeof(QV4::CompiledData::String);
+        s->offsetOn64Bit = sizeof(QV4::CompiledData::String);
 #if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
         memcpy(s + 1, qstr.constData(), qstr.length()*sizeof(ushort));
 #else
@@ -98,6 +102,7 @@ void QV4::Compiler::StringTableGenerator::serialize(CompiledData::Unit *unit)
         for (int i = 0; i < qstr.length(); ++i)
             uc[i] = qToLittleEndian<ushort>(qstr.at(i).unicode());
 #endif
+        reinterpret_cast<ushort *>(s + 1)[s->size] = 0;
 
         stringData += QV4::CompiledData::String::calculateSize(qstr);
     }