QXmlStream: don't create QStrings just to look up entities in a hash

Move the entity's name into the Entity struct and use a QStringView
into Entity::name as the hash key, instead of a QString.

This avoids having to create a QString from QStringRefs every time an
entity is looked up.

Only costs 280B in text size on optimized GCC 6.1 Linux AMD64 builds.

Change-Id: I8dbd04c90c1ff684b794685025204abc89f68a84
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>

3 files changed, 63 insertions, 39 deletions

diff --git a/src/corelib/xml/qxmlstream.cpp b/src/corelib/xml/qxmlstream.cpp
index 195466e803..6dfa6744b8 100644
--- a/src/corelib/xml/qxmlstream.cpp
+++ b/src/corelib/xml/qxmlstream.cpp
@@ -796,11 +796,17 @@ QXmlStreamReaderPrivate::QXmlStreamReaderPrivate(QXmlStreamReader *q)
     reallocateStack();
     entityResolver = 0;
     init();
-    entityHash.insert(QLatin1String("lt"), Entity::createLiteral(QLatin1String("<")));
-    entityHash.insert(QLatin1String("gt"), Entity::createLiteral(QLatin1String(">")));
-    entityHash.insert(QLatin1String("amp"), Entity::createLiteral(QLatin1String("&")));
-    entityHash.insert(QLatin1String("apos"), Entity::createLiteral(QLatin1String("'")));
-    entityHash.insert(QLatin1String("quot"), Entity::createLiteral(QLatin1String("\"")));
+#define ADD_PREDEFINED(n, v) \
+    do { \
+        Entity e = Entity::createLiteral(QLatin1String(n), QLatin1String(v)); \
+        entityHash.insert(qToStringViewIgnoringNull(e.name), std::move(e)); \
+    } while (false)
+    ADD_PREDEFINED("lt", "<");
+    ADD_PREDEFINED("gt", ">");
+    ADD_PREDEFINED("amp", "&");
+    ADD_PREDEFINED("apos", "'");
+    ADD_PREDEFINED("quot", "\"");
+#undef ADD_PREDEFINED
 }
 
 void QXmlStreamReaderPrivate::init()
diff --git a/src/corelib/xml/qxmlstream.g b/src/corelib/xml/qxmlstream.g
index 73085bda8a..fd69a6e4af 100644
--- a/src/corelib/xml/qxmlstream.g
+++ b/src/corelib/xml/qxmlstream.g
@@ -258,20 +258,24 @@ public:
     int readBufferPos;
     QXmlStreamSimpleStack<uint> putStack;
     struct Entity {
-        Entity(const QString& str = QString())
-            :value(str), external(false), unparsed(false), literal(false),
+        Entity() = default;
+        Entity(const QString &name, const QString &value)
+          :  name(name), value(value), external(false), unparsed(false), literal(false),
              hasBeenParsed(false), isCurrentlyReferenced(false){}
-        static inline Entity createLiteral(const QString &entity)
-            { Entity result(entity); result.literal = result.hasBeenParsed = true; return result; }
-        QString value;
+        static inline Entity createLiteral(QLatin1String name, QLatin1String value)
+            { Entity result(name, value); result.literal = result.hasBeenParsed = true; return result; }
+        QString name, value;
         uint external : 1;
         uint unparsed : 1;
         uint literal : 1;
         uint hasBeenParsed : 1;
         uint isCurrentlyReferenced : 1;
     };
-    QHash<QString, Entity> entityHash;
-    QHash<QString, Entity> parameterEntityHash;
+    // these hash tables use a QStringView as a key to avoid creating QStrings
+    // just for lookup. The keys are usually views into Entity::name and thus
+    // are guaranteed to have the same lifetime as the referenced data:
+    QHash<QStringView, Entity> entityHash;
+    QHash<QStringView, Entity> parameterEntityHash;
     QXmlStreamSimpleStack<Entity *>entityReferenceStack;
     inline bool referenceEntity(Entity &entity) {
         if (entity.isCurrentlyReferenced) {
@@ -424,6 +428,11 @@ public:
         const Value &symbol = sym(index);
         return QStringRef(&textBuffer, symbol.pos + symbol.prefix, symbol.len - symbol.prefix);
     }
+    QStringView symView(int index) const
+    {
+        const Value &symbol = sym(index);
+        return QStringView(textBuffer.data() + symbol.pos, symbol.len).mid(symbol.prefix);
+    }
     inline QStringRef symName(int index) {
         const Value &symbol = sym(index);
         return QStringRef(&textBuffer, symbol.pos, symbol.len);
@@ -1142,13 +1151,13 @@ entity_decl ::= entity_decl_start entity_value space_opt RANGLE;
             EntityDeclaration &entityDeclaration = entityDeclarations.top();
             if (!entityDeclaration.external)
                 entityDeclaration.value = symString(2);
-            QString entityName = entityDeclaration.name.toString();
-            QHash<QString, Entity> &hash = entityDeclaration.parameter ? parameterEntityHash : entityHash;
-            if (!hash.contains(entityName)) {
-                Entity entity(entityDeclaration.value.toString());
+            auto &hash = entityDeclaration.parameter ? parameterEntityHash : entityHash;
+            if (!hash.contains(qToStringViewIgnoringNull(entityDeclaration.name))) {
+                Entity entity(entityDeclaration.name.toString(),
+                              entityDeclaration.value.toString());
                 entity.unparsed = (!entityDeclaration.notationName.isNull());
                 entity.external = entityDeclaration.external;
-                hash.insert(entityName, entity);
+                hash.insert(qToStringViewIgnoringNull(entity.name), entity);
             }
         } break;
 ./
@@ -1612,7 +1621,7 @@ entity_ref ::= AMPERSAND name SEMICOLON;
 /.
         case $rule_number: {
             sym(1).len += sym(2).len + 1;
-            QString reference = symString(2).toString();
+            QStringView reference = symView(2);
             if (entityHash.contains(reference)) {
                 Entity &entity = entityHash[reference];
                 if (entity.unparsed) {
@@ -1633,7 +1642,7 @@ entity_ref ::= AMPERSAND name SEMICOLON;
             }
 
             if (entityResolver) {
-                QString replacementText = resolveUndeclaredEntity(reference);
+                QString replacementText = resolveUndeclaredEntity(reference.toString());
                 if (!replacementText.isNull()) {
                     putReplacement(replacementText);
                     textBuffer.chop(2 + sym(2).len);
@@ -1654,7 +1663,7 @@ pereference ::= PERCENT name SEMICOLON;
 /.
         case $rule_number: {
             sym(1).len += sym(2).len + 1;
-            QString reference = symString(2).toString();
+            QStringView reference = symView(2);
             if (parameterEntityHash.contains(reference)) {
                 referenceToParameterEntityDetected = true;
                 Entity &entity = parameterEntityHash[reference];
@@ -1685,7 +1694,7 @@ entity_ref_in_attribute_value ::= AMPERSAND name SEMICOLON;
 /.
         case $rule_number: {
             sym(1).len += sym(2).len + 1;
-            QString reference = symString(2).toString();
+            QStringView reference = symView(2);
             if (entityHash.contains(reference)) {
                 Entity &entity = entityHash[reference];
                 if (entity.unparsed || entity.value.isNull()) {
@@ -1706,7 +1715,7 @@ entity_ref_in_attribute_value ::= AMPERSAND name SEMICOLON;
             }
 
             if (entityResolver) {
-                QString replacementText = resolveUndeclaredEntity(reference);
+                QString replacementText = resolveUndeclaredEntity(reference.toString());
                 if (!replacementText.isNull()) {
                     putReplacement(replacementText);
                     textBuffer.chop(2 + sym(2).len);
diff --git a/src/corelib/xml/qxmlstream_p.h b/src/corelib/xml/qxmlstream_p.h
index 8ef6e4b8fe..5645d812eb 100644
--- a/src/corelib/xml/qxmlstream_p.h
+++ b/src/corelib/xml/qxmlstream_p.h
@@ -754,20 +754,24 @@ public:
     int readBufferPos;
     QXmlStreamSimpleStack<uint> putStack;
     struct Entity {
-        Entity(const QString& str = QString())
-            :value(str), external(false), unparsed(false), literal(false),
+        Entity() = default;
+        Entity(const QString &name, const QString &value)
+          :  name(name), value(value), external(false), unparsed(false), literal(false),
              hasBeenParsed(false), isCurrentlyReferenced(false){}
-        static inline Entity createLiteral(const QString &entity)
-            { Entity result(entity); result.literal = result.hasBeenParsed = true; return result; }
-        QString value;
+        static inline Entity createLiteral(QLatin1String name, QLatin1String value)
+            { Entity result(name, value); result.literal = result.hasBeenParsed = true; return result; }
+        QString name, value;
         uint external : 1;
         uint unparsed : 1;
         uint literal : 1;
         uint hasBeenParsed : 1;
         uint isCurrentlyReferenced : 1;
     };
-    QHash<QString, Entity> entityHash;
-    QHash<QString, Entity> parameterEntityHash;
+    // these hash tables use a QStringView as a key to avoid creating QStrings
+    // just for lookup. The keys are usually views into Entity::name and thus
+    // are guaranteed to have the same lifetime as the referenced data:
+    QHash<QStringView, Entity> entityHash;
+    QHash<QStringView, Entity> parameterEntityHash;
     QXmlStreamSimpleStack<Entity *>entityReferenceStack;
     inline bool referenceEntity(Entity &entity) {
         if (entity.isCurrentlyReferenced) {
@@ -920,6 +924,11 @@ public:
         const Value &symbol = sym(index);
         return QStringRef(&textBuffer, symbol.pos + symbol.prefix, symbol.len - symbol.prefix);
     }
+    QStringView symView(int index) const
+    {
+        const Value &symbol = sym(index);
+        return QStringView(textBuffer.data() + symbol.pos, symbol.len).mid(symbol.prefix);
+    }
     inline QStringRef symName(int index) {
         const Value &symbol = sym(index);
         return QStringRef(&textBuffer, symbol.pos, symbol.len);
@@ -1490,13 +1499,13 @@ bool QXmlStreamReaderPrivate::parse()
             EntityDeclaration &entityDeclaration = entityDeclarations.top();
             if (!entityDeclaration.external)
                 entityDeclaration.value = symString(2);
-            QString entityName = entityDeclaration.name.toString();
-            QHash<QString, Entity> &hash = entityDeclaration.parameter ? parameterEntityHash : entityHash;
-            if (!hash.contains(entityName)) {
-                Entity entity(entityDeclaration.value.toString());
+            auto &hash = entityDeclaration.parameter ? parameterEntityHash : entityHash;
+            if (!hash.contains(qToStringViewIgnoringNull(entityDeclaration.name))) {
+                Entity entity(entityDeclaration.name.toString(),
+                              entityDeclaration.value.toString());
                 entity.unparsed = (!entityDeclaration.notationName.isNull());
                 entity.external = entityDeclaration.external;
-                hash.insert(entityName, entity);
+                hash.insert(qToStringViewIgnoringNull(entity.name), entity);
             }
         } break;
 
@@ -1791,7 +1800,7 @@ bool QXmlStreamReaderPrivate::parse()
 
         case 240: {
             sym(1).len += sym(2).len + 1;
-            QString reference = symString(2).toString();
+            QStringView reference = symView(2);
             if (entityHash.contains(reference)) {
                 Entity &entity = entityHash[reference];
                 if (entity.unparsed) {
@@ -1812,7 +1821,7 @@ bool QXmlStreamReaderPrivate::parse()
             }
 
             if (entityResolver) {
-                QString replacementText = resolveUndeclaredEntity(reference);
+                QString replacementText = resolveUndeclaredEntity(reference.toString());
                 if (!replacementText.isNull()) {
                     putReplacement(replacementText);
                     textBuffer.chop(2 + sym(2).len);
@@ -1830,7 +1839,7 @@ bool QXmlStreamReaderPrivate::parse()
 
         case 241: {
             sym(1).len += sym(2).len + 1;
-            QString reference = symString(2).toString();
+            QStringView reference = symView(2);
             if (parameterEntityHash.contains(reference)) {
                 referenceToParameterEntityDetected = true;
                 Entity &entity = parameterEntityHash[reference];
@@ -1853,7 +1862,7 @@ bool QXmlStreamReaderPrivate::parse()
 
         case 243: {
             sym(1).len += sym(2).len + 1;
-            QString reference = symString(2).toString();
+            QStringView reference = symView(2);
             if (entityHash.contains(reference)) {
                 Entity &entity = entityHash[reference];
                 if (entity.unparsed || entity.value.isNull()) {
@@ -1874,7 +1883,7 @@ bool QXmlStreamReaderPrivate::parse()
             }
 
             if (entityResolver) {
-                QString replacementText = resolveUndeclaredEntity(reference);
+                QString replacementText = resolveUndeclaredEntity(reference.toString());
                 if (!replacementText.isNull()) {
                     putReplacement(replacementText);
                     textBuffer.chop(2 + sym(2).len);