Benchmarker: Keep raw data in case a regression is detected

If the difference between the old and new instruction count and/or
memory usage is higher than a (user-configurable) threshold, do not
remove the temporary directory and direct the user to its location. This
way, they can investigate the regression right away, rather than having
to re-run valgrind etc.

Change-Id: I8a35fde073806fa9daf0b28315a827207f8bd04d
Reviewed-by: Jake Petroules <jake.petroules@qt.io>

5 files changed, 55 insertions, 21 deletions

diff --git a/tests/benchmarker/benchmarker-main.cpp b/tests/benchmarker/benchmarker-main.cpp
index cb84576e2..e9d483f96 100644
--- a/tests/benchmarker/benchmarker-main.cpp
+++ b/tests/benchmarker/benchmarker-main.cpp
@@ -37,19 +37,24 @@
 
 using namespace qbsBenchmarker;
 
-static QByteArray relativeChange(qint64 oldVal, qint64 newVal)
+static bool hasRegression = false;
+
+static int relativeChange(qint64 oldVal, qint64 newVal)
+{
+    return newVal == 0 ? 0 : newVal * 100 / oldVal - 100;
+}
+
+static QByteArray relativeChangeString(int change)
 {
-    QByteArray change = newVal == 0
-            ? "0" : QByteArray::number(std::abs(newVal * 100 / oldVal - 100));
-    change += " %";
-    if (oldVal > newVal)
-        change.prepend('-');
-    else if (oldVal < newVal)
-        change.prepend('+');
-    return change;
+    QByteArray changeString = QByteArray::number(change);
+    changeString += " %";
+    if (change > 0)
+        changeString.prepend('+');
+    return changeString;
 }
 
-static void printResults(Activity activity, const BenchmarkResults &results)
+static void printResults(Activity activity, const BenchmarkResults &results,
+                         int regressionThreshold)
 {
     std::cout << "========== Performance data for ";
     switch (activity) {
@@ -68,26 +73,33 @@ static void printResults(Activity activity, const BenchmarkResults &results)
     const char * const indent = "    ";
     std::cout << indent << "Old instruction count: " << result.oldInstructionCount << std::endl;
     std::cout << indent << "New instruction count: " << result.newInstructionCount << std::endl;
+    int change = relativeChange(result.oldInstructionCount, result.newInstructionCount);
+    if (change > regressionThreshold)
+        hasRegression = true;
     std::cout << indent << "Relative change: "
-              << relativeChange(result.oldInstructionCount, result.newInstructionCount).constData()
+              << relativeChangeString(change).constData()
               << std::endl;
     std::cout << indent << "Old peak memory usage: " << result.oldPeakMemoryUsage << " Bytes"
               << std::endl;
     std::cout << indent
               << "New peak memory usage: " << result.newPeakMemoryUsage << " Bytes" << std::endl;
+    change = relativeChange(result.oldPeakMemoryUsage, result.newPeakMemoryUsage);
+    if (change > regressionThreshold)
+        hasRegression = true;
     std::cout << indent << "Relative change: "
-              << relativeChange(result.oldPeakMemoryUsage, result.newPeakMemoryUsage).constData()
+              << relativeChangeString(change).constData()
               << std::endl;
 }
 
-static void printResults(Activities activities, const BenchmarkResults &results)
+static void printResults(Activities activities, const BenchmarkResults &results,
+                         int regressionThreshold)
 {
     if (activities & ActivityResolving)
-        printResults(ActivityResolving, results);
+        printResults(ActivityResolving, results, regressionThreshold);
     if (activities & ActivityRuleExecution)
-        printResults(ActivityRuleExecution, results);
+        printResults(ActivityRuleExecution, results, regressionThreshold);
     if (activities & ActivityNullBuild)
-        printResults(ActivityNullBuild, results);
+        printResults(ActivityNullBuild, results, regressionThreshold);
 }
 
 int main(int argc, char *argv[])
@@ -99,7 +111,12 @@ int main(int argc, char *argv[])
         Benchmarker benchmarker(clParser.activies(), clParser.oldCommit(), clParser.newCommit(),
                                 clParser.testProjectFilePath(), clParser.qbsRepoDirPath());
         benchmarker.benchmark();
-        printResults(clParser.activies(), benchmarker.results());
+        printResults(clParser.activies(), benchmarker.results(), clParser.regressionThreshold());
+        if (hasRegression) {
+            benchmarker.keepRawData();
+            std::cout << "Performance regression detected. Raw benchmarking data available "
+                         "under " << qPrintable(benchmarker.rawDataBaseDir()) << '.' << std::endl;
+        }
     } catch (const Exception &e) {
         std::cerr << qPrintable(e.description()) << std::endl;
         return EXIT_FAILURE;
diff --git a/tests/benchmarker/benchmarker.cpp b/tests/benchmarker/benchmarker.cpp
index 26d863aaa..e4fb7b509 100644
--- a/tests/benchmarker/benchmarker.cpp
+++ b/tests/benchmarker/benchmarker.cpp
@@ -63,19 +63,19 @@ void Benchmarker::benchmark()
 {
     rememberCurrentRepoState();
     runProcess(QStringList() << "git" << "checkout" << m_oldCommit, m_qbsRepo);
-    const QString oldQbsBuildDir = m_baseOutputDir.path() + "/old-qbs-build";
+    const QString oldQbsBuildDir = m_baseOutputDir.path() + "/qbs-build." + m_oldCommit;
     std::cout << "Building from old repo state..." << std::endl;
     buildQbs(oldQbsBuildDir);
     runProcess(QStringList() << "git" << "checkout" << m_newCommit, m_qbsRepo);
-    const QString newQbsBuildDir = m_baseOutputDir.path() + "/new-qbs-build";
+    const QString newQbsBuildDir = m_baseOutputDir.path() + "/qbs-build." + m_newCommit;
     std::cout << "Building from new repo state..." << std::endl;
     buildQbs(newQbsBuildDir);
     std::cout << "Now running valgrind. This can take a while." << std::endl;
 
     ValgrindRunner oldDataRetriever(m_activities, m_testProject, oldQbsBuildDir,
-                                    m_baseOutputDir.path() + "/old-stuff");
+                                    m_baseOutputDir.path() + "/benchmark-data." + m_oldCommit);
     ValgrindRunner newDataRetriever(m_activities, m_testProject, newQbsBuildDir,
-                                    m_baseOutputDir.path() + "/new-stuff");
+                                    m_baseOutputDir.path() + "/benchmark-data." + m_newCommit);
     QFuture<void> oldFuture = QtConcurrent::run(&oldDataRetriever, &ValgrindRunner::run);
     QFuture<void> newFuture = QtConcurrent::run(&newDataRetriever, &ValgrindRunner::run);
     oldFuture.waitForFinished();
diff --git a/tests/benchmarker/benchmarker.h b/tests/benchmarker/benchmarker.h
index b6995de41..7027c9d15 100644
--- a/tests/benchmarker/benchmarker.h
+++ b/tests/benchmarker/benchmarker.h
@@ -58,8 +58,10 @@ public:
     ~Benchmarker();
 
     void benchmark();
+    void keepRawData() { m_baseOutputDir.setAutoRemove(false ); }
 
     BenchmarkResults results() const { return m_results; }
+    QString rawDataBaseDir() const { return m_baseOutputDir.path(); }
 
 private:
     void rememberCurrentRepoState();
diff --git a/tests/benchmarker/commandlineparser.cpp b/tests/benchmarker/commandlineparser.cpp
index 84650cfdc..f33fdcaf4 100644
--- a/tests/benchmarker/commandlineparser.cpp
+++ b/tests/benchmarker/commandlineparser.cpp
@@ -65,6 +65,11 @@ void CommandLineParser::parse()
                     .arg(resolveActivity(), ruleExecutionActivity(), nullBuildActivity(),
                          allActivities()), "activities", allActivities());
     parser.addOption(activitiesOption);
+    QCommandLineOption thresholdOption("regression-threshold",
+            "A relative increase higher than this is considered a performance regression. "
+            "All temporary data from running the benchmarks will be kept if that happens.",
+            "value in per cent");
+    parser.addOption(thresholdOption);
     parser.process(*QCoreApplication::instance());
     QList<QCommandLineOption> mandatoryOptions = QList<QCommandLineOption>()
             << oldCommitOption << newCommitOption << testProjectOption << qbsRepoOption;
@@ -94,6 +99,14 @@ void CommandLineParser::parse()
             throwException(activitiesOption.names().first(), activityString, parser.helpText());
         }
     }
+    m_regressionThreshold = 5;
+    if (parser.isSet(thresholdOption)) {
+        bool ok = true;
+        const QString rawThresholdValue = parser.value(thresholdOption);
+        m_regressionThreshold = rawThresholdValue.toInt(&ok);
+        if (!ok)
+            throwException(thresholdOption.names().first(), rawThresholdValue, parser.helpText());
+    }
 }
 
 void CommandLineParser::throwException(const QString &optionName, const QString &illegalValue,
diff --git a/tests/benchmarker/commandlineparser.h b/tests/benchmarker/commandlineparser.h
index bdb3b0eda..2c87daeef 100644
--- a/tests/benchmarker/commandlineparser.h
+++ b/tests/benchmarker/commandlineparser.h
@@ -46,6 +46,7 @@ public:
     QString newCommit() const { return m_newCommit; }
     QString testProjectFilePath() const { return m_testProjectFilePath; }
     QString qbsRepoDirPath() const { return m_qbsRepoDirPath; }
+    int regressionThreshold() const { return m_regressionThreshold; }
 
 private:
     [[noreturn]] void throwException(const QString &optionName, const QString &illegalValue,
@@ -57,6 +58,7 @@ private:
     QString m_newCommit;
     QString m_testProjectFilePath;
     QString m_qbsRepoDirPath;
+    int m_regressionThreshold;
 };
 
 } // namespace qbsBenchmarker