Fix last_devices to be quieter, and improve device affinity.

build/android/pylib/perf/test_runner.py

 # Copyright 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.
 
 """Runs perf tests.
 
 Our buildbot infrastructure requires each slave to run steps serially.
 This is sub-optimal for android, where these steps can run independently on
 multiple connected devices.
 
@@ skipping 47 matching lines @@
 import threading
 import time
 
 from pylib import cmd_helper
 from pylib import constants
 from pylib import forwarder
 from pylib.base import base_test_result
 from pylib.base import base_test_runner
 from pylib.device import device_errors
 
+NUM_DEVICE_AFFINITIES = 8

[jbudorick, 2015/05/23 01:06:49]

What if the number of devices is higher than 8? I think the speed team has
designs on something like that...

[luqui, 2015/05/27 20:01:12]

As far as I could tell there are no specs of device_affinity greater than 8. 
Maybe you could help me look harder (not sure where else to look)

[shatch, 2015/05/28 13:59:17]

This isn't a thing yet, but we did ask for some investigation into upping device
limits: crbug.com/466105, so it's something to keep in mind. You might be able
to derive this instead of hardcoding it by looking at max(test affinities).

[jbudorick, 2015/05/28 14:03:16]

Currently there aren't, and I don't think we have any bot deployments with >8
devices. However, as I said, the speed team is at least considering doing so,
and this doesn't handle that case very well.

+
 
 def OutputJsonList(json_input, json_output):
   with file(json_input, 'r') as i:
     all_steps = json.load(i)
   step_names = all_steps['steps'].keys()
   with file(json_output, 'w') as o:
     o.write(json.dumps(step_names))
   return 0
 
 
@@ skipping 72 matching lines @@
     if now - self._tick >= _HeartBeatLogger._PRINT_INTERVAL:
       self._tick = now
       print '--single-step output length %d' % self._len
       sys.stdout.flush()
 
   def stop(self):
     self._stopped.set()
 
 
 class TestRunner(base_test_runner.BaseTestRunner):
-  def __init__(self, test_options, device, shard_index, max_shard, tests,
-      flaky_tests):
+  def __init__(self, test_options, device, affinities, tests, flaky_tests):
     """A TestRunner instance runs a perf test on a single device.
 
     Args:
       test_options: A PerfOptions object.
       device: Device to run the tests.
-      shard_index: the index of this device.
-      max_shards: the maximum shard index.
+      affinities: the list of affinities to run on this shard.
       tests: a dict mapping test_name to command.
       flaky_tests: a list of flaky test_name.
     """
     super(TestRunner, self).__init__(device, None, 'Release')
     self._options = test_options
-    self._shard_index = shard_index
-    self._max_shard = max_shard
+    self._affinities = affinities
     self._tests = tests
     self._flaky_tests = flaky_tests
     self._output_dir = None
 
   @staticmethod
   def _IsBetter(result):
     if result['actual_exit_code'] == 0:
       return True
     pickled = os.path.join(constants.PERF_OUTPUT_DIR,
                            result['name'])
     if not os.path.exists(pickled):
       return True
     with file(pickled, 'r') as f:
       previous = pickle.loads(f.read())
     return result['actual_exit_code'] < previous['actual_exit_code']
 
   @staticmethod
   def _SaveResult(result):
     if TestRunner._IsBetter(result):
       with file(os.path.join(constants.PERF_OUTPUT_DIR,
                              result['name']), 'w') as f:
         f.write(pickle.dumps(result))
 
   def _CheckDeviceAffinity(self, test_name):
     """Returns True if test_name has affinity for this shard."""
-    affinity = (self._tests['steps'][test_name]['device_affinity'] %
-                self._max_shard)
-    if self._shard_index == affinity:
+    affinity = self._tests['steps'][test_name]['device_affinity']
+    assert 0 <= affinity and affinity < NUM_DEVICE_AFFINITIES, (

[jbudorick, 2015/05/23 01:06:49]

This should be an exception. We generally avoid assertions in the test runner,
especially on data we're reading from somewhere else (e.g. the perf test list).

[luqui, 2015/05/27 20:01:12]

Done.

+        'Got out-of-range device affinity %s' % affinity)
+    if affinity in self._affinities:
       return True
-    logging.info('Skipping %s on %s (affinity is %s, device is %s)',
-                 test_name, self.device_serial, affinity, self._shard_index)
+    logging.info(
+        'Skipping %s on %s (affinity is %s, allowed affinities are %s)',
+                 test_name, self.device_serial, affinity, self._affinities)
     return False
 
   def _CleanupOutputDirectory(self):
     if self._output_dir:
       shutil.rmtree(self._output_dir, ignore_errors=True)
       self._output_dir = None
 
   def _ReadChartjsonOutput(self):
     if not self._output_dir:
       return ''
@@ skipping 110 matching lines @@
     Returns:
       A tuple of (TestRunResults, retry).
     """
     _, result_type = self._LaunchPerfTest(test_name)
     results = base_test_result.TestRunResults()
     results.AddResult(base_test_result.BaseTestResult(test_name, result_type))
     retry = None
     if not results.DidRunPass():
       retry = test_name
     return results, retry