skpbench: simplify adb and reduce number of invocations

This change reduces a bit of startup overhead by condensing more bash
into fewer invocations of 'adb shell'.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2481413003

Review-Url: https://codereview.chromium.org/2481413003

6 files changed, 106 insertions, 108 deletions

diff --git a/tools/skpbench/_adb.py b/tools/skpbench/_adb.py
index 402e132a2b..75c0173034 100644
--- a/tools/skpbench/_adb.py
+++ b/tools/skpbench/_adb.py
@@ -3,40 +3,50 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+from __future__ import print_function
 import re
 import subprocess
 import sys
 
 class Adb:
-  def __init__(self, device_serial=None):
+  def __init__(self, device_serial=None, echofile=None):
     self.__invocation = ['adb']
     if device_serial:
       self.__invocation.extend(['-s', device_serial])
+    self.__echofile = echofile
+    self.__is_root = None
 
   def shell(self, cmd):
+    if self.__echofile:
+      self.__echo_cmd(cmd)
     subprocess.call(self.__invocation + ['shell', cmd], stdout=sys.stderr)
 
   def check(self, cmd):
+    if self.__echofile:
+      self.__echo_cmd(cmd)
     result = subprocess.check_output(self.__invocation + ['shell', cmd])
-    return result.rstrip()
-
-  def check_lines(self, cmd):
-    result = self.check(cmd)
-    return re.split('[\r\n]+', result)
-
-  def get_device_model(self):
-    result = self.check('getprop | grep ro.product.model')
-    result = re.match(r'\[ro.product.model\]:\s*\[(.*)\]', result)
-    return result.group(1) if result else 'unknown_product'
+    if self.__echofile:
+      print(result, file=self.__echofile)
+    return result
+
+  def root(self):
+    if not self.is_root():
+      subprocess.call(self.__invocation + ['root'], stdout=sys.stderr)
+      self.__is_root = None
+    return self.is_root()
 
   def is_root(self):
-    return self.check('whoami') == 'root'
-
-  def attempt_root(self):
-    if self.is_root():
-      return True
-    subprocess.call(self.__invocation + ['root'], stdout=sys.stderr)
-    return self.is_root()
+    if self.__is_root is None:
+      self.__is_root = ('root' == self.check('whoami').strip())
+    return self.__is_root
 
   def remount(self):
     subprocess.call(self.__invocation + ['remount'], stdout=sys.stderr)
+
+  def __echo_cmd(self, cmd):
+    escaped = [re.sub(r'([^a-zA-Z0-9])', r'\\\1', x)
+               for x in cmd.strip().splitlines()]
+    subprocess.call(self.__invocation + \
+                    ['shell', 'echo', '$(whoami)@$(getprop ro.serialno)$',
+                     " '\n>' ".join(escaped)],
+                    stdout=self.__echofile)
diff --git a/tools/skpbench/_adb_path.py b/tools/skpbench/_adb_path.py
index 72382bc1dc..4d8bce4425 100644
--- a/tools/skpbench/_adb_path.py
+++ b/tools/skpbench/_adb_path.py
@@ -22,11 +22,11 @@ def basename(pathname):
 def find_skps(skps):
   escapedskps = [re.sub(r'([^a-zA-Z0-9_/\.\*\?\[\!\]])', r'\\\1', x)
                  for x in skps]
-  return __ADB.check_lines('''\
+  return __ADB.check('''\
     for PATHNAME in %s; do
       if [ -d "$PATHNAME" ]; then
         find "$PATHNAME" -maxdepth 1 -name *.skp
       else
         echo "$PATHNAME"
       fi
-    done''' % ' '.join(escapedskps))
+    done''' % ' '.join(escapedskps)).splitlines()
diff --git a/tools/skpbench/_hardware_android.py b/tools/skpbench/_hardware_android.py
index 68c8522e3e..ebaba0ab6d 100644
--- a/tools/skpbench/_hardware_android.py
+++ b/tools/skpbench/_hardware_android.py
@@ -13,48 +13,42 @@ class HardwareAndroid(Hardware):
     Hardware.__init__(self)
     self.warmup_time = 5
     self._adb = adb
-    self._is_root = self._adb.attempt_root()
-    if self._is_root:
-      self._adb.remount()
-    self._initial_airplane_mode = None
-    self._initial_location_providers = None
-    self._initial_ASLR = None
 
-  def __enter__(self):
-    # turn on airplane mode.
-    self._initial_airplane_mode = \
-      self._adb.check('settings get global airplane_mode_on')
-    self._adb.shell('settings put global airplane_mode_on 1')
-
-    # disable GPS.
-    self._initial_location_providers = \
-      self._adb.check('settings get secure location_providers_allowed')
-    self._initial_location_providers = \
-      self._initial_location_providers.replace(',', ' ')
-    self._adb.shell('''\
-      for PROVIDER in %s; do
-        settings put secure location_providers_allowed -$PROVIDER
-      done''' % self._initial_location_providers)
-
-    if self._is_root:
-      # disable bluetooth, wifi, and mobile data.
-      # TODO: can we query these initial values?
-      self._adb.shell('''\
-        service call bluetooth_manager 8 &&
-        svc wifi disable &&
-        svc data disable''')
-
-      # kill the gui.
-      self._adb.shell('''\
-        setprop ctl.stop media &&
-        setprop ctl.stop zygote &&
-        setprop ctl.stop surfaceflinger &&
-        setprop ctl.stop drm''')
-
-      # disable ASLR.
+    if self._adb.root():
+      self._adb.remount()
       self._initial_ASLR = \
         self._adb.check('cat /proc/sys/kernel/randomize_va_space')
-      self._adb.shell('echo 0 > /proc/sys/kernel/randomize_va_space')
+
+  def __enter__(self):
+    self._adb.shell('\n'.join([
+      # turn on airplane mode.
+      '''
+      settings put global airplane_mode_on 1''',
+
+      # disable GPS.
+      '''
+      for MODE in gps wifi network; do
+        settings put secure location_providers_allowed -$MODE
+      done''']))
+
+    if self._adb.is_root():
+      self._adb.shell('\n'.join([
+        # disable bluetooth, wifi, and mobile data.
+        '''
+        service call bluetooth_manager 8
+        svc wifi disable
+        svc data disable''',
+
+        # kill the gui.
+        '''
+        setprop ctl.stop media
+        setprop ctl.stop zygote
+        setprop ctl.stop surfaceflinger
+        setprop ctl.stop drm''',
+
+        # disable ASLR
+        '''
+        echo 0 > /proc/sys/kernel/randomize_va_space''']))
     else:
       print("WARNING: no adb root access; results may be unreliable.",
             file=sys.stderr)
@@ -64,27 +58,18 @@ class HardwareAndroid(Hardware):
   def __exit__(self, exception_type, exception_value, traceback):
     Hardware.__exit__(self, exception_type, exception_value, traceback)
 
-    if self._is_root:
-      # restore ASLR.
-      self._adb.shell('echo %s > /proc/sys/kernel/randomize_va_space' %
-                      self._initial_ASLR)
-
-      # revive the gui.
-      self._adb.shell('''\
-        setprop ctl.start drm &&
-        setprop ctl.start surfaceflinger &&
-        setprop ctl.start zygote &&
-        setprop ctl.start media''')
-    else:
-      # restore GPS (doesn't seem to work if we killed the gui).
-      self._adb.shell('''\
-        for PROVIDER in %s; do
-          settings put secure location_providers_allowed +$PROVIDER
-        done''' % self._initial_location_providers)
-
-      # restore airplane mode (doesn't seem to work if we killed the gui).
-      self._adb.shell('settings put global airplane_mode_on %s' %
-                      self._initial_airplane_mode)
+    if self._adb.is_root():
+      self._adb.shell('\n'.join([
+        # restore ASLR.
+        '''
+        echo %s > /proc/sys/kernel/randomize_va_space''' % self._initial_ASLR,
+
+        # revive the gui.
+        '''
+        setprop ctl.start drm
+        setprop ctl.start surfaceflinger
+        setprop ctl.start zygote
+        setprop ctl.start media''']))
 
   def sanity_check(self):
     Hardware.sanity_check(self)
diff --git a/tools/skpbench/_hardware_nexus_6p.py b/tools/skpbench/_hardware_nexus_6p.py
index 7e9bb5545a..077933bb41 100644
--- a/tools/skpbench/_hardware_nexus_6p.py
+++ b/tools/skpbench/_hardware_nexus_6p.py
@@ -23,7 +23,7 @@ class HardwareNexus6P(HardwareAndroid):
     self._unlock_clocks()
 
   def _lock_clocks(self):
-    if not self._is_root:
+    if not self._adb.is_root():
       return
 
     self._adb.shell('''\
@@ -73,7 +73,7 @@ class HardwareNexus6P(HardwareAndroid):
       echo 9887 > /sys/class/devfreq/qcom,gpubw.70/min_freq''')
 
   def _unlock_clocks(self):
-    if not self._is_root:
+    if not self._adb.is_root():
       return
 
     # restore ddr settings to default.
@@ -121,10 +121,10 @@ class HardwareNexus6P(HardwareAndroid):
   def sanity_check(self):
     HardwareAndroid.sanity_check(self)
 
-    if not self._is_root:
+    if not self._adb.is_root():
       return
 
-    result = self._adb.check_lines('''\
+    result = self._adb.check('''\
       cat /sys/class/power_supply/battery/capacity \
           /sys/devices/system/cpu/online \
           /sys/class/thermal/thermal_zone14/temp \
@@ -147,7 +147,7 @@ class HardwareNexus6P(HardwareAndroid):
       [Expectation(int, exact_value=CPU_CLOCK_RATE, name='cpu_%i clock rate' %i)
        for i in range(4, 7)]
 
-    Expectation.check_all(expectations, result)
+    Expectation.check_all(expectations, result.splitlines())
 
   def sleep(self, sleeptime):
     self._unlock_clocks()
diff --git a/tools/skpbench/_hardware_pixel_c.py b/tools/skpbench/_hardware_pixel_c.py
index 3ea74c1c03..842ee0c502 100644
--- a/tools/skpbench/_hardware_pixel_c.py
+++ b/tools/skpbench/_hardware_pixel_c.py
@@ -24,45 +24,47 @@ class HardwarePixelC(HardwareAndroid):
     self._unlock_clocks()
 
   def _lock_clocks(self):
-    if not self._is_root:
+    if not self._adb.is_root():
       return
 
-    # lock cpu clocks.
-    self._adb.shell('''\
+    self._adb.shell('\n'.join([
+      # lock cpu clocks.
+      '''
       for N in $(seq 0 3); do
         echo userspace > /sys/devices/system/cpu/cpu$N/cpufreq/scaling_governor
         echo %i > /sys/devices/system/cpu/cpu$N/cpufreq/scaling_setspeed
-      done''' % CPU_CLOCK_RATE)
+      done''' % CPU_CLOCK_RATE,
 
-    # lock gpu/emc clocks.
-    self._adb.shell('''\
+      # lock gpu/emc clocks.
+      '''
       chown root:root /sys/devices/57000000.gpu/pstate
-      echo %s > /sys/devices/57000000.gpu/pstate''' % GPU_EMC_PROFILE_ID)
+      echo %s > /sys/devices/57000000.gpu/pstate''' % GPU_EMC_PROFILE_ID]))
 
   def _unlock_clocks(self):
-    if not self._is_root:
+    if not self._adb.is_root():
       return
 
-    # unlock gpu/emc clocks.
-    self._adb.shell('''\
+    self._adb.shell('\n'.join([
+      # unlock gpu/emc clocks.
+      '''
       echo auto > /sys/devices/57000000.gpu/pstate
-      chown system:system /sys/devices/57000000.gpu/pstate''')
+      chown system:system /sys/devices/57000000.gpu/pstate''',
 
-    # unlock cpu clocks.
-    self._adb.shell('''\
+      # unlock cpu clocks.
+      '''
       for N in $(seq 0 3); do
         echo 0 > /sys/devices/system/cpu/cpu$N/cpufreq/scaling_setspeed
-        echo interactive > /sys/devices/system/cpu/cpu$N/cpufreq/scaling_governor
-      done''')
+        echo interactive >/sys/devices/system/cpu/cpu$N/cpufreq/scaling_governor
+      done''']))
 
   def sanity_check(self):
     HardwareAndroid.sanity_check(self)
 
-    if not self._is_root:
+    if not self._adb.is_root():
       return
 
     # only issue one shell command in an attempt to minimize interference.
-    result = self._adb.check_lines('''\
+    result = self._adb.check('''\
       cat /sys/class/power_supply/bq27742-0/capacity \
           /sys/class/thermal/thermal_zone7/temp \
           /sys/class/thermal/thermal_zone0/temp \
@@ -86,7 +88,7 @@ class HardwarePixelC(HardwareAndroid):
        for i in range(4)] + \
       [Expectation(str, exact_value=GPU_EMC_PROFILE, name='gpu/emc profile')]
 
-    Expectation.check_all(expectations, result)
+    Expectation.check_all(expectations, result.splitlines())
 
   def sleep(self, sleeptime):
     self._unlock_clocks()
diff --git a/tools/skpbench/skpbench.py b/tools/skpbench/skpbench.py
index 3e5b45138f..fc5082b94e 100755
--- a/tools/skpbench/skpbench.py
+++ b/tools/skpbench/skpbench.py
@@ -68,7 +68,7 @@ else:
   import _os_path as _path
 
 def dump_commandline_if_verbose(commandline):
-  if FLAGS.verbosity >= 4:
+  if FLAGS.verbosity >= 5:
     quoted = ['\'%s\'' % re.sub(r'([\\\'])', r'\\\1', x) for x in commandline]
     print(' '.join(quoted), file=sys.stderr)
 
@@ -253,14 +253,14 @@ def run_benchmarks(configs, skps, hardware):
 
       except HardwareException as exception:
         skpbench.terminate()
-        if FLAGS.verbosity >= 5:
+        if FLAGS.verbosity >= 4:
           hardware.print_debug_diagnostics()
         if FLAGS.verbosity >= 1:
           print("%s; taking a %i second nap..." %
                 (exception.message, exception.sleeptime), file=sys.stderr)
         benches.appendleft(benchargs) # retry the same bench next time.
         hardware.sleep(exception.sleeptime)
-        if FLAGS.verbosity >= 5:
+        if FLAGS.verbosity >= 4:
           hardware.print_debug_diagnostics()
         SKPBench.run_warmup(hardware.warmup_time)
 
@@ -272,8 +272,9 @@ def main():
   skps = _path.find_skps(FLAGS.skps)
 
   if FLAGS.adb:
-    adb = Adb(FLAGS.device_serial)
-    model = adb.get_device_model()
+    adb = Adb(FLAGS.device_serial,
+              echofile=(sys.stderr if FLAGS.verbosity >= 5 else None))
+    model = adb.check('getprop ro.product.model').strip()
     if model == 'Pixel C':
       from _hardware_pixel_c import HardwarePixelC
       hardware = HardwarePixelC(adb)