Changeset 236996 in webkit

Timestamp:

Oct 9, 2018, 8:15:39 PM (8 years ago)

Author:

rniwa@webkit.org

Message:

ManifestGenerator shouldn't need more than 1GB of memory or run for 30 seconds
​https://bugs.webkit.org/show_bug.cgi?id=190393

Reviewed by Antti Koivisto and unofficially reviewed by Dewei Zhu.

This patch reduces the runtime of /api/manifest from 13s to 7s and reduces the memory requirement from
1GB to 400MB for the internal dashboard in my local testing.

The biggest perf win comes from avoid running a complex query over test_configurations to compute
the latest modified date across different test configuration types ("current" vs. "baseline").
Instead, we now fetch the entire table row by row and compute the latest modified date in memory.

Also call intval in many more places to avoid generating double quotes, which is a pretty significant
proportion of the JSON file size at this point.

Finally, use references in more places to avoid deep copying of arrays.

• public/include/manifest-generator.php:

(ManifestGenerator::generate): Skip the generation of "dashboard" since it's only used by v1 UI.
(ManifestGenerator::tests): Don't copy each row.
(ManifestGenerator::metrics): Ditto.
(ManifestGenerator::platforms): Implement the aforementioned optimization. Instead of grouping
test configurations for a given metric and platform together, fetch them all and do in-memory
processing in PHP. Avoid more copying as well.
(ManifestGenerator::repositories): Avoid more copying.
(ManifestGenerator::bug_trackers): Ditto.
(ManifestGenerator::fetch_triggerables): Ditto.

Location:

trunk/Websites/perf.webkit.org

Files:

• ChangeLog (modified) (1 diff)
• public/include/manifest-generator.php (modified) (13 diffs)

trunk/Websites/perf.webkit.org/ChangeLog

@@ +1 @@
+2018-10-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        ManifestGenerator shouldn't need more than 1GB of memory or run for 30 seconds
+        https://bugs.webkit.org/show_bug.cgi?id=190393
+
+        Reviewed by Antti Koivisto and unofficially reviewed by Dewei Zhu.
+
+        This patch reduces the runtime of /api/manifest from 13s to 7s and reduces the memory requirement from
+        1GB to 400MB for the internal dashboard in my local testing.
+
+        The biggest perf win comes from avoid running a complex query over test_configurations to compute
+        the latest modified date across different test configuration types ("current" vs. "baseline").
+        Instead, we now fetch the entire table row by row and compute the latest modified date in memory.
+
+        Also call intval in many more places to avoid generating double quotes, which is a pretty significant
+        proportion of the JSON file size at this point.
+
+        Finally, use references in more places to avoid deep copying of arrays.
+
+        * public/include/manifest-generator.php:
+        (ManifestGenerator::generate): Skip the generation of "dashboard" since it's only used by v1 UI.
+        (ManifestGenerator::tests): Don't copy each row.
+        (ManifestGenerator::metrics): Ditto.
+        (ManifestGenerator::platforms): Implement the aforementioned optimization. Instead of grouping
+        test configurations for a given metric and platform together, fetch them all and do in-memory
+        processing in PHP. Avoid more copying as well.
+        (ManifestGenerator::repositories): Avoid more copying.
+        (ManifestGenerator::bug_trackers): Ditto.
+        (ManifestGenerator::fetch_triggerables): Ditto.
+
 2018-10-08  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Websites/perf.webkit.org/public/include/manifest-generator.php

@@ -30 +30 @@
         $metrics = (object)$this->metrics();
         $platforms = (object)$this->platforms($platform_table, false);
-        $dashboard = (object)$this->platforms($platform_table, true);
         $repositories = (object)$this->repositories($repositories_table, $repositories_with_commit);
 
@@ -38 +37 @@
             'metrics' => &$metrics,
             'all' => &$platforms,
-            'dashboard' => &$dashboard,
+            'dashboard' => (object)array(), // Only used by v1 UI.
             'repositories' => &$repositories,
             'builders' => (object)$this->builders(),
@@ -65 +64 @@
         if (!$tests_table)
             return $tests;
-        foreach ($tests_table as $test_row) {
+        foreach ($tests_table as &$test_row) {
             $tests[$test_row['test_id']] = array(
                 'name' => $test_row['test_name'],
                 'url' => $test_row['test_url'],
-                'parentId' => $test_row['test_parent'],
+                'parentId' => $test_row['test_parent'] ? intval($test_row['test_parent']) : NULL,
             );
         }
@@ -80 +79 @@
         if (!$metrics_table)
             return $metrics;
-        foreach ($metrics_table as $row) {
+        foreach ($metrics_table as &$row) {
             $metrics[$row['metric_id']] = array(
                 'name' => $row['metric_name'],
-                'test' => $row['metric_test'],
+                'test' => intval($row['metric_test']),
                 'aggregator' => $row['aggregator_name']);
         }
@@ -89 +88 @@
     }
 
-    private function platforms($platform_table, $is_dashboard) {
-        $metrics = $this->db->query_and_fetch_all('SELECT config_metric AS metric_id, config_platform AS platform_id,
-            extract(epoch from max(config_runs_last_modified) at time zone \'utc\') * 1000 AS last_modified, bool_or(config_is_in_dashboard) AS in_dashboard
-            FROM test_configurations GROUP BY config_metric, config_platform ORDER BY config_platform');
+    private function platforms(&$platform_table, $is_dashboard) {
+        $config_query = $this->db->query('SELECT config_platform, config_metric,
+            extract(epoch from config_runs_last_modified at time zone \'utc\') * 1000 AS last_modified
+            FROM test_configurations');
 
         $platform_metrics = array();
 
-        if ($metrics) {
+        if ($config_query) {
             $current_platform_entry = null;
-            foreach ($metrics as $metric_row) {
-                if ($is_dashboard && !Database::is_true($metric_row['in_dashboard']))
-                    continue;
-
-                $platform_id = $metric_row['platform_id'];
-                if (!$current_platform_entry || $current_platform_entry['id'] != $platform_id) {
-                    $current_platform_entry = &array_ensure_item_has_array($platform_metrics, $platform_id);
-                    $current_platform_entry['id'] = $platform_id;
-                    array_ensure_item_has_array($current_platform_entry, 'metrics');
-                    array_ensure_item_has_array($current_platform_entry, 'last_modified');
+            $last_modified_map = array();
+            while (1) {
+                $config_row = $this->db->fetch_next_row($config_query);
+                if (!$config_row)
+                    break;
+
+                $platform_id = $config_row['config_platform'];
+                $metric_id = $config_row['config_metric'];
+                $last_modified = intval($config_row['last_modified']);
+
+                $key = $platform_id . '-' . $metric_id;
+                if (array_key_exists($key, $last_modified_map)) {
+                    $last_modified_map[$key] = max($last_modified_map[$key], $last_modified);
+                    continue;
                 }
-
-                array_push($current_platform_entry['metrics'], $metric_row['metric_id']);
-                array_push($current_platform_entry['last_modified'], intval($metric_row['last_modified']));
+                $last_modified_map[$key] = $last_modified;
+
+                $current_platform_entry = &array_ensure_item_has_array($platform_metrics, $platform_id);
+                array_ensure_item_has_array($current_platform_entry, 'metrics');
+                array_push($current_platform_entry['metrics'], intval($metric_id));
             }
         }
@@ -118 +123 @@
         $platforms = array();
         if ($platform_table) {
-            foreach ($platform_table as $platform_row) {
+            foreach ($platform_table as &$platform_row) {
                 if (Database::is_true($platform_row['platform_hidden']))
                     continue;
                 $id = $platform_row['platform_id'];
                 if (array_key_exists($id, $platform_metrics)) {
+                    $metrics = &$platform_metrics[$id]['metrics'];
+                    $last_modified = array();
+                    foreach ($metrics as $metric_id)
+                        array_push($last_modified, $last_modified_map[$id . '-' . $metric_id]);
                     $platforms[$id] = array(
-                        'name' => $platform_row['platform_name'],
-                        'metrics' => $platform_metrics[$id]['metrics'],
-                        'lastModified' => $platform_metrics[$id]['last_modified']);
+                        'name' => &$platform_row['platform_name'],
+                        'metrics' => &$metrics,
+                        'lastModified' => &$last_modified
+                    );
                 }
             }
@@ -137 +147 @@
         if (!$repositories_table)
             return $repositories;
-        foreach ($repositories_table as $row) {
+        foreach ($repositories_table as &$row) {
             $repositories[$row['repository_id']] = array(
                 'name' => $row['repository_name'],
@@ -154 +164 @@
             return array();
         $builders = array();
-        foreach ($builders_table as $row)
+        foreach ($builders_table as &$row)
             $builders[$row['builder_id']] = array('name' => $row['builder_name'], 'buildUrl' => $row['builder_build_url']);
 
@@ -173 +183 @@
         $bug_trackers_table = $this->db->fetch_table('bug_trackers');
         if ($bug_trackers_table) {
-            foreach ($bug_trackers_table as $row) {
+            foreach ($bug_trackers_table as &$row) {
                 $bug_trackers[$row['tracker_id']] = array(
                     'name' => $row['tracker_name'],
@@ -217 +227 @@
                 array_ensure_item_has_array($repository_set_by_group, $group_id);
                 array_push($repository_set_by_group[$group_id], array(
-                    'repository' => $repository_row['trigrepo_repository'],
+                    'repository' => intval($repository_row['trigrepo_repository']),
                     'acceptsPatch' => Database::is_true($repository_row['trigrepo_accepts_patch'])));
             }
@@ -228 +238 @@
                 $repository_list = array_get($repository_set_by_group, $group_id, array());
                 array_push($triggerable['repositoryGroups'], array(
-                    'id' => $group_row['repositorygroup_id'],
+                    'id' => intval($group_row['repositorygroup_id']),
                     'name' => $group_row['repositorygroup_name'],
                     'description' => $group_row['repositorygroup_description'],
@@ -235 +245 @@
                     'repositories' => $repository_list));
                 // V2 UI compatibility.
-                foreach ($repository_list as $repository_data) {
+                foreach ($repository_list as &$repository_data) {
                     $repository_id = $repository_data['repository'];
                     $set = &$triggerable_id_to_repository_set[$triggerable_id];
@@ -254 +264 @@
                     continue;
                 $triggerable = &$id_to_triggerable[$triggerable_id];
-                array_push($triggerable['configurations'], array($row['trigconfig_test'], $row['trigconfig_platform']));
+                array_push($triggerable['configurations'], array(intval($row['trigconfig_test']), intval($row['trigconfig_platform'])));
             }
         }