Support Griffin in perf tests (#16016)

* Support brave field trials in perf tests
* Formating
* Fix review issues
* Add win cmd size assert

chromium_src/tools/perf/core/perf_benchmark.py

@@ -0,0 +1,44 @@
+# Copyright (c) 2022 The Brave Authors. All rights reserved.
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this file,
+# you can obtain one at https://mozilla.org/MPL/2.0/.
+"""A inline part of perf_benchmark.py"""
+
+import override_utils
+
+@override_utils.override_method(PerfBenchmark)
+def _GetVariationsBrowserArgs(self, original_method,
+                              finder_options,
+                              current_args,
+                              possible_browser=None):
+  """Override to pass field_trials to Brave browser
+
+  It parses json config location from browser args (--field-trial-config) and
+  uses chromium GenerateArgs() to generate a cmd-line to enable proper
+  features & trials.
+  Note: it should be used instead of --enable-field-trial-config.
+  """
+  field_trial_config = None
+  for arg in current_args:
+    PREFIX = '--field-trial-config='
+    if arg.startswith(PREFIX):
+      field_trial_config = arg[len(PREFIX):]
+
+  if field_trial_config:
+    if possible_browser is None:
+      possible_browser = browser_finder.FindBrowser(finder_options)
+    if not possible_browser:
+      return []
+    target_os = self.FixupTargetOS(possible_browser.target_os)
+
+    args = fieldtrial_util.GenerateArgs(
+      field_trial_config,
+      target_os,
+      current_args)
+    if target_os == 'windows':
+      # Windows system has 8k cmd size limit. There is not way to pass a huge
+      # trials using this method. If you get this assert consider simplifying
+      # Griffin config or bundling the testing_field_trials to the browser.
+      assert sum(len(x) + 1 for x in args) < 7000, 'cmd line is near the limit'
+    return args
+  return original_method(self, finder_options, current_args, possible_browser)

patches/tools-perf-core-perf_benchmark.py.patch

@@ -0,0 +1,9 @@
+diff --git a/tools/perf/core/perf_benchmark.py b/tools/perf/core/perf_benchmark.py
+index 2da4191c86d584a578d821863ee482abb85c20d0..0512f8cdf20e6b39d078b24df027feaa10661cbf 100644
+--- a/tools/perf/core/perf_benchmark.py
++++ b/tools/perf/core/perf_benchmark.py
+@@ -211,3 +211,4 @@ class PerfBenchmark(benchmark.Benchmark):
+     if 'XVFB_DISPLAY' in os.environ:
+       return True
+     return False
++from import_inline import inline_file_from_src; inline_file_from_src("brave/chromium_src/tools/perf/core/perf_benchmark.py", globals(), locals())

tools/perf/components/browser_type.py

@@ -11,7 +11,7 @@ import json
 import shutil
 import re
 
-from typing import List
+from typing import List, Optional
 
 from urllib.request import urlopen
 from io import BytesIO
@@ -101,14 +101,23 @@ class BrowserType:
   def DownloadBrowserBinary(self, tag: str, out_dir: str) -> str:
     raise NotImplementedError()
 
+  # pylint: disable=no-self-use
+  def MakeFieldTrials(self, _tag: str, _out_dir: str,
+                      _variations_repo_dir: Optional[str]) -> Optional[str]:
+    return None
+
 
 class BraveBrowserTypeImpl(BrowserType):
   _channel: str
+  _use_field_trials: bool
 
-  def __init__(self, name: str, channel: str, extra_browser_args: List[str],
-               extra_benchmark_args: List[str]):
-    super().__init__(name, extra_browser_args, extra_benchmark_args, False)
+  def __init__(self, name: str, channel: str, use_field_trials: bool):
+    extra_benchmark_args = []
+    if not use_field_trials:
+      extra_benchmark_args.append('--compatibility-mode=no-field-trials')
+    super().__init__(name, [], extra_benchmark_args, False)
     self._channel = channel
+    self._use_field_trials = use_field_trials
 
   @classmethod
   def _GetSetupDownloadUrl(cls, tag) -> str:
@@ -141,23 +150,69 @@ class BraveBrowserTypeImpl(BrowserType):
 
     return _DownloadArchiveAndUnpack(out_dir, self._GetZipDownloadUrl(tag))
 
+  def MakeFieldTrials(self, tag: str, out_dir: str,
+                      variations_repo_dir: Optional[str]) -> Optional[str]:
+    if not self._use_field_trials:
+      return None
+    if not variations_repo_dir:
+      raise RuntimeError('Set --variations-repo-dir to use field trials')
+    return _MakeTestingFieldTrials(out_dir, tag, variations_repo_dir)
+
 
 def _ParseVersion(version_string) -> List[str]:
   return version_string.split('.')
 
 
+def _FetchTag(tag: str):
+  tag_str = f'refs/tags/{tag}'
+  args = ['git', 'fetch', 'origin', tag_str]
+  GetProcessOutput(args, cwd=path_util.GetBraveDir(), check=True)
+  return tag_str
+
+
+def _GetBuildDate(tag: str) -> str:
+  tag_str = _FetchTag(tag)
+  _, output = GetProcessOutput(['git', 'show', '-s', '--format=%ci', tag_str],
+                               cwd=path_util.GetBraveDir(),
+                               check=True)
+  return output.rstrip()
+
+
+def _MakeTestingFieldTrials(out_dir: str,
+                            tag: str,
+                            variations_repo_dir: str,
+                            branch: str = 'production') -> str:
+  chromium_version = _ParseVersion(_GetChromiumVersion(tag))
+  assert re.match(r'v\d+\.\d+\.\d+', tag)
+  combined_version = chromium_version[0] + '.' + tag[1:]
+  logging.debug('combined_version %s', combined_version)
+  target_path = os.path.join(out_dir, 'fieldtrial_testing_config.json')
+
+  date = _GetBuildDate(tag)
+  args = [
+      'python3', 'seed/fieldtrials_testing_config_generator.py',
+      f'--output={target_path}', f'--target-date={date}',
+      f'--target-branch={branch}', f'--target-version={combined_version}',
+      '--target-channel=NIGHTLY'
+  ]
+  GetProcessOutput(args, cwd=variations_repo_dir, check=True)
+  return target_path
+
+
+def _GetChromiumVersion(tag: str) -> str:
+  tag_str = _FetchTag(tag)
+  package_json = json.loads(
+      subprocess.check_output(['git', 'show', f'{tag_str}:package.json'],
+                              cwd=path_util.GetBraveDir()))
+  return package_json['config']['projects']['chrome']['tag']
+
+
 def _GetNearestChromiumUrl(tag: str) -> str:
   chrome_versions = {}
   with open(path_util.GetChromeReleasesJsonPath(), 'r') as config_file:
     chrome_versions = json.load(config_file)
 
-  args = ['git', 'fetch', 'origin', (f'refs/tags/{tag}')]
-  logging.debug('Run binary: %s', ' '.join(args))
-  subprocess.check_call(args, cwd=path_util.GetBraveDir())
-  package_json = json.loads(
-      subprocess.check_output(['git', 'show', 'FETCH_HEAD:package.json'],
-                              cwd=path_util.GetBraveDir()))
-  requested_version = package_json['config']['projects']['chrome']['tag']
+  requested_version = _GetChromiumVersion(tag)
   logging.debug('Got requested_version: %s', requested_version)
 
   parsed_requested_version = _ParseVersion(requested_version)
@@ -210,8 +265,9 @@ def ParseBrowserType(string_type: str) -> BrowserType:
                                  ['--compatibility-mode=no-field-trials'],
                                  False)
   if string_type == 'brave':
-    return BraveBrowserTypeImpl('brave', 'Nightly', [],
-                                ['--compatibility-mode=no-field-trials'])
+    return BraveBrowserTypeImpl('brave', 'Nightly', True)
+  if string_type == 'brave_no_trials':
+    return BraveBrowserTypeImpl('brave', 'Nightly', False)
   if string_type.startswith('custom'):
     return BrowserType(string_type, [], [], False)
 

tools/perf/components/perf_config.py

@@ -12,6 +12,7 @@ from components.browser_type import BrowserType, ParseBrowserType
 
 
 class RunnerConfig:
+  """A description of a browser configuration that is able to run tests."""
   tag: Optional[str] = None
   location: Optional[str] = None
   label: Optional[str] = None
@@ -41,6 +42,8 @@ class RunnerConfig:
 
 
 class BenchmarkConfig:
+  """A description of one benchmark that can be launched on some RunnerConfigs.
+  """
   name: str
   pageset_repeat: int = 1
   stories: List[str]
@@ -59,6 +62,11 @@ class BenchmarkConfig:
 
 
 class PerfConfig:
+  """A config includes configurations & benchmarks that should be launched.
+
+  Each benchmark is launched on each configuration.
+  The class has 1-1 match to .json5 files used to setup tests.
+  """
   runners: List[RunnerConfig]
   benchmarks: List[BenchmarkConfig]
 

tools/perf/components/perf_test_runner.py

@@ -6,12 +6,11 @@
 import os
 import json
 import logging
-import shlex
 import shutil
 import time
 
 from copy import deepcopy
-from typing import Tuple, Optional, List
+from typing import Tuple, Optional, List, NamedTuple
 
 from components import path_util, perf_profile
 from components.perf_config import BenchmarkConfig, RunnerConfig
@@ -21,37 +20,24 @@ from components.perf_test_utils import (GetRevisionNumberAndHash,
                                         GetProcessOutput)
 
 
-class CommonOptions:
-  verbose = False
-  do_run_tests = True
-  do_report = False
-  report_on_failure = False
-  local_run = False
-  working_directory = ''
-  benchmarks: List[BenchmarkConfig]
+class CommonOptions(NamedTuple):
+  verbose: bool = False
+  do_run_tests: bool = True
+  do_report: bool = False
+  report_on_failure: bool = False
+  local_run: bool = False
+  variations_repo_dir: Optional[str] = None
+  working_directory: str = ''
 
   @classmethod
-  def make_local(cls, working_directory: str, verbose: bool,
-                 benchmarks: List[BenchmarkConfig]) -> 'CommonOptions':
-    options = CommonOptions()
-    options.verbose = verbose
-    options.working_directory = working_directory
-    options.benchmarks = benchmarks
-    options.local_run = True
-    return options
-
-  @classmethod
-  def from_args(cls, args,
-                benchmarks: List[BenchmarkConfig]) -> 'CommonOptions':
-    options = CommonOptions()
-    options.verbose = args.verbose
-    options.do_run_tests = not args.report_only
-    options.do_report = not args.no_report and not args.local_run
-    options.report_on_failure = args.report_on_failure
-    options.local_run = args.local_run
-    options.working_directory = args.working_directory
-    options.benchmarks = benchmarks
-    return options
+  def from_args(cls, args) -> 'CommonOptions':
+    return CommonOptions(verbose=args.verbose,
+                         do_run_tests=not args.report_only,
+                         do_report=not args.no_report and not args.local_run,
+                         report_on_failure=args.report_on_failure,
+                         local_run=args.local_run,
+                         variations_repo_dir=args.variations_repo_dir,
+                         working_directory=args.working_directory)
 
 
 def ReportToDashboardImpl(browser_type: BrowserType, dashboard_bot_name: str,
@@ -107,23 +93,29 @@ def ReportToDashboardImpl(browser_type: BrowserType, dashboard_bot_name: str,
   return False, ['Reporting ' + revision + ' failed'], None
 
 
+# pylint: disable=too-many-instance-attributes
 class RunableConfiguration:
   common_options: CommonOptions
-
+  benchmarks: List[BenchmarkConfig]
   config: RunnerConfig
   binary_path: Optional[str] = None
   out_dir: str
   profile_dir: Optional[str] = None
+  field_trial_config: Optional[str] = None
 
   status_line: str = ''
   logs: List[str] = []
 
-  def __init__(self, config: RunnerConfig, binary_path: Optional[str],
-               out_dir: str, common_options: CommonOptions):
+  def __init__(self, config: RunnerConfig, benchmarks: List[BenchmarkConfig],
+               binary_path: Optional[str], out_dir: str,
+               common_options: CommonOptions,
+               field_trial_config: Optional[str]):
     self.config = config
+    self.benchmarks = benchmarks
     self.binary_path = binary_path
     self.out_dir = out_dir
     self.common_options = common_options
+    self.field_trial_config = field_trial_config
 
   def PrepareProfile(self) -> bool:
     start_time = time.time()
@@ -148,15 +140,12 @@ class RunableConfiguration:
     rebase_benchmark.stories = ['BraveSearch_cold']
     rebase_benchmark.pageset_repeat = 1
 
-    return self.RunSingleTest(self.profile_dir, self.binary_path,
-                              rebase_runner_config, rebase_benchmark, None,
-                              True, self.common_options.verbose)
+    return self.RunSingleTest(rebase_runner_config, rebase_benchmark, None,
+                              True)
 
-  @classmethod
-  def RunSingleTest(cls, profile_dir: Optional[str], binary_path: str,
-                    config: RunnerConfig, benchmark_config: BenchmarkConfig,
-                    out_dir: Optional[str], local_run: bool,
-                    verbose: bool) -> bool:
+  def RunSingleTest(self, config: RunnerConfig,
+                    benchmark_config: BenchmarkConfig, out_dir: Optional[str],
+                    local_run: bool) -> bool:
     args = [path_util.GetVpython3Path()]
     args.append(os.path.join(path_util.GetChromiumPerfDir(), 'run_benchmark'))
 
@@ -178,17 +167,15 @@ class RunableConfiguration:
                        'run_performance_tests.py'))
 
       args.append(f'--benchmarks={benchmark_name}')
-      if out_dir:
-        args.append('--isolated-script-test-output=' +
-                    os.path.join(out_dir, benchmark_name, 'output.json'))
-      else:
-        args.append('--output-format=none')
+      assert out_dir
+      args.append('--isolated-script-test-output=' +
+                  os.path.join(out_dir, benchmark_name, 'output.json'))
 
-    if profile_dir:
-      args.append(f'--profile-dir={profile_dir}')
+    if self.profile_dir:
+      args.append(f'--profile-dir={self.profile_dir}')
 
     args.append('--browser=exact')
-    args.append(f'--browser-executable={binary_path}')
+    args.append(f'--browser-executable={self.binary_path}')
     args.append('--pageset-repeat=%d' % benchmark_config.pageset_repeat)
 
     if len(benchmark_config.stories) > 0:
@@ -197,15 +184,18 @@ class RunableConfiguration:
 
     extra_browser_args = deepcopy(config.extra_browser_args)
     extra_browser_args.extend(config.browser_type.GetExtraBrowserArgs())
+    if self.field_trial_config:
+      extra_browser_args.append(
+          f'--field-trial-config={self.field_trial_config}')
 
     args.extend(config.browser_type.GetExtraBenchmarkArgs())
     args.extend(config.extra_benchmark_args)
 
-    if verbose:
-      args.append('--show-stdout')
+    if self.common_options.verbose:
+      args.extend(['--show-stdout', '--verbose'])
 
     if len(extra_browser_args) > 0:
-      args.append('--extra-browser-args=' + shlex.join(extra_browser_args))
+      args.append('--extra-browser-args=' + ' '.join(extra_browser_args))
 
     success, _ = GetProcessOutput(args, cwd=path_util.GetChromiumPerfDir())
     return success
@@ -223,17 +213,15 @@ class RunableConfiguration:
       return False
 
     start_time = time.time()
-    for benchmark in self.common_options.benchmarks:
+    for benchmark in self.benchmarks:
       if self.common_options.local_run:
         test_out_dir = os.path.join(self.out_dir, os.pardir, benchmark.name)
       else:
         test_out_dir = os.path.join(self.out_dir, 'results')
       logging.info('Running test %s', benchmark.name)
 
-      test_success = self.RunSingleTest(self.profile_dir, self.binary_path,
-                                        self.config, benchmark, test_out_dir,
-                                        self.common_options.local_run,
-                                        self.common_options.verbose)
+      test_success = self.RunSingleTest(self.config, benchmark, test_out_dir,
+                                        self.common_options.local_run)
 
       if not test_success:
         has_failure = True
@@ -288,6 +276,7 @@ class RunableConfiguration:
 
 
 def PrepareBinariesAndDirectories(configurations: List[RunnerConfig],
+                                  benchmarks: List[BenchmarkConfig],
                                   common_options: CommonOptions
                                   ) -> List[RunableConfiguration]:
   runable_configurations: List[RunableConfiguration] = []
@@ -300,15 +289,20 @@ def PrepareBinariesAndDirectories(configurations: List[RunnerConfig],
         description += f'[tag_{config.tag}]'
     assert (description)
     out_dir = os.path.join(common_options.working_directory, description)
+
     binary_path = None
 
     if common_options.do_run_tests:
       shutil.rmtree(out_dir, True)
+      os.makedirs(out_dir)
       binary_path = PrepareBinary(out_dir, config.tag, config.location,
                                   config.browser_type)
+      field_trial_config = config.browser_type.MakeFieldTrials(
+          config.tag, out_dir, common_options.variations_repo_dir)
       logging.info('%s : %s directory %s', description, binary_path, out_dir)
     runable_configurations.append(
-        RunableConfiguration(config, binary_path, out_dir, common_options))
+        RunableConfiguration(config, benchmarks, binary_path, out_dir,
+                             common_options, field_trial_config))
   return runable_configurations
 
 
@@ -329,9 +323,10 @@ def SpawnConfigurationsFromTargetList(target_list: List[str],
 
 
 def RunConfigurations(configurations: List[RunnerConfig],
+                      benchmarks: List[BenchmarkConfig],
                       common_options: CommonOptions) -> bool:
   runable_configurations = PrepareBinariesAndDirectories(
-      configurations, common_options)
+      configurations, benchmarks, common_options)
 
   has_failure = False
   logs: List[str] = []
@@ -342,7 +337,7 @@ def RunConfigurations(configurations: List[RunnerConfig],
     logs.extend(config_logs)
 
   if common_options.local_run:
-    for benchmark in common_options.benchmarks:
+    for benchmark in benchmarks:
       logs.append(benchmark.name + ' : file://' + os.path.join(
           common_options.working_directory, benchmark.name, 'results.html'))
 

tools/perf/perf_compare.py

@@ -38,11 +38,10 @@ def main():
   json_config = perf_test_utils.LoadJsonConfig(args.config)
   config = perf_config.PerfConfig(json_config)
 
-  common_options = perf_test_runner.CommonOptions.make_local(
-      args.working_directory, args.verbose, config.benchmarks)
+  common_options = perf_test_runner.CommonOptions.from_args(args)
 
-  return 0 if perf_test_runner.RunConfigurations(config.runners,
-                                                 common_options) else 1
+  return 0 if perf_test_runner.RunConfigurations(
+      config.runners, config.benchmarks, common_options) else 1
 
 
 if __name__ == '__main__':

tools/perf/run_dashboard_perftests.py

@@ -38,6 +38,8 @@ def main():
   parser.add_argument('--report-only', action='store_true')
   parser.add_argument('--report-on-failure', action='store_true')
   parser.add_argument('--local-run', action='store_true')
+  parser.add_argument('--variations-repo-dir', type=str)
+
   parser.add_argument('--verbose', action='store_true')
   args = parser.parse_args()
 
@@ -50,8 +52,7 @@ def main():
   json_config = perf_test_utils.LoadJsonConfig(args.config)
   config = perf_config.PerfConfig(json_config)
 
-  common_options = perf_test_runner.CommonOptions.from_args(
-      args, config.benchmarks)
+  common_options = perf_test_runner.CommonOptions.from_args(args)
 
   if len(config.runners) != 1:
     raise RuntimeError('Only one configuration should be specified.')
@@ -59,8 +60,8 @@ def main():
   configurations = perf_test_runner.SpawnConfigurationsFromTargetList(
       targets, config.runners[0])
 
-  return 0 if perf_test_runner.RunConfigurations(configurations,
-                                                 common_options) else 1
+  return 0 if perf_test_runner.RunConfigurations(
+      configurations, config.benchmarks, common_options) else 1
 
 
 if __name__ == '__main__':