Fix bisect script to match a results with {mean, stddev} format.

tools/bisect-perf-regression.py

 #!/usr/bin/env python
 # Copyright (c) 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Performance Test Bisect Tool
 
 This script bisects a series of changelists using binary search. It starts at
 a bad revision where a performance metric has regressed, and asks for a last
 known-good revision. It will then binary search across this revision range by
@@ skipping 1935 matching lines @@
     """
     # Format is: RESULT <graph>: <trace>= <value> <units>
     metric_formatted = re.escape('RESULT %s: %s=' % (metric[0], metric[1]))
 
     text_lines = text.split('\n')
     values_list = []
 
     for current_line in text_lines:
       # Parse the output from the performance test for the metric we're
       # interested in.
+      # The log will be parsed looking for format:
+      # <*>RESULT <graph_name>: <trace_name>= <value>
       metric_re = metric_formatted +\
                   "(\s)*(?P<values>[0-9]+(\.[0-9]*)?)"
       metric_re = re.compile(metric_re)
       regex_results = metric_re.search(current_line)
 
       if not regex_results is None:
         values_list += [regex_results.group('values')]
       else:
+        # The log will be parsed looking for format:
+        # <*>RESULT <graph_name>: <trace_name>= [<value>,value,value,...]
         metric_re = metric_formatted +\
                     "(\s)*\[(\s)*(?P<values>[0-9,.]+)\]"
         metric_re = re.compile(metric_re)
         regex_results = metric_re.search(current_line)
 
         if not regex_results is None:
           metric_values = regex_results.group('values')
 
           values_list += metric_values.split(',')
+        else:
+          # The log will be parsed looking for format:
+          # <*>RESULT <graph_name>: <trace_name>= {<mean>, <std deviation>}

[qyearsley, 2014/05/14 22:28:33]

It would probably be neater/more readable to put these three regular expressions
up at the top (before the loop), and avoid nesting the else expressions. For
example:

single_result_re = metric_re + '\s*(?P<VALUE>[0-9]+(\.[0-9]*)?)'
multi_results_re = metric_re + '\s*\[\s*(?P<VALUES>[0-9,.]+)\]'
mean_stddev_re = (
    metric_re +
    '\s*\{\s*(?P<MEAN>[0-9]+(\.[0-9]*)?),\s*' +
    '(?P<STDDEV>[0-9]+(\.[0-9]*)?)\}')

single_result_match = re.search(single_result_re, current_line)
multi_results_match = re.search(multi_results_re, current_line)
mean_stddev_match = re.search(mean_stddev_re, current_line)

if single_result_match:
  # ...
elif multi_results_match:
  # ...
elif mean_stddev_match:
  # ...
else:
  # ...

Looking more closely at these regular expressions, I have a few questions:
 (1) For single result lines and square-bracket value list result lines, are we
not allowing negative numbers? Most tests don't output negative numbers but some
might and I think it's valid.
 (2) It looks like we're allowing whitespace after the opening bracket but NOT
before the closing bracket. Is there a reason for this? i.e. we're allowing [
1,2] but not [1,2 ]. (Might as well allow space before the closing bracket as
well).
 (3) The second regex is a too accepting. It would accept:
     [123.123.123,.3..3] or [.]
     (But it would not accept: [1, 2, 3] or [-1,2].

[prasadv, 2014/05/15 19:49:09]

Agreed, tried to trim this method and make it more readable.
Also added tests for each of the results patterns.

On 2014/05/14 22:28:33, qyearsley wrote:
Done.

+          metric_re = metric_formatted +\
+                      "(\s)*\{(\s)*(?P<values>[-\d\., ]+)\}"
+          metric_re = re.compile(metric_re)
+          regex_results = metric_re.search(current_line)
+          if not regex_results is None:
+            metric_values = regex_results.group('values')
+            if metric_values:
+              # Here we are interested only in mean.
+              values_list += [metric_values.split(',')[0]]
 
     values_list = [float(v) for v in values_list if IsStringFloat(v)]
 
     # If the metric is times/t, we need to sum the timings in order to get
     # similar regression results as the try-bots.
     metrics_to_sum = [['times', 't'], ['times', 'page_load_time'],
         ['cold_times', 'page_load_time'], ['warm_times', 'page_load_time']]
 
     if metric in metrics_to_sum:
       if values_list:
@@ skipping 1910 matching lines @@
       # The perf dashboard scrapes the "results" step in order to comment on
       # bugs. If you change this, please update the perf dashboard as well.
       bisect_utils.OutputAnnotationStepStart('Results')
     print 'Error: %s' % e.message
     if opts.output_buildbot_annotations:
       bisect_utils.OutputAnnotationStepClosed()
   return 1
 
 if __name__ == '__main__':
   sys.exit(main())