Optimization: Minimize refreshing when root context changes

Modifying a context requires expressions to be refreshed incase they
depend on a name resolution that changes.  As it is common to modify
the root context, and the root context can't hide any names, it is easy
to constrain the expressions that require refreshing.  In a well
behaving app, this should mean that no expressions are reevaluated when
the root context is modified.

Change-Id: Id3b48cd595fdd6b5a3dc6f26319f652dccaef79c
Reviewed-by: Roberto Raggi <roberto.raggi@nokia.com>

diff --git a/src/declarative/qml/qdeclarativecontext.cpp b/src/declarative/qml/qdeclarativecontext.cpp
index d7a6424..1723603 100644 (file)
--- a/src/declarative/qml/qdeclarativecontext.cpp
+++ b/src/declarative/qml/qdeclarativecontext.cpp
@@ -511,18 +511,18 @@ QObject *QDeclarativeContextPrivate::context_at(QDeclarativeListProperty<QObject
 
 QDeclarativeContextData::QDeclarativeContextData()
 : parent(0), engine(0), isInternal(false), ownedByParent(false), isJSContext(false), 
-  isPragmaLibraryContext(false), publicContext(0), activeVMEData(0), propertyNames(0), contextObject(0),
-  imports(0), childContexts(0), nextChild(0), prevChild(0), expressions(0), contextObjects(0), 
-  contextGuards(0), idValues(0), idValueCount(0), linkedContext(0), componentAttached(0), 
-  v4bindings(0), v8bindings(0)
+  isPragmaLibraryContext(false), unresolvedNames(false), publicContext(0), activeVMEData(0),
+  propertyNames(0), contextObject(0), imports(0), childContexts(0), nextChild(0), prevChild(0),
+  expressions(0), contextObjects(0), contextGuards(0), idValues(0), idValueCount(0), linkedContext(0),
+  componentAttached(0), v4bindings(0), v8bindings(0)
 {
 }
 
 QDeclarativeContextData::QDeclarativeContextData(QDeclarativeContext *ctxt)
 : parent(0), engine(0), isInternal(false), ownedByParent(false), isJSContext(false), 
-  isPragmaLibraryContext(false), publicContext(ctxt), activeVMEData(0), propertyNames(0),
-  contextObject(0), imports(0), childContexts(0), nextChild(0), prevChild(0), expressions(0), 
-  contextObjects(0), contextGuards(0), idValues(0), idValueCount(0), linkedContext(0), 
+  isPragmaLibraryContext(false), unresolvedNames(false), publicContext(ctxt), activeVMEData(0),
+  propertyNames(0), contextObject(0), imports(0), childContexts(0), nextChild(0), prevChild(0),
+  expressions(0), contextObjects(0), contextGuards(0), idValues(0), idValueCount(0), linkedContext(0),
   componentAttached(0), v4bindings(0), v8bindings(0)
 {
 }
@@ -664,26 +664,31 @@ void QDeclarativeContextData::refreshExpressionsRecursive(QDeclarativeAbstractEx
         expression->refresh();
 }
 
-void QDeclarativeContextData::refreshExpressionsRecursive()
+static inline bool expressions_to_run(QDeclarativeContextData *ctxt, bool isGlobalRefresh)
+{
+    return ctxt->expressions && (!isGlobalRefresh || ctxt->unresolvedNames);
+}
+
+void QDeclarativeContextData::refreshExpressionsRecursive(bool isGlobal)
 {
     // For efficiency, we try and minimize the number of guards we have to create
-    if (expressions && (nextChild || childContexts)) {
+    if (expressions_to_run(this, isGlobal) && (nextChild || childContexts)) {
         QDeclarativeGuardedContextData guard(this);
 
         if (childContexts)
-            childContexts->refreshExpressionsRecursive();
+            childContexts->refreshExpressionsRecursive(isGlobal);
 
         if (guard.isNull()) return;
 
         if (nextChild)
-            nextChild->refreshExpressionsRecursive();
+            nextChild->refreshExpressionsRecursive(isGlobal);
 
         if (guard.isNull()) return;
 
-        if (expressions)
+        if (expressions_to_run(this, isGlobal))
             refreshExpressionsRecursive(expressions);
 
-    } else if (expressions) {
+    } else if (expressions_to_run(this, isGlobal)) {
 
         refreshExpressionsRecursive(expressions);
 
@@ -691,18 +696,18 @@ void QDeclarativeContextData::refreshExpressionsRecursive()
 
         QDeclarativeGuardedContextData guard(this);
 
-        childContexts->refreshExpressionsRecursive();
+        childContexts->refreshExpressionsRecursive(isGlobal);
 
         if (!guard.isNull() && nextChild)
-            nextChild->refreshExpressionsRecursive();
+            nextChild->refreshExpressionsRecursive(isGlobal);
 
     } else if (nextChild) {
 
-        nextChild->refreshExpressionsRecursive();
+        nextChild->refreshExpressionsRecursive(isGlobal);
 
     } else if (childContexts) {
 
-        childContexts->refreshExpressionsRecursive();
+        childContexts->refreshExpressionsRecursive(isGlobal);
 
     }
 }
@@ -712,22 +717,24 @@ void QDeclarativeContextData::refreshExpressionsRecursive()
 // *structure* (not values) changes.
 void QDeclarativeContextData::refreshExpressions()
 {
+    bool isGlobal = (parent == 0);
+
     // For efficiency, we try and minimize the number of guards we have to create
-    if (expressions && childContexts) {
+    if (expressions_to_run(this, isGlobal) && childContexts) {
         QDeclarativeGuardedContextData guard(this);
 
-        childContexts->refreshExpressionsRecursive();
+        childContexts->refreshExpressionsRecursive(isGlobal);
 
-        if (!guard.isNull() && expressions)
+        if (!guard.isNull() && expressions_to_run(this, isGlobal))
             refreshExpressionsRecursive(expressions);
 
-    } else if (expressions) {
+    } else if (expressions_to_run(this, isGlobal)) {
 
         refreshExpressionsRecursive(expressions);
 
     } else if (childContexts) {
 
-        childContexts->refreshExpressionsRecursive();
+        childContexts->refreshExpressionsRecursive(isGlobal);
 
     }
 }

diff --git a/src/declarative/qml/qdeclarativecontext_p.h b/src/declarative/qml/qdeclarativecontext_p.h
index 6676a06..fb6473c 100644 (file)
--- a/src/declarative/qml/qdeclarativecontext_p.h
+++ b/src/declarative/qml/qdeclarativecontext_p.h
@@ -143,6 +143,7 @@ public:
     quint32 ownedByParent:1; // unrelated to isInternal; parent context deletes children if true.
     quint32 isJSContext:1;
     quint32 isPragmaLibraryContext:1;
+    quint32 unresolvedNames:1; // True if expressions in this context failed to resolve a toplevel name
     quint32 dummy:28;
     QDeclarativeContext *publicContext;
 
@@ -216,7 +217,7 @@ public:
     }
 
 private:
-    void refreshExpressionsRecursive();
+    void refreshExpressionsRecursive(bool isGlobal);
     void refreshExpressionsRecursive(QDeclarativeAbstractExpression *);
     ~QDeclarativeContextData() {}
 };

diff --git a/src/declarative/qml/v8/qv8contextwrapper.cpp b/src/declarative/qml/v8/qv8contextwrapper.cpp
index 46cf0e4..20e479b 100644 (file)
--- a/src/declarative/qml/v8/qv8contextwrapper.cpp
+++ b/src/declarative/qml/v8/qv8contextwrapper.cpp
@@ -258,6 +258,7 @@ v8::Handle<v8::Value> QV8ContextWrapper::Getter(v8::Local<v8::String> property,
     // Its possible we could delay the calculation of the "actual" context (in the case
     // of sub contexts) until it is definately needed.
     QDeclarativeContextData *context = resource->getContext();
+    QDeclarativeContextData *expressionContext = context;
 
     if (!context)
         return v8::Undefined();
@@ -362,6 +363,8 @@ v8::Handle<v8::Value> QV8ContextWrapper::Getter(v8::Local<v8::String> property,
         context = context->parent;
     }
 
+    expressionContext->unresolvedNames = true;
+
     QString error = QLatin1String("Can't find variable: ") + engine->toString(property);
     v8::ThrowException(v8::Exception::ReferenceError(engine->toString(error)));
     return v8::Undefined();
@@ -394,6 +397,7 @@ v8::Handle<v8::Value> QV8ContextWrapper::Setter(v8::Local<v8::String> property,
     // Its possible we could delay the calculation of the "actual" context (in the case
     // of sub contexts) until it is definately needed.
     QDeclarativeContextData *context = resource->getContext();
+    QDeclarativeContextData *expressionContext = context;
 
     if (!context)
         return v8::Undefined();
@@ -436,6 +440,8 @@ v8::Handle<v8::Value> QV8ContextWrapper::Setter(v8::Local<v8::String> property,
         context = context->parent;
     }
 
+    expressionContext->unresolvedNames = true;
+
     if (!resource->readOnly) {
         return v8::Handle<v8::Value>();
     } else {

diff --git a/tests/auto/declarative/qdeclarativecontext/data/refreshExpressionsRootContext.qml b/tests/auto/declarative/qdeclarativecontext/data/refreshExpressionsRootContext.qml
new file mode 100644 (file)
index 0000000..bd82cd9
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativecontext/data/refreshExpressionsRootContext.qml
@@ -0,0 +1,6 @@
+import QtQuick 2.0
+
+QtObject {
+    property var dummy: countCommand.doCommand(), unresolvedName
+}
+

diff --git a/tests/auto/declarative/qdeclarativecontext/tst_qdeclarativecontext.cpp b/tests/auto/declarative/qdeclarativecontext/tst_qdeclarativecontext.cpp
index dee2cd9..68599a2 100644 (file)
--- a/tests/auto/declarative/qdeclarativecontext/tst_qdeclarativecontext.cpp
+++ b/tests/auto/declarative/qdeclarativecontext/tst_qdeclarativecontext.cpp
@@ -78,6 +78,7 @@ private slots:
 
     void refreshExpressions();
     void refreshExpressionsCrash();
+    void refreshExpressionsRootContext();
 
 private:
     QDeclarativeEngine engine;
@@ -610,6 +611,39 @@ void tst_qdeclarativecontext::refreshExpressions()
     delete o1;
 }
 
+// Test that updating the root context, only causes expressions in contexts with an
+// unresolved name to reevaluate
+void tst_qdeclarativecontext::refreshExpressionsRootContext()
+{
+    QDeclarativeEngine engine;
+
+    CountCommand command;
+    engine.rootContext()->setContextProperty("countCommand", &command);
+
+    QDeclarativeComponent component(&engine, TEST_FILE("refreshExpressions.qml"));
+    QDeclarativeComponent component2(&engine, TEST_FILE("refreshExpressionsRootContext.qml"));
+
+    QDeclarativeContext context(engine.rootContext());
+    QDeclarativeContext context2(engine.rootContext());
+
+    QString warning = component2.url().toString() + QLatin1String(":4: ReferenceError: Can't find variable: unresolvedName");
+
+    QObject *o1 = component.create(&context);
+
+    QTest::ignoreMessage(QtWarningMsg, qPrintable(warning));
+    QObject *o2 = component2.create(&context2);
+
+    QCOMPARE(command.count, 3);
+
+    QTest::ignoreMessage(QtWarningMsg, qPrintable(warning));
+    QDeclarativeContextData::get(engine.rootContext())->refreshExpressions();
+
+    QCOMPARE(command.count, 4);
+
+    delete o2;
+    delete o1;
+}
+
 QTEST_MAIN(tst_qdeclarativecontext)
 
 #include "tst_qdeclarativecontext.moc"