Always put Telemetry tests at the end to avoid starving fast tests

This also add this check to PRESUBMIT perf_json_config_validator script

Bug: 873389
Cq-Include-Trybots: master.tryserver.chromium.perf:obbs_fyi
Change-Id: I16fd762e567d5a41d75a221456e12e14a03f0b85
Reviewed-on: https://chromium-review.googlesource.com/1171578Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#582422}

testing/buildbot/chromium.perf.fyi.json

@@ -184,13 +184,12 @@
       },
       {
         "args": [
-          "-v",
-          "--browser=release",
-          "--upload-results",
-          "--run-ref-build",
-          "--test-shard-map-filename=mac1012_5_shard_map.json"
+          "--gtest-benchmark-name",
+          "views_perftests",
+          "--non-telemetry=true",
+          "--migrated-test=true"
         ],
-        "isolate_name": "performance_test_suite",
+        "isolate_name": "views_perftests",
         "merge": {
           "args": [
             "--service-account-file",
@@ -198,9 +197,9 @@
           ],
           "script": "//tools/perf/process_perf_results.py"
         },
-        "name": "performance_test_suite",
+        "name": "views_perftests",
         "override_compile_targets": [
-          "performance_test_suite"
+          "views_perftests"
         ],
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -215,7 +214,7 @@
           "hard_timeout": 25200,
           "ignore_task_failure": false,
           "io_timeout": 1800,
-          "shards": 5,
+          "shards": 1,
           "upload_test_results": true
         },
         "trigger_script": {
@@ -228,12 +227,13 @@
       },
       {
         "args": [
-          "--gtest-benchmark-name",
-          "views_perftests",
-          "--non-telemetry=true",
-          "--migrated-test=true"
+          "-v",
+          "--browser=release",
+          "--upload-results",
+          "--run-ref-build",
+          "--test-shard-map-filename=mac1012_5_shard_map.json"
         ],
-        "isolate_name": "views_perftests",
+        "isolate_name": "performance_test_suite",
         "merge": {
           "args": [
             "--service-account-file",
@@ -241,9 +241,9 @@
           ],
           "script": "//tools/perf/process_perf_results.py"
         },
-        "name": "views_perftests",
+        "name": "performance_test_suite",
         "override_compile_targets": [
-          "views_perftests"
+          "performance_test_suite"
         ],
         "swarming": {
           "can_use_on_swarming_builders": true,
@@ -258,7 +258,7 @@
           "hard_timeout": 25200,
           "ignore_task_failure": false,
           "io_timeout": 1800,
-          "shards": 1,
+          "shards": 5,
           "upload_test_results": true
         },
         "trigger_script": {

tools/perf/core/perf_data_generator.py

@@ -70,14 +70,6 @@ NEW_PERF_RECIPE_FYI_TESTERS = {
   'testers' : {
     'OBBS Mac 10.12 Perf': {
       'tests': [
-        {
-          'isolate': 'performance_test_suite',
-          'extra_args': [
-            '--run-ref-build',
-            '--test-shard-map-filename=mac1012_5_shard_map.json',
-          ],
-          'num_shards': 5
-        },
         {
           'isolate': 'net_perftests',
           'num_shards': 1,
@@ -87,6 +79,14 @@ NEW_PERF_RECIPE_FYI_TESTERS = {
           'isolate': 'views_perftests',
           'num_shards': 1,
           'telemetry': False,
+        },
+        {
+          'isolate': 'performance_test_suite',
+          'extra_args': [
+            '--run-ref-build',
+            '--test-shard-map-filename=mac1012_5_shard_map.json',
+          ],
+          'num_shards': 5
         }
       ],
       'platform': 'mac',
@@ -949,11 +949,20 @@ def load_and_update_fyi_json(fyi_waterfall_file):
 
 def generate_telemetry_tests(testers, tests):
   for tester, tester_config in testers['testers'].iteritems():
-    isolated_scripts = []
+    telemetry_tests = []
+    gtest_tests = []
     for test in tester_config['tests']:
-      isolated_scripts.append(generate_performance_test(tester_config, test))
+      generated_script = generate_performance_test(tester_config, test)
+      if test.get('telemetry', True):
+        telemetry_tests.append(generated_script)
+      else:
+        gtest_tests.append(generated_script)
+    telemetry_tests.sort(key=lambda x: x['name'])
+    gtest_tests.sort(key=lambda x: x['name'])
     tests[tester] = {
-      'isolated_scripts': sorted(isolated_scripts, key=lambda x: x['name'])
+      # Put Telemetry tests as the end since they tend to run longer to avoid
+      # starving gtests (see crbug.com/873389).
+      'isolated_scripts': gtest_tests + telemetry_tests
     }
 
 

tools/perf/core/perf_json_config_validator.py

@@ -101,7 +101,9 @@ def _ValidateBrowserType(builder_name, test_config):
 
 def ValidateTestingBuilder(builder_name, builder_data):
   isolated_scripts = builder_data['isolated_scripts']
+  test_names = []
   for test_config in isolated_scripts:
+    test_names.append(test_config['name'])
     _ValidateSwarmingDimension(
         builder_name,
         swarming_dimensions=test_config['swarming'].get('dimension_sets', {}))
@@ -110,6 +112,17 @@ def ValidateTestingBuilder(builder_name, builder_data):
       _ValidateShardingData(builder_name, test_config)
       _ValidateBrowserType(builder_name, test_config)
 
+  if ('performance_test_suite' in test_names or
+      'performance_webview_test_suite' in test_names):
+    if test_names[-1] not in ('performance_test_suite',
+                              'performance_webview_test_suite'):
+      raise ValueError(
+          'performance_test_suite or performance_webview_test_suite must run '
+          'at the end of builder %s to avoid starving other test step '
+          '(see crbug.com/873389). Instead found %s' % (
+            repr(builder_name), test_names[-1]))
+
+
 
 def _IsBuilderName(name):
   return not name.startswith('AAA')