V4 IR: Change data type used in RemoveSharedExpressions pass.

Profiling the loading of a pretty morbidly large QML file consistently showed
that this was quite slow, around 1300ms in removeSharedExpressions, of which a
good >600-700ms (I didn't count exactly, but it was a very large amount) was
down to allocating and freeing QHash nodes.

As we don't require removals, leaving insertion and lookup as the only two
remaining options, a sorted vector becomes a viable alternative (credit to João
Abecasis for the idea).

An additional benefit of this change is that the two hash lookups are now
compressed into a single 'hash' lookup (via the lower_bound call) instead of
separately using contains() / insert().

Measuring the exact saving is difficult, but it looks like this saves between
700-1000ms off the runtime RemoveSharedExpressions. After this patch, malloc and
free are dominating the optimizer run, instead of any particular method.

Change-Id: I6c0bb8495eac4dd3613ba0274e8802d7bd609460
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

diff --git a/src/qml/compiler/qv4jsir.cpp b/src/qml/compiler/qv4jsir.cpp
index e23ca1d..fbfcf31 100644 (file)
--- a/src/qml/compiler/qv4jsir.cpp
+++ b/src/qml/compiler/qv4jsir.cpp
@@ -44,6 +44,8 @@
 #include <QtCore/qset.h>
 #include <cmath>
 
+#include <vector>
+
 #ifdef CONST
 #undef CONST
 #endif
@@ -152,7 +154,7 @@ AluOp binaryOperator(int op)
 struct RemoveSharedExpressions: IR::StmtVisitor, IR::ExprVisitor
 {
     CloneExpr clone;
-    QSet<Expr *> subexpressions; // contains all the non-cloned subexpressions in the given function
+    std::vector<Expr *> subexpressions; // contains all the non-cloned subexpressions in the given function. sorted using std::lower_bound.
     Expr *uniqueExpr;
 
     RemoveSharedExpressions(): uniqueExpr(0) {}
@@ -176,18 +178,19 @@ struct RemoveSharedExpressions: IR::StmtVisitor, IR::ExprVisitor
     template <typename _Expr>
     _Expr *cleanup(_Expr *expr)
     {
-        if (subexpressions.contains(expr)) {
-             // the cloned expression is unique by definition
-            // so we don't need to add it to `subexpressions'.
-            return clone(expr);
+        std::vector<Expr *>::iterator it = std::lower_bound(subexpressions.begin(), subexpressions.end(), expr);
+        if (it == subexpressions.end() || *it != expr) {
+            subexpressions.insert(it, expr);
+            IR::Expr *e = expr;
+            qSwap(uniqueExpr, e);
+            expr->accept(this);
+            qSwap(uniqueExpr, e);
+            return static_cast<_Expr *>(e);
         }
 
-        subexpressions.insert(expr);
-        IR::Expr *e = expr;
-        qSwap(uniqueExpr, e);
-        expr->accept(this);
-        qSwap(uniqueExpr, e);
-        return static_cast<_Expr *>(e);
+        // the cloned expression is unique by definition
+        // so we don't need to add it to `subexpressions'.
+        return clone(expr);
     }
 
     // statements