[tools] Add variants support for run_perf.py

We usually run benchmarks in multiple variants: default, future, noopt
This is currently only achieved by copying the run-perf json file and
changing the flags at the top-level (or copy whole subsections).

Using "variants" we can duplicate the tests at the current level with
different values and easily create benchmarks that differ only in v8
flags.

Drive-by-fix:
- Add Node.__iter__ and log the whole config graph in debug mode
- Add GraphConfig.__str__ method for better debugging
- Rename TraceConfig to LeafTraceConfig
- Rename RunnableTraceConfig to RunnableLeafTraceConfig
- Make --filter accept a regexp to better filter out variants

Bug: v8:12821, v8:11113
Change-Id: I56a2ba2dd24da15c7757406e9961746219cd8061
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3596128Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Tamer Tas <tmrts@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80307}

tools/run_perf.py

@@ -24,6 +24,13 @@ The suite json format is expected to be:
   "retry_count": <how many times to retry failures (in addition to first try)",
   "retry_count_XXX": <how many times to retry failures for arch XXX>
   "resources": [<js file to be moved to android device>, ...]
+  "variants": [
+    {
+      "name": <name of the variant>,
+      "flags": [<flag to the test file>, ...],
+      <other suite properties>
+    }, ...
+  ]
   "main": <main js perf runner file>,
   "results_regexp": <optional regexp>,
   "results_processor": <optional python results processor script>,
@@ -57,6 +64,10 @@ The results_regexp will be applied to the processed output.
 
 A suite without "tests" is considered a performance test itself.
 
+Variants can be used to run different configurations at the current level. This
+essentially copies the sub suites at the current level and can be used to avoid
+duplicating a lot of nested "tests" were for instance only the "flags" change.
+
 Full example (suite with one runner):
 {
   "path": ["."],
@@ -81,10 +92,15 @@ Full example (suite with several runners):
 {
   "path": ["."],
   "owners": ["username@chromium.org", "otherowner@google.com"],
-  "flags": ["--expose-gc"],
   "archs": ["ia32", "x64"],
+  "flags": ["--expose-gc"]},
   "run_count": 5,
   "units": "score",
+  "variants:" {
+    {"name": "default", "flags": []},
+    {"name": "future",  "flags": ["--future"]},
+    {"name": "noopt",   "flags": ["--noopt"]},
+  }
   "tests": [
     {"name": "Richards",
      "path": ["richards"],
@@ -243,7 +259,7 @@ class ResultTracker(object):
     Returns:
       True if specified confidence level have been achieved.
     """
-    if not isinstance(graph_config, TraceConfig):
+    if not isinstance(graph_config, LeafTraceConfig):
       return all(self.HasEnoughRuns(child, confidence_level)
                  for child in graph_config.children)
 
@@ -300,6 +316,11 @@ class Node(object):
   def children(self):
     return self._children
 
+  def __iter__(self):
+    yield self
+    for child in self.children:
+      yield from iter(child)
+
 
 class DefaultSentinel(Node):
   """Fake parent node with all default values."""
@@ -321,6 +342,10 @@ class DefaultSentinel(Node):
     self.units = 'score'
     self.total = False
     self.owners = []
+    self.main = None
+
+  def __str__(self):
+    return type(self).__name__
 
 
 class GraphConfig(Node):
@@ -339,6 +364,11 @@ class GraphConfig(Node):
     assert isinstance(suite.get('test_flags', []), list)
     assert isinstance(suite.get('resources', []), list)
 
+    # Only used by child classes
+    self.main = suite.get('main', parent.main)
+    # Keep parent for easier debugging
+    self.parent = parent
+
     # Accumulated values.
     self.path = parent.path[:] + suite.get('path', [])
     self.graphs = parent.graphs[:] + [suite['name']]
@@ -368,11 +398,15 @@ class GraphConfig(Node):
     # suite name is expected.
     # TODO(machenbach): Currently that makes only sense for the leaf level.
     # Multiple place holders for multiple levels are not supported.
-    if parent.results_regexp:
-      regexp_default = parent.results_regexp % re.escape(suite['name'])
-    else:
-      regexp_default = None
-    self.results_regexp = suite.get('results_regexp', regexp_default)
+    self.results_regexp = suite.get('results_regexp', None)
+    if self.results_regexp is None and parent.results_regexp:
+      try:
+        self.results_regexp = parent.results_regexp % re.escape(suite['name'])
+      except TypeError as e:
+        raise TypeError(
+            "Got error while preparing results_regexp: "
+            "parent.results_regexp='%s' suite.name='%s' suite='%s', error: %s" %
+            (parent.results_regexp, suite['name'], str(suite)[:100], e))
 
     # A similar regular expression for the standard deviation (optional).
     if parent.stddev_regexp:
@@ -385,13 +419,39 @@ class GraphConfig(Node):
   def name(self):
     return '/'.join(self.graphs)
 
+  def __str__(self):
+    return "%s(%s)" % (type(self).__name__, self.name)
 
-class TraceConfig(GraphConfig):
+
+class VariantConfig(GraphConfig):
+  """Represents an intermediate node that has children that are all
+  variants of each other"""
+
+  def __init__(self, suite, parent, arch):
+    super(VariantConfig, self).__init__(suite, parent, arch)
+    assert "variants" in suite
+    for variant in suite.get('variants'):
+      assert "variants" not in variant, \
+        "Cannot directly nest variants:" + str(variant)[:100]
+      assert "name" in variant, \
+          "Variant must have 'name' property: " + str(variant)[:100]
+      assert len(variant) >= 2, \
+          "Variant must define other properties than 'name': " + str(variant)
+
+
+class LeafTraceConfig(GraphConfig):
   """Represents a leaf in the suite tree structure."""
   def __init__(self, suite, parent, arch):
-    super(TraceConfig, self).__init__(suite, parent, arch)
+    super(LeafTraceConfig, self).__init__(suite, parent, arch)
     assert self.results_regexp
-    assert self.owners
+    if '%s' in self.results_regexp:
+      raise Exception(
+          "results_regexp at the wrong level. "
+          "Regexp should not contain '%%s': results_regexp='%s' name=%s" %
+          (self.results_regexp, self.name))
+
+  def AppendChild(self, node):
+    raise Exception("%s cannot have child configs." % type(self).__name__)
 
   def ConsumeOutput(self, output, result_tracker):
     """Extracts trace results from the output.
@@ -403,6 +463,14 @@ class TraceConfig(GraphConfig):
     Returns:
       The raw extracted result value or None if an error occurred.
     """
+
+    if len(self.children) > 0:
+      results_for_total = []
+      for trace in self.children:
+        result = trace.ConsumeOutput(output, result_tracker)
+        if result:
+          results_for_total.append(result)
+
     result = None
     stddev = None
 
@@ -435,16 +503,59 @@ class TraceConfig(GraphConfig):
     return result
 
 
-class RunnableConfig(GraphConfig):
+class TraceConfig(GraphConfig):
+  """
+  A TraceConfig contains either TraceConfigs or LeafTraceConfigs
+  """
+
+  def ConsumeOutput(self, output, result_tracker):
+    """Processes test run output and updates result tracker.
+
+    Args:
+      output: Output object from the test run.
+      result_tracker: ResultTracker object to be updated.
+      count: Index of the test run (used for better logging).
+    """
+    results_for_total = []
+    for trace in self.children:
+      result = trace.ConsumeOutput(output, result_tracker)
+      if result:
+        results_for_total.append(result)
+
+    if self.total:
+      # Produce total metric only when all traces have produced results.
+      if len(self.children) != len(results_for_total):
+        result_tracker.AddError(
+            'Not all traces have produced results. Can not compute total for '
+            '%s.' % self.name)
+        return
+
+      # Calculate total as a the geometric mean for results from all traces.
+      total_trace = LeafTraceConfig(
+          {
+              'name': 'Total',
+              'units': self.children[0].units
+          }, self, self.arch)
+      result_tracker.AddTraceResult(total_trace,
+                                    GeometricMean(results_for_total), '')
+
+  def AppendChild(self, node):
+    if node.__class__ not in (TraceConfig, LeafTraceConfig):
+      raise Exception(
+          "%s only allows TraceConfig and LeafTraceConfig as child configs." %
+          type(self).__name__)
+    super(TraceConfig, self).AppendChild(node)
+
+
+class RunnableConfig(TraceConfig):
   """Represents a runnable suite definition (i.e. has a main file).
   """
   def __init__(self, suite, parent, arch):
     super(RunnableConfig, self).__init__(suite, parent, arch)
     self.arch = arch
-
-  @property
-  def main(self):
-    return self._suite.get('main', '')
+    assert self.main, "No main js file provided"
+    if not self.owners:
+      logging.error("No owners provided for %s" % self.name)
 
   def ChangeCWD(self, suite_path):
     """Changes the cwd to to path defined in the current graph.
@@ -491,69 +602,107 @@ class RunnableConfig(GraphConfig):
     if self.results_processor:
       output = RunResultsProcessor(self.results_processor, output, count)
 
-    results_for_total = []
-    for trace in self.children:
-      result = trace.ConsumeOutput(output, result_tracker)
-      if result:
-        results_for_total.append(result)
-
-    if self.total:
-      # Produce total metric only when all traces have produced results.
-      if len(self.children) != len(results_for_total):
-        result_tracker.AddError(
-            'Not all traces have produced results. Can not compute total for '
-            '%s.' % self.name)
-        return
-
-      # Calculate total as a the geometric mean for results from all traces.
-      total_trace = TraceConfig(
-          {'name': 'Total', 'units': self.children[0].units}, self, self.arch)
-      result_tracker.AddTraceResult(
-          total_trace, GeometricMean(results_for_total), '')
+    self.ConsumeOutput(output, result_tracker)
 
 
-class RunnableTraceConfig(TraceConfig, RunnableConfig):
+class RunnableLeafTraceConfig(LeafTraceConfig, RunnableConfig):
   """Represents a runnable suite definition that is a leaf."""
   def __init__(self, suite, parent, arch):
-    super(RunnableTraceConfig, self).__init__(suite, parent, arch)
+    super(RunnableLeafTraceConfig, self).__init__(suite, parent, arch)
+    if not self.owners:
+      logging.error("No owners provided for %s" % self.name)
 
   def ProcessOutput(self, output, result_tracker, count):
     result_tracker.AddRunnableDuration(self, output.duration)
     self.ConsumeOutput(output, result_tracker)
 
 
-def MakeGraphConfig(suite, arch, parent):
+def MakeGraphConfig(suite, parent, arch):
+  cls = GetGraphConfigClass(suite, parent)
+  return cls(suite, parent, arch)
+
+
+def GetGraphConfigClass(suite, parent):
   """Factory method for making graph configuration objects."""
-  if isinstance(parent, RunnableConfig):
-    # Below a runnable can only be traces.
-    return TraceConfig(suite, parent, arch)
+  if isinstance(parent, TraceConfig):
+    if suite.get("tests"):
+      return TraceConfig
+    return LeafTraceConfig
   elif suite.get('main') is not None:
     # A main file makes this graph runnable. Empty strings are accepted.
     if suite.get('tests'):
       # This graph has subgraphs (traces).
-      return RunnableConfig(suite, parent, arch)
+      return RunnableConfig
     else:
       # This graph has no subgraphs, it's a leaf.
-      return RunnableTraceConfig(suite, parent, arch)
+      return RunnableLeafTraceConfig
   elif suite.get('tests'):
     # This is neither a leaf nor a runnable.
-    return GraphConfig(suite, parent, arch)
+    return GraphConfig
   else:  # pragma: no cover
-    raise Exception('Invalid suite configuration.')
+    raise Exception('Invalid suite configuration.' + str(suite)[:200])
 
 
-def BuildGraphConfigs(suite, arch, parent):
+def BuildGraphConfigs(suite, parent, arch):
   """Builds a tree structure of graph objects that corresponds to the suite
   configuration.
-  """
 
+  - GraphConfig:
+    - Can have arbitrary children
+    - can be used to store properties used by it's children
+
+  - VariantConfig
+    - Has variants of the same (any) type as children
+
+  For all other configs see the override AppendChild methods.
+
+  Example 1:
+  - GraphConfig
+    - RunnableLeafTraceConfig (no children)
+    -  ...
+
+  Example 2:
+  - RunnableConfig
+    - LeafTraceConfig (no children)
+    - ...
+
+  Example 3:
+  - RunnableConfig
+    - LeafTraceConfig (optional)
+    - TraceConfig
+      - LeafTraceConfig (no children)
+      - ...
+      - TraceConfig (optional)
+        - ...
+      - ...
+
+  Example 4:
+  - VariantConfig
+    - RunnableConfig
+      - ...
+    - RunnableConfig
+      - ...
+  """
   # TODO(machenbach): Implement notion of cpu type?
   if arch not in suite.get('archs', SUPPORTED_ARCHS):
     return None
 
-  graph = MakeGraphConfig(suite, arch, parent)
+  variants = suite.get('variants', [])
+  if len(variants) == 0:
+    graph = MakeGraphConfig(suite, parent, arch)
+    for subsuite in suite.get('tests', []):
+      BuildGraphConfigs(subsuite, graph, arch)
+  else:
+    graph = VariantConfig(suite, parent, arch)
+    variant_class = GetGraphConfigClass(suite, parent)
+    for variant_suite in variants:
+      # Propagate down the results_regexp if it's not override in the variant
+      variant_suite.setdefault('results_regexp',
+                               suite.get('results_regexp', None))
+      variant_graph = variant_class(variant_suite, graph, arch)
+      graph.AppendChild(variant_graph)
       for subsuite in suite.get('tests', []):
-    BuildGraphConfigs(subsuite, arch, graph)
+        BuildGraphConfigs(subsuite, variant_graph, arch)
   parent.AppendChild(graph)
   return graph
 
@@ -605,6 +754,7 @@ class Platform(object):
   def __init__(self, args):
     self.shell_dir = args.shell_dir
     self.shell_dir_secondary = args.shell_dir_secondary
+    self.is_dry_run = args.dry_run
     self.extra_flags = args.extra_flags.split()
     self.args = args
 
@@ -699,11 +849,12 @@ class DesktopPlatform(Platform):
     shell_dir = self.shell_dir_secondary if secondary else self.shell_dir
     cmd = runnable.GetCommand(self.command_prefix, shell_dir, self.extra_flags)
     logging.debug('Running command: %s' % cmd)
-    output = cmd.execute()
+    output = Output() if self.is_dry_run else cmd.execute()
 
     if output.IsSuccess() and '--prof' in self.extra_flags:
       os_prefix = {'linux': 'linux', 'macos': 'mac'}.get(utils.GuessOS())
       if os_prefix:
+        if not self.is_dry_run:
           tick_tools = os.path.join(TOOLS_BASE, '%s-tick-processor' % os_prefix)
           subprocess.check_call(tick_tools + ' --only-summary', shell=True)
       else:  # pragma: no cover
@@ -771,6 +922,7 @@ class AndroidPlatform(Platform):  # pragma: no cover
     output = Output()
     start = time.time()
     try:
+      if not self.is_dry_run:
         output.stdout = self.driver.run(
             target_dir=target_dir,
             binary=runnable.binary,
@@ -950,12 +1102,12 @@ def Main(argv):
                       '"powersave" for more stable results, or "performance" '
                       'for shorter completion time of suite, with potentially '
                       'more noise in results.')
-  parser.add_argument('--filter',
-                      help='Only run the benchmarks beginning with this '
-                      'string. For example: '
+  parser.add_argument(
+      '--filter',
+      help='Only run the benchmarks matching with this '
+      'regex. For example: '
       '--filter=JSTests/TypedArrays/ will run only TypedArray '
-                      'benchmarks from the JSTests suite.',
-                      default='')
+      'benchmarks from the JSTests suite.')
   parser.add_argument('--confidence-level', type=float,
                       help='Repeatedly runs each benchmark until specified '
                       'confidence level is reached. The value is interpreted '
@@ -976,6 +1128,11 @@ def Main(argv):
   parser.add_argument('--run-count', type=int, default=0,
                       help='Override the run count specified by the test '
                       'suite. The default 0 uses the suite\'s config.')
+  parser.add_argument(
+      '--dry-run',
+      default=False,
+      action='store_true',
+      help='Do not run any actual tests.')
   parser.add_argument('-v', '--verbose', default=False, action='store_true',
                       help='Be verbose and print debug output.')
   parser.add_argument('suite', nargs='+', help='Path to the suite config file.')
@@ -1032,6 +1189,13 @@ def Main(argv):
     args.json_test_results_secondary = os.path.abspath(
         args.json_test_results_secondary)
 
+  try:
+    if args.filter:
+      args.filter = re.compile(args.filter)
+  except re.error:
+    logging.error("Invalid regular expression for --filter=%s" % args.filter)
+    return INFRA_FAILURE_RETCODE
+
   # Ensure all arguments have absolute path before we start changing current
   # directory.
   args.suite = list(map(os.path.abspath, args.suite))
@@ -1061,7 +1225,12 @@ def Main(argv):
 
       # Build the graph/trace tree structure.
       default_parent = DefaultSentinel(default_binary_name)
-      root = BuildGraphConfigs(suite, args.arch, default_parent)
+      root = BuildGraphConfigs(suite, default_parent, args.arch)
+
+      if logging.DEBUG >= logging.root.level:
+        logging.debug("Config tree:")
+        for node in iter(root):
+          logging.debug("  %s", node)
 
       # Callback to be called on each node on traversal.
       def NodeCB(node):
@@ -1072,8 +1241,8 @@ def Main(argv):
       try:
         for runnable in FlattenRunnables(root, NodeCB):
           runnable_name = '/'.join(runnable.graphs)
-          if (not runnable_name.startswith(args.filter) and
-              runnable_name + '/' != args.filter):
+          if args.filter and args.filter.search(runnable_name):
+            logging.info('Skipping suite "%s" due to filter', runnable_name)
             continue
           logging.info('>>> Running suite: %s', runnable_name)
 

tools/unittests/run_perf_test.py

@@ -42,6 +42,35 @@ V8_JSON = {
   ]
 }
 
+V8_VARIANTS_JSON = {
+    'path': ['.'],
+    'owners': ['username@chromium.org'],
+    'binary': 'd7',
+    'timeout': 60,
+    'flags': ['--flag'],
+    'main': 'run.js',
+    'run_count': 1,
+    'results_regexp': '%s: (.+)$',
+    'variants': [{
+        'name': 'default',
+        'flags': [],
+    }, {
+        'name': 'VariantA',
+        'flags': ['--variant-a-flag'],
+    }, {
+        'name': 'VariantB',
+        'flags': ['--variant-b-flag'],
+    }],
+    'tests': [
+        {
+            'name': 'Richards',
+        },
+        {
+            'name': 'DeltaBlue',
+        },
+    ]
+}
+
 V8_NESTED_SUITES_JSON = {
   'path': ['.'],
   'owners': ['username@chromium.org'],
@@ -138,14 +167,17 @@ class PerfTest(unittest.TestCase):
     with open(self._test_input, 'w') as f:
       f.write(json.dumps(json_content))
 
-  def _MockCommand(self, *args, **kwargs):
+  def _MockCommand(self, raw_dirs, raw_outputs, *args, **kwargs):
     on_bots = kwargs.pop('on_bots', False)
     # Fake output for each test run.
-    test_outputs = [Output(stdout=arg,
+    test_outputs = [
+        Output(
+            stdout=output,
             timed_out=kwargs.get('timed_out', False),
             exit_code=kwargs.get('exit_code', 0),
-                           duration=42)
-                    for arg in args[1]]
+            duration=42) for output in raw_outputs
+    ]
+
     def create_cmd(*args, **kwargs):
       cmd = mock.MagicMock()
       def execute(*args, **kwargs):
@@ -168,9 +200,16 @@ class PerfTest(unittest.TestCase):
         mock.MagicMock(side_effect=return_values)).start()
 
     # Check that d8 is called from the correct cwd for each test run.
-    dirs = [os.path.join(TEST_WORKSPACE, arg) for arg in args[0]]
-    def chdir(*args, **kwargs):
-      self.assertEqual(dirs.pop(), args[0])
+    dirs = [os.path.join(TEST_WORKSPACE, dir) for dir in raw_dirs]
+
+    def chdir(dir, *args, **kwargs):
+      if not dirs:
+        raise Exception("Missing test chdir '%s'" % dir)
+      expected_dir = dirs.pop()
+      self.assertEqual(
+          expected_dir, dir,
+          "Unexpected chdir: expected='%s' got='%s'" % (expected_dir, dir))
+
     os.chdir = mock.MagicMock(side_effect=chdir)
 
     subprocess.check_call = mock.MagicMock()
@@ -190,6 +229,12 @@ class PerfTest(unittest.TestCase):
     with open(file_name or self._test_output) as f:
       return json.load(f)
 
+  def _VerifyResultTraces(self, traces, file_name=None):
+    sorted_expected = sorted(traces, key=SORT_KEY)
+    sorted_results = sorted(
+        self._LoadResults(file_name)['traces'], key=SORT_KEY)
+    self.assertListEqual(sorted_expected, sorted_results)
+
   def _VerifyResults(self, suite, units, traces, file_name=None):
     self.assertListEqual(sorted([
       {'units': units,
@@ -244,6 +289,60 @@ class PerfTest(unittest.TestCase):
     self._VerifyMock(
         os.path.join('out', 'x64.release', 'd7'), '--flag', 'run.js')
 
+  def testOneRunVariants(self):
+    self._WriteTestInput(V8_VARIANTS_JSON)
+    self._MockCommand(['.', '.', '.'], [
+        'x\nRichards: 3.3\nDeltaBlue: 3000\ny\n',
+        'x\nRichards: 2.2\nDeltaBlue: 2000\ny\n',
+        'x\nRichards: 1.1\nDeltaBlue: 1000\ny\n'
+    ])
+    self.assertEqual(0, self._CallMain())
+    self._VerifyResultTraces([
+        {
+            'units': 'score',
+            'graphs': ['test', 'default', 'Richards'],
+            'results': [1.1],
+            'stddev': ''
+        },
+        {
+            'units': 'score',
+            'graphs': ['test', 'default', 'DeltaBlue'],
+            'results': [1000],
+            'stddev': ''
+        },
+        {
+            'units': 'score',
+            'graphs': ['test', 'VariantA', 'Richards'],
+            'results': [2.2],
+            'stddev': ''
+        },
+        {
+            'units': 'score',
+            'graphs': ['test', 'VariantA', 'DeltaBlue'],
+            'results': [2000],
+            'stddev': ''
+        },
+        {
+            'units': 'score',
+            'graphs': ['test', 'VariantB', 'Richards'],
+            'results': [3.3],
+            'stddev': ''
+        },
+        {
+            'units': 'score',
+            'graphs': ['test', 'VariantB', 'DeltaBlue'],
+            'results': [3000],
+            'stddev': ''
+        },
+    ])
+    self._VerifyErrors([])
+    self._VerifyMockMultiple(
+        (os.path.join('out', 'x64.release', 'd7'), '--flag', 'run.js'),
+        (os.path.join('out', 'x64.release',
+                      'd7'), '--flag', '--variant-a-flag', 'run.js'),
+        (os.path.join('out', 'x64.release',
+                      'd7'), '--flag', '--variant-b-flag', 'run.js'))
+
   def testOneRunWithTestFlags(self):
     test_input = dict(V8_JSON)
     test_input['test_flags'] = ['2', 'test_name']
@@ -378,6 +477,7 @@ class PerfTest(unittest.TestCase):
         (os.path.join('out', 'x64.release', 'd8'),
          '--flag', '--flag2', 'run.js'))
 
+
   def testOneRunStdDevRegExp(self):
     test_input = dict(V8_JSON)
     test_input['stddev_regexp'] = r'^%s-stddev: (.+)$'