Some engine-related optimizations.

- Re-enable script program cache; it is safe to use now.
- Create a new evaluation context only for the first build of a project.
- To help with all of this, use a shared pointer for
  RulesEvaluationContext, as it does not have a clear owner.

Change-Id: I90a7fcaf855573b756f29daff37038e98aa7c1fe
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

13 files changed, 83 insertions, 76 deletions

diff --git a/src/lib/api/internaljobs.cpp b/src/lib/api/internaljobs.cpp
index fcd830585..d877491dd 100644
--- a/src/lib/api/internaljobs.cpp
+++ b/src/lib/api/internaljobs.cpp
@@ -144,17 +144,16 @@ void InternalSetupProjectJob::doResolve()
 
 void InternalSetupProjectJob::execute()
 {
-    QScopedPointer<RulesEvaluationContext> evalContext(new RulesEvaluationContext);
+    RulesEvaluationContextPtr evalContext(new RulesEvaluationContext);
     evalContext->setObserver(observer());
     const QStringList searchPaths = Settings().searchPaths();
     const BuildProjectLoader::LoadResult loadResult = BuildProjectLoader().load(m_projectFilePath,
-            evalContext.data(), m_buildRoot, m_buildConfig, searchPaths);
+            evalContext, m_buildRoot, m_buildConfig, searchPaths);
 
     ResolvedProjectPtr rProject;
     if (!loadResult.discardLoadedProject)
         m_buildProject = loadResult.loadedProject;
     if (m_buildProject) {
-        evalContext.take();
         rProject = m_buildProject->resolvedProject();
     } else {
         if (loadResult.changedResolvedProject) {
@@ -183,14 +182,15 @@ void InternalSetupProjectJob::execute()
     }
     qbsDebug("");
 
-    if (m_buildProject)
-        return;
+    if (!m_buildProject) {
+        TimedActivityLogger resolveLogger(QLatin1String("Resolving build project"));
+        m_buildProject = BuildProjectResolver().resolveProject(rProject, evalContext);
+        if (loadResult.loadedProject)
+            m_buildProject->rescueDependencies(loadResult.loadedProject);
+    }
 
-    TimedActivityLogger resolveLogger(QLatin1String("Resolving build project"));
-    m_buildProject = BuildProjectResolver().resolveProject(rProject, evalContext.data());
-    if (loadResult.loadedProject)
-        m_buildProject->rescueDependencies(loadResult.loadedProject);
-    evalContext.take();
+    // The evalutation context cannot be re-used for building, which runs in a different thread.
+    m_buildProject->setEvaluationContext(RulesEvaluationContextPtr());
 }
 
 
diff --git a/src/lib/buildgraph/buildproject.cpp b/src/lib/buildgraph/buildproject.cpp
index 311149446..ce7f999a5 100644
--- a/src/lib/buildgraph/buildproject.cpp
+++ b/src/lib/buildgraph/buildproject.cpp
@@ -52,7 +52,6 @@ BuildProject::BuildProject() : m_evalContext(0), m_dirty(false)
 
 BuildProject::~BuildProject()
 {
-    delete m_evalContext;
     qDeleteAll(m_dependencyArtifacts);
 }
 
@@ -83,9 +82,8 @@ QString BuildProject::buildGraphFilePath() const
     return deriveBuildGraphFilePath(resolvedProject()->buildDirectory, resolvedProject()->id());
 }
 
-void BuildProject::setEvaluationContext(RulesEvaluationContext *evalContext)
+void BuildProject::setEvaluationContext(const RulesEvaluationContextPtr &evalContext)
 {
-    delete m_evalContext;
     m_evalContext = evalContext;
 }
 
@@ -216,7 +214,7 @@ void BuildProject::removeArtifact(Artifact *artifact)
 
 void BuildProject::updateNodesThatMustGetNewTransformer()
 {
-    RulesEvaluationContext::Scope s(evaluationContext());
+    RulesEvaluationContext::Scope s(evaluationContext().data());
     foreach (Artifact *artifact, m_artifactsThatMustGetNewTransformers)
         updateNodeThatMustGetNewTransformer(artifact);
     m_artifactsThatMustGetNewTransformers.clear();
@@ -281,7 +279,7 @@ void BuildProject::store(PersistentPool &pool) const
 
 
 BuildProjectPtr BuildProjectResolver::resolveProject(const ResolvedProjectPtr &resolvedProject,
-                                                     RulesEvaluationContext *evalContext)
+                                                     const RulesEvaluationContextPtr &evalContext)
 {
     m_productCache.clear();
     m_project = BuildProjectPtr(new BuildProject);
@@ -376,11 +374,11 @@ BuildProductPtr BuildProjectResolver::resolveProduct(const ResolvedProductPtr &r
         }
         transformer->rule = rule;
 
-        RulesEvaluationContext::Scope s(evalContext());
+        RulesEvaluationContext::Scope s(evalContext().data());
         BuildGraph::setupScriptEngineForProduct(engine(), rProduct, transformer->rule, scope());
         transformer->setupInputs(engine(), scope());
         transformer->setupOutputs(engine(), scope());
-        transformer->createCommands(rtrafo->transform, engine());
+        transformer->createCommands(rtrafo->transform, evalContext());
         if (transformer->commands.isEmpty())
             throw Error(QString("There's a transformer without commands."), rtrafo->transform->location);
     }
@@ -403,7 +401,7 @@ BuildProductPtr BuildProjectResolver::resolveProduct(const ResolvedProductPtr &r
     return product;
 }
 
-RulesEvaluationContext *BuildProjectResolver::evalContext() const
+RulesEvaluationContextPtr BuildProjectResolver::evalContext() const
 {
     return m_project->evaluationContext();
 }
@@ -437,7 +435,7 @@ static bool isConfigCompatible(const QVariantMap &userCfg, const QVariantMap &pr
 }
 
 BuildProjectLoader::LoadResult BuildProjectLoader::load(const QString &projectFilePath,
-        RulesEvaluationContext *evalContext, const QString &buildRoot, const QVariantMap &cfg,
+        const RulesEvaluationContextPtr &evalContext, const QString &buildRoot, const QVariantMap &cfg,
         const QStringList &loaderSearchPaths)
 {
     m_result = LoadResult();
diff --git a/src/lib/buildgraph/buildproject.h b/src/lib/buildgraph/buildproject.h
index 6e40936ee..bf7c913ed 100644
--- a/src/lib/buildgraph/buildproject.h
+++ b/src/lib/buildgraph/buildproject.h
@@ -46,7 +46,6 @@ namespace qbs {
 namespace Internal {
 class BuildProjectLoader;
 class BuildProjectResolver;
-class RulesEvaluationContext;
 class ScriptEngine;
 
 class BuildProject : public PersistentObject
@@ -61,8 +60,8 @@ public:
     static QString deriveBuildGraphFilePath(const QString &buildDir, const QString &projectId);
     QString buildGraphFilePath() const;
 
-    void setEvaluationContext(RulesEvaluationContext *evalContext);
-    RulesEvaluationContext *evaluationContext() const { return m_evalContext; }
+    void setEvaluationContext(const RulesEvaluationContextPtr &evalContext);
+    RulesEvaluationContextPtr evaluationContext() const { return m_evalContext; }
 
     ResolvedProjectPtr resolvedProject() const;
     QSet<BuildProductPtr> buildProducts() const;
@@ -91,7 +90,7 @@ private:
     void updateNodeThatMustGetNewTransformer(Artifact *artifact);
 
 private:
-    RulesEvaluationContext *m_evalContext;
+    RulesEvaluationContextPtr m_evalContext;
     ResolvedProjectPtr m_resolvedProject;
     QSet<BuildProductPtr> m_buildProducts;
     ArtifactList m_dependencyArtifacts;
@@ -105,12 +104,12 @@ class BuildProjectResolver
 {
 public:
     BuildProjectPtr resolveProject(const ResolvedProjectPtr &resolvedProject,
-                                   RulesEvaluationContext *evalContext);
+                                   const RulesEvaluationContextPtr &evalContext);
 
 private:
     BuildProductPtr resolveProduct(const ResolvedProductPtr &rProduct);
 
-    RulesEvaluationContext *evalContext() const;
+    RulesEvaluationContextPtr evalContext() const;
     ScriptEngine *engine() const;
     QScriptValue scope() const;
 
@@ -131,7 +130,7 @@ public:
         bool discardLoadedProject;
     };
 
-    LoadResult load(const QString &projectFilePath, RulesEvaluationContext *evalContext,
+    LoadResult load(const QString &projectFilePath, const RulesEvaluationContextPtr &evalContext,
                     const QString &buildRoot, const QVariantMap &cfg,
                     const QStringList &loaderSearchPaths);
 
@@ -142,7 +141,7 @@ private:
     void removeArtifactAndExclusiveDependents(Artifact *artifact,
                                               QList<Artifact*> *removedArtifacts = 0);
 
-    RulesEvaluationContext *m_evalContext;
+    RulesEvaluationContextPtr m_evalContext;
     LoadResult m_result;
 };
 
diff --git a/src/lib/buildgraph/executor.cpp b/src/lib/buildgraph/executor.cpp
index fa4d6a8c5..ec5f338da 100644
--- a/src/lib/buildgraph/executor.cpp
+++ b/src/lib/buildgraph/executor.cpp
@@ -91,15 +91,12 @@ private:
 
 Executor::Executor(QObject *parent)
     : QObject(parent)
-    , m_evalContext(new RulesEvaluationContext)
     , m_progressObserver(0)
     , m_state(ExecutorIdle)
 {
     m_inputArtifactScanContext = new InputArtifactScannerContext(&m_scanResultCache);
     m_autoMoc = new AutoMoc;
     m_autoMoc->setScanResultCache(&m_scanResultCache);
-    foreach (ExecutorJob *job, findChildren<ExecutorJob *>())
-        job->setMainThreadScriptEngine(m_evalContext->engine());
 }
 
 Executor::~Executor()
@@ -145,22 +142,35 @@ void Executor::doBuild(const QList<BuildProductPtr> &productsToBuild)
     m_error.clear();
     m_explicitlyCanceled = false;
 
-    QSet<BuildProject *> projects;
-    foreach (const BuildProductConstPtr &buildProduct, productsToBuild)
-        projects << buildProduct->project;
-    foreach (BuildProject * const project, projects)
+    setState(ExecutorRunning);
+
+    if (productsToBuild.isEmpty()) {
+        qbsTrace() << "No products to build, finishing.";
+        QTimer::singleShot(0, this, SLOT(finish())); // Don't call back on the caller.
+        return;
+    }
+
+    doSanityChecks();
+    BuildProject * const project = productsToBuild.first()->project;
+    m_evalContext = project->evaluationContext();
+    if (!m_evalContext) { // Is null before the first build.
+        m_evalContext = RulesEvaluationContextPtr(new RulesEvaluationContext);
         project->setEvaluationContext(m_evalContext);
+    }
+
+    qbsDebug("[EXEC] preparing executor for %d jobs in parallel", m_buildOptions.maxJobCount);
+    addExecutorJobs(m_buildOptions.maxJobCount);
+    foreach (ExecutorJob * const job, m_availableJobs)
+        job->setDryRun(m_buildOptions.dryRun);
 
     initializeArtifactsState();
-    setState(ExecutorRunning);
     Artifact::BuildState initialBuildState = m_buildOptions.changedFiles.isEmpty()
             ? Artifact::Buildable : Artifact::Built;
 
     QList<Artifact *> changedArtifacts;
     foreach (const QString &filePath, m_buildOptions.changedFiles) {
         QList<Artifact *> artifacts;
-        foreach (const BuildProject * const project, projects)
-            artifacts.append(project->lookupArtifacts(filePath));
+        artifacts.append(project->lookupArtifacts(filePath));
         if (artifacts.isEmpty()) {
             qbsWarning() << QString("Out of date file '%1' provided but not found.").arg(QDir::toNativeSeparators(filePath));
             continue;
@@ -202,17 +212,6 @@ void Executor::doBuild(const QList<BuildProductPtr> &productsToBuild)
 void Executor::setBuildOptions(const BuildOptions &buildOptions)
 {
     m_buildOptions = buildOptions;
-
-    qbsDebug("[EXEC] preparing executor for %d jobs in parallel", m_buildOptions.maxJobCount);
-    const int actualJobNumber = m_availableJobs.count() + m_processingJobs.count();
-    if (actualJobNumber > m_buildOptions.maxJobCount) {
-        removeExecutorJobs(actualJobNumber - m_buildOptions.maxJobCount);
-    } else {
-        addExecutorJobs(m_buildOptions.maxJobCount - actualJobNumber);
-    }
-
-    foreach (ExecutorJob * const job, m_availableJobs)
-        job->setDryRun(m_buildOptions.dryRun);
 }
 
 static void initArtifactsBottomUp(Artifact *artifact)
@@ -566,6 +565,14 @@ void Executor::setupProgressObserver(bool mocWillRun)
     m_progressObserver->initialize(tr("Building"), totalEffort);
 }
 
+void Executor::doSanityChecks()
+{
+    Q_ASSERT(!m_productsToBuild.isEmpty());
+    BuildProject * const p = m_productsToBuild.first()->project;
+    for (int i = 1; i < m_productsToBuild.count(); ++i)
+        Q_ASSERT(m_productsToBuild.at(i)->project == p);
+}
+
 void Executor::addExecutorJobs(int jobNumber)
 {
     for (int i = 1; i <= jobNumber; i++) {
@@ -578,18 +585,6 @@ void Executor::addExecutorJobs(int jobNumber)
     }
 }
 
-void Executor::removeExecutorJobs(int jobNumber)
-{
-    if (jobNumber >= m_availableJobs.count()) {
-        qDeleteAll(m_availableJobs);
-        m_availableJobs.clear();
-    } else {
-        for (int i = 1; i <= jobNumber; i++) {
-            delete m_availableJobs.takeLast();
-        }
-    }
-}
-
 void Executor::runAutoMoc()
 {
     bool autoMocApplied = false;
diff --git a/src/lib/buildgraph/executor.h b/src/lib/buildgraph/executor.h
index 7ddb66b9e..494a397c8 100644
--- a/src/lib/buildgraph/executor.h
+++ b/src/lib/buildgraph/executor.h
@@ -46,7 +46,6 @@ class AutoMoc;
 class ExecutorJob;
 class InputArtifactScannerContext;
 class ProgressObserver;
-class RulesEvaluationContext;
 class ScanResultCache;
 
 class Executor : public QObject
@@ -89,13 +88,13 @@ private:
     void initializeArtifactsState();
     void setState(ExecutorState);
     void addExecutorJobs(int jobNumber);
-    void removeExecutorJobs(int jobNumber);
     void runAutoMoc();
     void insertLeavesAfterAddingDependencies(QVector<Artifact *> dependencies);
     void cancelJobs();
     void setupProgressObserver(bool mocWillRun);
+    void doSanityChecks();
 
-    RulesEvaluationContext *m_evalContext;
+    RulesEvaluationContextPtr m_evalContext;
     BuildOptions m_buildOptions;
     ProgressObserver *m_progressObserver;
     QList<ExecutorJob*> m_availableJobs;
diff --git a/src/lib/buildgraph/forward_decls.h b/src/lib/buildgraph/forward_decls.h
index 9daa72b2e..e4c04fac7 100644
--- a/src/lib/buildgraph/forward_decls.h
+++ b/src/lib/buildgraph/forward_decls.h
@@ -48,6 +48,9 @@ class Transformer;
 typedef QSharedPointer<Transformer> TransformerPtr;
 typedef QSharedPointer<const Transformer> TransformerConstPtr;
 
+class RulesEvaluationContext;
+typedef QSharedPointer<RulesEvaluationContext> RulesEvaluationContextPtr;
+
 } // namespace Internal
 } // namespace qbs
 
diff --git a/src/lib/buildgraph/rulesapplicator.cpp b/src/lib/buildgraph/rulesapplicator.cpp
index bd19b1242..7f62f1297 100644
--- a/src/lib/buildgraph/rulesapplicator.cpp
+++ b/src/lib/buildgraph/rulesapplicator.cpp
@@ -54,7 +54,7 @@ RulesApplicator::RulesApplicator(BuildProduct *product, ArtifactsPerFileTagMap &
 
 void RulesApplicator::applyAllRules()
 {
-    RulesEvaluationContext::Scope s(m_buildProduct->project->evaluationContext());
+    RulesEvaluationContext::Scope s(m_buildProduct->project->evaluationContext().data());
     foreach (const RuleConstPtr &rule, m_buildProduct->topSortedRules())
         applyRule(rule);
 }
@@ -173,7 +173,7 @@ void RulesApplicator::doApply(const ArtifactList &inputArtifacts)
     }
 
     m_transformer->setupOutputs(engine(), scope());
-    m_transformer->createCommands(m_rule->script, engine());
+    m_transformer->createCommands(m_rule->script, evalContext());
     if (m_transformer->commands.isEmpty())
         throw Error(QString("There's a rule without commands: %1.").arg(m_rule->toString()), m_rule->script->location);
 }
@@ -291,7 +291,7 @@ QString RulesApplicator::resolveOutPath(const QString &path) const
     return result;
 }
 
-RulesEvaluationContext *RulesApplicator::evalContext() const
+RulesEvaluationContextPtr RulesApplicator::evalContext() const
 {
     return m_buildProduct->project->evaluationContext();
 }
diff --git a/src/lib/buildgraph/rulesapplicator.h b/src/lib/buildgraph/rulesapplicator.h
index 405353c10..e0090e6b6 100644
--- a/src/lib/buildgraph/rulesapplicator.h
+++ b/src/lib/buildgraph/rulesapplicator.h
@@ -39,7 +39,6 @@
 
 namespace qbs {
 namespace Internal {
-class RulesEvaluationContext;
 class ScriptEngine;
 
 typedef QMap<QString, ArtifactList> ArtifactsPerFileTagMap;
@@ -57,7 +56,7 @@ private:
     Artifact *createOutputArtifact(const RuleArtifactConstPtr &ruleArtifact,
                                    const ArtifactList &inputArtifacts);
     QString resolveOutPath(const QString &path) const;
-    RulesEvaluationContext *evalContext() const;
+    RulesEvaluationContextPtr evalContext() const;
     ScriptEngine *engine() const;
     QScriptValue scope() const;
 
diff --git a/src/lib/buildgraph/rulesevaluationcontext.cpp b/src/lib/buildgraph/rulesevaluationcontext.cpp
index d0b41bdef..89f6767aa 100644
--- a/src/lib/buildgraph/rulesevaluationcontext.cpp
+++ b/src/lib/buildgraph/rulesevaluationcontext.cpp
@@ -56,6 +56,14 @@ RulesEvaluationContext::~RulesEvaluationContext()
     delete m_engine;
 }
 
+QScriptProgram RulesEvaluationContext::scriptProgram(const QString &script)
+{
+    QScriptProgram p = m_scriptHash[script];
+    if (p.isNull())
+        p = QScriptProgram(script);
+    return p;
+}
+
 void RulesEvaluationContext::initializeObserver(const QString &description, int maximumProgress)
 {
     if (m_observer)
diff --git a/src/lib/buildgraph/rulesevaluationcontext.h b/src/lib/buildgraph/rulesevaluationcontext.h
index 2395cc34f..cc253f5ed 100644
--- a/src/lib/buildgraph/rulesevaluationcontext.h
+++ b/src/lib/buildgraph/rulesevaluationcontext.h
@@ -31,7 +31,10 @@
 
 #include <language/forward_decls.h>
 
+#include <QHash>
+#include <QScriptProgram>
 #include <QScriptValue>
+#include <QString>
 
 namespace qbs {
 namespace Internal {
@@ -56,6 +59,7 @@ public:
 
     ScriptEngine *engine() const { return m_engine; }
     QScriptValue scope() const { return m_scope; }
+    QScriptProgram scriptProgram(const QString &script);
 
     void setObserver(ProgressObserver *observer) { m_observer = observer; }
     void initializeObserver(const QString &description, int maximumProgress);
@@ -73,6 +77,7 @@ private:
     unsigned int m_initScopeCalls;
     QScriptValue m_scope;
     QScriptValue m_prepareScriptScope;
+    QHash<QString, QScriptProgram> m_scriptHash;
 };
 
 } // namespace Internal
diff --git a/src/lib/buildgraph/transformer.cpp b/src/lib/buildgraph/transformer.cpp
index eff5c1f60..0c9fa6a8a 100644
--- a/src/lib/buildgraph/transformer.cpp
+++ b/src/lib/buildgraph/transformer.cpp
@@ -26,10 +26,11 @@
 ** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
 **
 ****************************************************************************/
-
 #include "transformer.h"
+
 #include "artifact.h"
 #include "command.h"
+#include "rulesevaluationcontext.h"
 #include <language/language.h>
 #include <language/scriptengine.h>
 #include <tools/error.h>
@@ -127,12 +128,12 @@ static AbstractCommand *createCommandFromScriptValue(const QScriptValue &scriptV
     return cmdBase;
 }
 
-void Transformer::createCommands(const PrepareScriptConstPtr &script, ScriptEngine *engine)
+void Transformer::createCommands(const PrepareScriptConstPtr &script,
+                                 const RulesEvaluationContextPtr &evalContext)
 {
-//    if (script->cachedScript.isNull())
-        script->cachedScript = QScriptProgram(script->script);
-
-    QScriptValue scriptValue = engine->evaluate(script->cachedScript);
+    QScriptProgram scriptProgram = evalContext->scriptProgram(script->script);
+    ScriptEngine * const engine = evalContext->engine();
+    QScriptValue scriptValue = engine->evaluate(scriptProgram);
     if (engine->hasUncaughtException())
         throw Error("evaluating prepare script: " + engine->uncaughtException().toString(),
                     CodeLocation(script->location.fileName,
diff --git a/src/lib/buildgraph/transformer.h b/src/lib/buildgraph/transformer.h
index 374b0d577..4467e3956 100644
--- a/src/lib/buildgraph/transformer.h
+++ b/src/lib/buildgraph/transformer.h
@@ -65,7 +65,8 @@ public:
 
     void setupInputs(QScriptEngine *scriptEngine, QScriptValue targetScriptValue);
     void setupOutputs(QScriptEngine *scriptEngine, QScriptValue targetScriptValue);
-    void createCommands(const PrepareScriptConstPtr &script, ScriptEngine *engine);
+    void createCommands(const PrepareScriptConstPtr &script,
+                        const RulesEvaluationContextPtr &evalContext);
 
 private:
     Transformer();
diff --git a/src/lib/language/language.h b/src/lib/language/language.h
index c4f6abd50..1654fbeda 100644
--- a/src/lib/language/language.h
+++ b/src/lib/language/language.h
@@ -214,7 +214,6 @@ public:
 
     QString script;
     CodeLocation location;
-    mutable QScriptProgram cachedScript;
 
 private:
     PrepareScript() {}