From: Aaron Kennedy Date: Thu, 3 Nov 2011 16:52:53 +0000 (+0000) Subject: Optimization: Minimize refreshing when root context changes X-Git-Url: http://git.silmor.de/gitweb/?a=commitdiff_plain;h=3e84e81da776234bf62570b439f1e21848f83c4b;p=konrad%2Fqtdeclarative.git 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 --- diff --git a/src/declarative/qml/qdeclarativecontext.cpp b/src/declarative/qml/qdeclarativecontext.cpp index d7a6424..1723603 100644 --- a/src/declarative/qml/qdeclarativecontext.cpp +++ b/src/declarative/qml/qdeclarativecontext.cpp @@ -511,18 +511,18 @@ QObject *QDeclarativeContextPrivate::context_at(QDeclarativeListPropertyrefresh(); } -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 --- 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 --- a/src/declarative/qml/v8/qv8contextwrapper.cpp +++ b/src/declarative/qml/v8/qv8contextwrapper.cpp @@ -258,6 +258,7 @@ v8::Handle QV8ContextWrapper::Getter(v8::Local 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 QV8ContextWrapper::Getter(v8::Local 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 QV8ContextWrapper::Setter(v8::Local 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 QV8ContextWrapper::Setter(v8::Local property, context = context->parent; } + expressionContext->unresolvedNames = true; + if (!resource->readOnly) { return v8::Handle(); } else { diff --git a/tests/auto/declarative/qdeclarativecontext/data/refreshExpressionsRootContext.qml b/tests/auto/declarative/qdeclarativecontext/data/refreshExpressionsRootContext.qml new file mode 100644 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 --- 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"