Perf dashboard should automatically add aggregators
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Jan 2016 23:47:14 +0000 (23:47 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Jan 2016 23:47:14 +0000 (23:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152818

Reviewed by Chris Dumez.

When an aggregator entry is missing in aggregators table, automatically insert it in /api/report.

In a very early version of the perf dashboard, we had the ability to define a custom aggregator
in an admin page. In practice, nobody used or needed this feature so we got rid of it even before
the dashboard was landed into WebKit repository. This patch cleans up that mess.

* run-tests.js:
(main): Added the filtering capability.
(TestEnvironment): Expose the config JSON in the test environment.

* public/include/report-processor.php:
(ReportProcessor): Renamed name_to_aggregator now that it only contains ID.
(ReportProcessor::__construct): No longer fetches the aggregator table. An equivalent work is done
in newly added ensure_aggregators.
(ReportProcessor::process): Calls ensure_aggregators which populates name_to_aggregator_id.
(ReportProcessor::ensure_aggregators): Added. Add the builtin aggregators: Arithmetic, Geometric,
Harmonic, and Total.
(TestRunsGenerator): Renamed name_to_aggregator now that it only contains ID.
(TestRunsGenerator::__construct):
(TestRunsGenerator::add_aggregated_metric): Don't include aggregator_definition here since it's
never used now that all the aggregations are done natively in PHP.
(TestRunsGenerator::$aggregators): Added. We don't include SquareSum since it's only used for
computing run_square_sum_cache in test_runs table and it's useless elsewhere.
(TestRunsGenerator::aggregate_values): Add a comment about that.

* tests/api-report.js: Updated a test case to reflect the change.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@194738 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/include/report-processor.php
Websites/perf.webkit.org/run-tests.js
Websites/perf.webkit.org/tests/api-report.js

index 49bd549..d24f1bb 100644 (file)
@@ -1,3 +1,37 @@
+2016-01-07  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Perf dashboard should automatically add aggregators
+        https://bugs.webkit.org/show_bug.cgi?id=152818
+
+        Reviewed by Chris Dumez.
+
+        When an aggregator entry is missing in aggregators table, automatically insert it in /api/report.
+
+        In a very early version of the perf dashboard, we had the ability to define a custom aggregator
+        in an admin page. In practice, nobody used or needed this feature so we got rid of it even before
+        the dashboard was landed into WebKit repository. This patch cleans up that mess.
+
+        * run-tests.js:
+        (main): Added the filtering capability.
+        (TestEnvironment): Expose the config JSON in the test environment.
+
+        * public/include/report-processor.php:
+        (ReportProcessor): Renamed name_to_aggregator now that it only contains ID.
+        (ReportProcessor::__construct): No longer fetches the aggregator table. An equivalent work is done
+        in newly added ensure_aggregators.
+        (ReportProcessor::process): Calls ensure_aggregators which populates name_to_aggregator_id.
+        (ReportProcessor::ensure_aggregators): Added. Add the builtin aggregators: Arithmetic, Geometric,
+        Harmonic, and Total.
+        (TestRunsGenerator): Renamed name_to_aggregator now that it only contains ID.
+        (TestRunsGenerator::__construct):
+        (TestRunsGenerator::add_aggregated_metric): Don't include aggregator_definition here since it's
+        never used now that all the aggregations are done natively in PHP.
+        (TestRunsGenerator::$aggregators): Added. We don't include SquareSum since it's only used for
+        computing run_square_sum_cache in test_runs table and it's useless elsewhere.
+        (TestRunsGenerator::aggregate_values): Add a comment about that.
+
+        * tests/api-report.js: Updated a test case to reflect the change.
+
 2016-01-06  Ryosuke Niwa  <rniwa@webkit.org>
 
         Perf dashboard JSON API should fail gracefully when postgres is down
index 556d98b..bd5c91d 100644 (file)
@@ -4,19 +4,13 @@ require_once('../include/json-header.php');
 
 class ReportProcessor {
     private $db;
-    private $name_to_aggregator;
+    private $name_to_aggregator_id;
     private $report_id;
     private $runs;
 
     function __construct($db) {
         $this->db = $db;
-        $this->name_to_aggregator = array();
-        $aggregator_table = $db->fetch_table('aggregators');
-        if ($aggregator_table) {
-            foreach ($aggregator_table as $aggregator_row) {
-                $this->name_to_aggregator[$aggregator_row['aggregator_name']] = $aggregator_row;
-            }
-        }
+        $this->name_to_aggregator_id = array();
     }
 
     private function exit_with_error($message, $details = NULL) {
@@ -78,7 +72,9 @@ class ReportProcessor {
         if (!$existing_report_id)
             $this->store_report($report, $build_data);
 
-        $this->runs = new TestRunsGenerator($this->db, $this->name_to_aggregator, $this->report_id);
+        $this->ensure_aggregators();
+
+        $this->runs = new TestRunsGenerator($this->db, $this->name_to_aggregator_id, $this->report_id);
         $this->recursively_ensure_tests($report['tests']);
 
         $this->runs->aggregate();
@@ -113,6 +109,15 @@ class ReportProcessor {
             $this->exit_with_error('FailedToStoreRunReport');
     }
 
+    private function ensure_aggregators() {
+        foreach (TestRunsGenerator::$aggregators as $name) {
+            $id = $this->db->select_or_insert_row('aggregators', 'aggregator', array('name' => $name));
+            if (!$id)
+                $this->exit_with_error('FailedToInsertAggregator', array('aggregator' => $name));
+            $this->name_to_aggregator_id[$name] = $id;
+        }
+    }
+
     private function resolve_build_id($build_data, $revisions, $build_request_id) {
         // FIXME: This code has a race condition. See <rdar://problem/15876303>.
         $results = $this->db->query_and_fetch_all("SELECT build_id, build_slave FROM builds
@@ -209,15 +214,15 @@ class ReportProcessor {
 
 class TestRunsGenerator {
     private $db;
-    private $name_to_aggregator;
+    private $name_to_aggregator_id;
     private $report_id;
     private $metrics_to_aggregate;
     private $parent_to_values;
     private $values_to_commit;
 
-    function __construct($db, $name_to_aggregator, $report_id) {
+    function __construct($db, $name_to_aggregator_id, $report_id) {
         $this->db = $db;
-        $this->name_to_aggregator = $name_to_aggregator or array();
+        $this->name_to_aggregator_id = $name_to_aggregator_id;
         $this->report_id = $report_id;
         $this->metrics_to_aggregate = array();
         $this->parent_to_values = array();
@@ -232,10 +237,11 @@ class TestRunsGenerator {
     }
 
     function add_aggregated_metric($parent_id, $test_id, $test_name, $metric_name, $aggregator_name, $level) {
-        array_key_exists($aggregator_name, $this->name_to_aggregator) or $this->exit_with_error('AggregatorNotFound', array('name' => $aggregator_name));
+        array_key_exists($aggregator_name, $this->name_to_aggregator_id)
+            or $this->exit_with_error('AggregatorNotFound', array('name' => $aggregator_name));
 
         $metric_id = $this->db->select_or_insert_row('test_metrics', 'metric', array('name' => $metric_name,
-            'test' => $test_id, 'aggregator' => $this->name_to_aggregator[$aggregator_name]['aggregator_id']));
+            'test' => $test_id, 'aggregator' => $this->name_to_aggregator_id[$aggregator_name]));
         if (!$metric_id)
             $this->exit_with_error('FailedToAddAggregatedMetric', array('name' => $metric_name, 'test' => $test_id, 'aggregator' => $aggregator_name));
 
@@ -246,7 +252,6 @@ class TestRunsGenerator {
             'test_name' => $test_name,
             'metric_name' => $metric_name,
             'aggregator' => $aggregator_name,
-            'aggregator_definition' => $this->name_to_aggregator[$aggregator_name]['aggregator_definition'],
             'level' => $level));
     }
 
@@ -355,6 +360,8 @@ class TestRunsGenerator {
         return array('values' => $values_by_iterations, 'group_sizes' => $group_sizes);
     }
 
+    static public $aggregators = array('Arithmetic', 'Geometric', 'Harmonic', 'Total');
+
     private function aggregate_values($aggregator, $values) {
         switch ($aggregator) {
         case 'Arithmetic':
@@ -365,7 +372,7 @@ class TestRunsGenerator {
             return count($values) / array_sum(array_map(function ($x) { return 1 / $x; }, $values));
         case 'Total':
             return array_sum($values);
-        case 'SquareSum':
+        case 'SquareSum': # This aggregator is only used internally to compute run_square_sum_cache in test_runs table.
             return array_sum(array_map(function ($x) { return $x * $x; }, $values));
         default:
             $this->exit_with_error('UnknownAggregator', array('aggregator' => $aggregator));
index a730f59..5dbcc5a 100644 (file)
@@ -88,8 +88,9 @@ function SerializedTaskQueue() {
     }
 }
 
-function main() {
+function main(argv) {
     var client = connect(true);
+    var filter = argv[2];
     confirmUserWantsDatabaseToBeInitializedIfNeeded(client, function (error, shouldContinue) {
         if (error)
             console.error(error);
@@ -110,7 +111,7 @@ function main() {
             var testCaseQueue = new SerializedTaskQueue();
             var testFileQueue = new SerializedTaskQueue();
             fs.readdirSync(pathToTests()).map(function (testFile) {
-                if (!testFile.match(/.js$/))
+                if (!testFile.match(/.js$/) || (filter && testFile.indexOf(filter) != 0))
                     return;
                 testFileQueue.addTask(function (error, callback) {
                     var testContent = fs.readFileSync(pathToTests(testFile), 'utf-8');
@@ -282,6 +283,8 @@ function TestEnvironment(testCaseQueue) {
         return hash.digest('hex');
     }
 
+    this.config = config;
+
     this.notifyDone = function () { currentTestContext.done(); }
 }
 
@@ -334,4 +337,4 @@ function TestContext(testGroup, testCase, callback) {
     process.stdout.write(this.description() + ': ');
 }
 
-main();
+main(process.argv);
index d5945e8..4d3adb9 100644 (file)
@@ -372,12 +372,12 @@ describe("/api/report", function () {
         });
     }
 
-    it("should reject when aggregators are missing", function () {
+    it("should not reject when aggregators are missing", function () {
         addBuilder(reportWithTwoLevelsOfAggregations, function () {
             postJSON('/api/report/', reportWithTwoLevelsOfAggregations, function (response) {
                 assert.equal(response.statusCode, 200);
-                assert.equal(JSON.parse(response.responseText)['status'], 'AggregatorNotFound');
-                assert.equal(JSON.parse(response.responseText)['failureStored'], true);
+                assert.equal(JSON.parse(response.responseText)['status'], 'OK');
+                assert.equal(JSON.parse(response.responseText)['failureStored'], false);
                 notifyDone();
             });
         });