more fine-grained cache disabling

Disable caching only for properties that reference product in some way.
This fixes the performance regression introduced by 0d477159e6.
Additionally, the property cache in ModuleProperties must be disabled
too. Otherwise we might return wrong cached values.

Task-number: QBS-729
Task-number: QBS-732
Change-Id: I4c46e4c5ddf20053851d7ee5e9a51f6f3f082a6d
Reviewed-by: Christian Kandeler <christian.kandeler@theqtcompany.com>

8 files changed, 42 insertions, 6 deletions

diff --git a/src/lib/corelib/language/itemreaderastvisitor.cpp b/src/lib/corelib/language/itemreaderastvisitor.cpp
index c7946f32b..178d5c9ac 100644
--- a/src/lib/corelib/language/itemreaderastvisitor.cpp
+++ b/src/lib/corelib/language/itemreaderastvisitor.cpp
@@ -454,15 +454,18 @@ bool ItemReaderASTVisitor::visitStatement(AST::Statement *statement)
     m_sourceValue->setLocation(statement->firstSourceLocation().startLine,
                                statement->firstSourceLocation().startColumn);
 
-    bool usesBase, usesOuter;
+    bool usesBase, usesOuter, usesProduct;
     IdentifierSearch idsearch;
     idsearch.add(QLatin1String("base"), &usesBase);
     idsearch.add(QLatin1String("outer"), &usesOuter);
+    idsearch.add(QLatin1String("product"), &usesProduct);
     idsearch.start(statement);
     if (usesBase)
         m_sourceValue->m_flags |= JSSourceValue::SourceUsesBase;
     if (usesOuter)
         m_sourceValue->m_flags |= JSSourceValue::SourceUsesOuter;
+    if (usesProduct)
+        m_sourceValue->m_flags |= JSSourceValue::SourceUsesProduct;
     return false;
 }
 
diff --git a/src/lib/corelib/language/projectresolver.cpp b/src/lib/corelib/language/projectresolver.cpp
index ebd3a25db..7071257e6 100644
--- a/src/lib/corelib/language/projectresolver.cpp
+++ b/src/lib/corelib/language/projectresolver.cpp
@@ -73,6 +73,7 @@ ProjectResolver::ProjectResolver(ModuleLoader *ldr, const BuiltinDeclarations *b
     , m_logger(logger)
     , m_engine(m_evaluator->engine())
     , m_progressObserver(0)
+    , m_disableCachedEvaluation(false)
 {
 }
 
@@ -994,9 +995,9 @@ void ProjectResolver::resolveProductDependencies(ProjectContext *projectContext)
             productContext.product = usedProduct;
             m_productContext = &productContext;
             const ValuePerItem oldProducts = replaceProduct(exportItem, productItemValue);
-            m_evaluator->setCachingEnabled(false);
+            m_disableCachedEvaluation = true;
             QVariantMap exportedConfig = evaluateModuleValues(exportItem);
-            m_evaluator->setCachingEnabled(true);
+            m_disableCachedEvaluation = false;
             restoreOldProducts(oldProducts);
 
             // Re-evaluate direct properties of the Export item using the exporting product.
@@ -1112,7 +1113,21 @@ QVariantMap ProjectResolver::evaluateProperties(Item *item,
             {
                 break;
             }
+
+            bool cacheDisabled = false;
+            const JSSourceValuePtr srcValue = it.value().staticCast<JSSourceValue>();
+            if (m_disableCachedEvaluation && (pd.type() == PropertyDeclaration::Path
+                                || srcValue->sourceUsesProduct())) {
+                cacheDisabled = true;
+                m_evaluator->setCachingEnabled(false);
+                m_evaluator->engine()->setPropertyCacheEnabled(false);
+            }
+
             const QScriptValue scriptValue = m_evaluator->property(item, it.key());
+            if (cacheDisabled) {
+                m_evaluator->setCachingEnabled(true);
+                m_evaluator->engine()->setPropertyCacheEnabled(true);
+            }
             if (Q_UNLIKELY(m_evaluator->engine()->hasErrorOrException(scriptValue)))
                 throw ErrorInfo(scriptValue.toString(), it.value()->location());
 
diff --git a/src/lib/corelib/language/projectresolver.h b/src/lib/corelib/language/projectresolver.h
index 05f326b2c..dbbe4096b 100644
--- a/src/lib/corelib/language/projectresolver.h
+++ b/src/lib/corelib/language/projectresolver.h
@@ -136,6 +136,7 @@ private:
     ProgressObserver *m_progressObserver;
     ProductContext *m_productContext;
     ModuleContext *m_moduleContext;
+    bool m_disableCachedEvaluation;
     QMap<QString, ResolvedProductPtr> m_productsByName;
     QHash<ResolvedProductPtr, Item *> m_productItemMap;
     mutable QHash<FileContextConstPtr, ResolvedFileContextPtr> m_fileContextMap;
diff --git a/src/lib/corelib/language/scriptengine.cpp b/src/lib/corelib/language/scriptengine.cpp
index 00d16c989..1e2d227b5 100644
--- a/src/lib/corelib/language/scriptengine.cpp
+++ b/src/lib/corelib/language/scriptengine.cpp
@@ -78,7 +78,7 @@ uint qHash(const ScriptEngine::PropertyCacheKey &k, uint seed = 0)
 }
 
 ScriptEngine::ScriptEngine(const Logger &logger, QObject *parent)
-    : QScriptEngine(parent), m_logger(logger)
+    : QScriptEngine(parent), m_propertyCacheEnabled(true), m_logger(logger)
 {
     setProcessEventsInterval(1000); // For the cancelation mechanism to work.
     m_cancelationError = currentContext()->throwValue(tr("Execution canceled"));
diff --git a/src/lib/corelib/language/scriptengine.h b/src/lib/corelib/language/scriptengine.h
index 7d70834d5..56c82e24b 100644
--- a/src/lib/corelib/language/scriptengine.h
+++ b/src/lib/corelib/language/scriptengine.h
@@ -77,6 +77,8 @@ public:
         return m_propertiesRequestedFromArtifact;
     }
 
+    void setPropertyCacheEnabled(bool enable) { m_propertyCacheEnabled = enable; }
+    bool isPropertyCacheEnabled() const { return m_propertyCacheEnabled; }
     void addToPropertyCache(const QString &moduleName, const QString &propertyName,
             bool oneValue, const PropertyMapConstPtr &propertyMap, const QVariant &value);
     QVariant retrieveFromPropertyCache(const QString &moduleName, const QString &propertyName,
@@ -154,6 +156,7 @@ private:
 
     ScriptValueCache m_scriptValueCache;
     QHash<QString, QScriptValue> m_jsImportCache;
+    bool m_propertyCacheEnabled;
     QHash<PropertyCacheKey, QVariant> m_propertyCache;
     PropertyList m_propertiesRequestedInScript;
     QHash<QString, PropertyList> m_propertiesRequestedFromArtifact;
diff --git a/src/lib/corelib/language/testdata/exports.qbs b/src/lib/corelib/language/testdata/exports.qbs
index b1adfc214..b17fbf7cf 100644
--- a/src/lib/corelib/language/testdata/exports.qbs
+++ b/src/lib/corelib/language/testdata/exports.qbs
@@ -46,4 +46,8 @@ Project {
             dummy.cxxFlags: ["-bar"]
         }
     }
+    Application {
+        name: "myapp3"
+        Depends { name: "productWithInheritedExportItem" }
+    }
 }
diff --git a/src/lib/corelib/language/tst_language.cpp b/src/lib/corelib/language/tst_language.cpp
index c38fa5876..61cfd9b51 100644
--- a/src/lib/corelib/language/tst_language.cpp
+++ b/src/lib/corelib/language/tst_language.cpp
@@ -411,7 +411,7 @@ void TestLanguage::exports()
         TopLevelProjectPtr project = loader->loadProject(defaultParameters);
         QVERIFY(project);
         QHash<QString, ResolvedProductPtr> products = productsFromProject(project);
-        QCOMPARE(products.count(), 8);
+        QCOMPARE(products.count(), 9);
         ResolvedProductPtr product;
         product = products.value("myapp");
         QVERIFY(product);
@@ -454,6 +454,14 @@ void TestLanguage::exports()
                                      << "modules" << "dummy" << "defines";
         propertyValue = getConfigProperty(product->moduleProperties->value(), propertyName);
         QCOMPARE(propertyValue.toStringList(), QStringList() << "ABC");
+        QCOMPARE(PropertyFinder().propertyValue(product->moduleProperties->value(), "dummy",
+                                                "productName").toString(), QString("myapp2"));
+
+        // Check whether we're returning incorrect cached values.
+        product = products.value("myapp3");
+        QVERIFY(product);
+        QCOMPARE(PropertyFinder().propertyValue(product->moduleProperties->value(), "dummy",
+                                                "productName").toString(), QString("myapp3"));
     }
     catch (const ErrorInfo &e) {
         exceptionCaught = true;
diff --git a/src/lib/corelib/language/value.h b/src/lib/corelib/language/value.h
index 7d2861685..aef428592 100644
--- a/src/lib/corelib/language/value.h
+++ b/src/lib/corelib/language/value.h
@@ -83,7 +83,8 @@ class JSSourceValue : public Value
         NoFlags = 0x00,
         SourceUsesBase = 0x01,
         SourceUsesOuter = 0x02,
-        HasFunctionForm = 0x04
+        SourceUsesProduct = 0x04,
+        HasFunctionForm = 0x08
     };
     Q_DECLARE_FLAGS(Flags, Flag)
 
@@ -108,6 +109,7 @@ public:
 
     bool sourceUsesBase() const { return m_flags.testFlag(SourceUsesBase); }
     bool sourceUsesOuter() const { return m_flags.testFlag(SourceUsesOuter); }
+    bool sourceUsesProduct() const { return m_flags.testFlag(SourceUsesProduct); }
     bool hasFunctionForm() const { return m_flags.testFlag(HasFunctionForm); }
     void setHasFunctionForm(bool b);