Cache device apk checksums in device_utils.py

build/android/pylib/device/device_utils.py

 # Copyright 2014 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.
 
 """Provides a variety of device interactions based on adb.
 
 Eventually, this will be based on adb_wrapper.
 """
 # pylint: disable=unused-argument
 
@@ skipping 158 matching lines @@
     elif isinstance(device, pylib.android_commands.AndroidCommands):
       self.adb = adb_wrapper.AdbWrapper(device.GetDevice())
       self.old_interface = device
     else:
       raise ValueError('Unsupported device value: %r' % device)
     self._commands_installed = None
     self._default_timeout = default_timeout
     self._default_retries = default_retries
     self._cache = {}
     self._client_caches = {}
+    self._package_to_device_apk_checksums_cache = {}

[jbudorick, 2015/07/16 16:31:33]

This should use self._cache, which we already have some lifetime control for,
rather than using its own dict -- something like
self._cache['package_checksums'] = {} and self._cache['package_paths'] = {}

[agrieve, 2015/07/16 19:01:20]

I think these caches are better split out since their freshness are tied to
package installs rather than the device itself.

+    self._package_to_device_apk_paths_cache = {}
     assert hasattr(self, decorators.DEFAULT_TIMEOUT_ATTR)
     assert hasattr(self, decorators.DEFAULT_RETRIES_ATTR)
 
   def __eq__(self, other):
     """Checks whether |other| refers to the same device as |self|.
 
     Args:
       other: The object to compare to. This can be a basestring, an instance
         of adb_wrapper.AdbWrapper, or an instance of DeviceUtils.
     Returns:
@@ skipping 147 matching lines @@
     value = self.RunShellCommand('echo $EXTERNAL_STORAGE',
                                  single_line=True,
                                  check_return=True)
     if not value:
       raise device_errors.CommandFailedError('$EXTERNAL_STORAGE is not set',
                                              str(self))
     self._cache['external_storage'] = value
     return value
 
   @decorators.WithTimeoutAndRetriesFromInstance()
-  def GetApplicationPaths(self, package, timeout=None, retries=None):
+  def GetApplicationPaths(self, package, timeout=None, retries=None,
+                          skip_cache=False):

[jbudorick, 2015/07/16 16:31:33]

Do we envision external clients using this, or does this option exist solely for
the pm_ready check? If the latter, perhaps we should expose this option in a
private method that gets called by the public one.

If this sticks around, it needs to be:
 - before timeout and retries
 - documented

[agrieve, 2015/07/16 19:01:21]

Made it an internal helper. 

     """Get the paths of the installed apks on the device for the given package.
 
     Args:
       package: Name of the package.
 
     Returns:
       List of paths to the apks on the device for the given package.
     """
+    cached_result = self._package_to_device_apk_paths_cache.get(package)
+    if cached_result is not None and not skip_cache:
+      return list(cached_result)
     # 'pm path' is liable to incorrectly exit with a nonzero number starting
     # in Lollipop.
     # TODO(jbudorick): Check if this is fixed as new Android versions are
     # released to put an upper bound on this.
     should_check_return = (self.build_version_sdk <
                            constants.ANDROID_SDK_VERSION_CODES.LOLLIPOP)
     output = self.RunShellCommand(
         ['pm', 'path', package], check_return=should_check_return)
     apks = []
     for line in output:
       if not line.startswith('package:'):
         raise device_errors.CommandFailedError(
             'pm path returned: %r' % '\n'.join(output), str(self))
       apks.append(line[len('package:'):])
+    self._package_to_device_apk_paths_cache[package] = list(apks)
     return apks
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def GetApplicationDataDirectory(self, package, timeout=None, retries=None):
     """Get the data directory on the device for the given package.
 
     Args:
       package: Name of the package.
 
     Returns:
@@ skipping 32 matching lines @@
     def sd_card_ready():
       try:
         self.RunShellCommand(['test', '-d', self.GetExternalStoragePath()],
                              check_return=True)
         return True
       except device_errors.AdbCommandFailedError:
         return False
 
     def pm_ready():
       try:
-        return self.GetApplicationPaths('android')
+        return self.GetApplicationPaths('android', skip_cache=True)
       except device_errors.CommandFailedError:
         return False
 
     def boot_completed():
       return self.GetProp('sys.boot_completed') == '1'
 
     def wifi_enabled():
       return 'Wi-Fi is enabled' in self.RunShellCommand(['dumpsys', 'wifi'],
                                                         check_return=False)
 
@@ skipping 55 matching lines @@
       CommandTimeoutError if the installation times out.
       DeviceUnreachableError on missing device.
     """
     package_name = apk_helper.GetPackageName(apk_path)
     device_paths = self.GetApplicationPaths(package_name)
     if device_paths:
       if len(device_paths) > 1:
         logging.warning(
             'Installing single APK (%s) when split APKs (%s) are currently '
             'installed.', apk_path, ' '.join(device_paths))
-      (files_to_push, _) = self._GetChangedAndStaleFiles(
-          apk_path, device_paths[0])
-      should_install = bool(files_to_push)
+      apks_to_install, host_checksums = (
+          self._ComputeStaleApks(package_name, [apk_path]))
+      should_install = bool(apks_to_install)
       if should_install and not reinstall:
-        self.adb.Uninstall(package_name)
+        self._Uninstall(package_name)
     else:
       should_install = True
+      host_checksums = None
     if should_install:
-      self.adb.Install(apk_path, reinstall=reinstall)
+      # We won't know the resulting device apk names.
+      self._package_to_device_apk_paths_cache.pop(package_name, 0)
+      try:
+        self.adb.Install(apk_path, reinstall=reinstall)
+        self._package_to_device_apk_checksums_cache[package_name] = (
+            host_checksums)
+      except:
+        self._package_to_device_apk_paths_cache.pop(package_name, 0)
+        raise
 
   @decorators.WithTimeoutAndRetriesDefaults(
       INSTALL_DEFAULT_TIMEOUT,
       INSTALL_DEFAULT_RETRIES)
   def InstallSplitApk(self, base_apk, split_apks, reinstall=False,
                       timeout=None, retries=None):
     """Install a split APK.
 
     Noop if all of the APK splits are already installed.
 
@@ skipping 12 matching lines @@
     """
     self._CheckSdkLevel(constants.ANDROID_SDK_VERSION_CODES.LOLLIPOP)
 
     all_apks = [base_apk] + split_select.SelectSplits(
         self, base_apk, split_apks)
     package_name = apk_helper.GetPackageName(base_apk)
     device_apk_paths = self.GetApplicationPaths(package_name)
 
     if device_apk_paths:
       partial_install_package = package_name
-      device_checksums = md5sum.CalculateDeviceMd5Sums(device_apk_paths, self)
-      host_checksums = md5sum.CalculateHostMd5Sums(all_apks)
-      apks_to_install = [k for (k, v) in host_checksums.iteritems()
-                         if v not in device_checksums.values()]
+      apks_to_install, host_checksums = (
+          self._ComputeStaleApks(package_name, all_apks))
       if apks_to_install and not reinstall:
-        self.adb.Uninstall(package_name)
+        self._Uninstall(package_name)
         partial_install_package = None
         apks_to_install = all_apks
     else:
       partial_install_package = None
       apks_to_install = all_apks
+      host_checksums = None
     if apks_to_install:
-      self.adb.InstallMultiple(
-          apks_to_install, partial=partial_install_package, reinstall=reinstall)
+      # We won't know the resulting device apk names.
+      self._package_to_device_apk_paths_cache.pop(package_name, 0)
+      try:
+        self.adb.InstallMultiple(
+            apks_to_install, partial=partial_install_package,
+            reinstall=reinstall)
+        self._package_to_device_apk_checksums_cache[package_name] = (
+            host_checksums)
+      except:
+        self._package_to_device_apk_paths_cache.pop(package_name, 0)
+        raise
+
+  def _Uninstall(self, package_name):

[jbudorick, 2015/07/16 16:31:32]

This should _not_ be named "Uninstall". As-is, it's liable to confuse a user who
looks in here and thinks "why is this private?" instead of "oh, uninstall is
provided by AdbWrapper"

[agrieve, 2015/07/16 19:01:21]

Deleted in favour of hooking self.adb.Uninstall

+    try:
+      self.adb.Uninstall(package_name)
+    except:
+      # Clear cache since we can't be sure of the state.
+      self._package_to_device_apk_paths_cache.pop(package_name, 0)
+      self._package_to_device_apk_checksums_cache.pop(package_name, 0)
+      raise
+    self._package_to_device_apk_paths_cache[package_name] = []
+    self._package_to_device_apk_checksums_cache[package_name] = set()
 
   def _CheckSdkLevel(self, required_sdk_level):
     """Raises an exception if the device does not have the required SDK level.
     """
     if self.build_version_sdk < required_sdk_level:
       raise device_errors.DeviceVersionError(
           ('Requires SDK level %s, device is SDK level %s' %
            (required_sdk_level, self.build_version_sdk)),
            device_serial=self.adb.GetDeviceSerial())
 
@@ skipping 448 matching lines @@
       to_push = []
       for host_abs_path, host_checksum in host_checksums.iteritems():
         device_abs_path = '%s/%s' % (
             real_device_path, os.path.relpath(host_abs_path, real_host_path))
         device_checksum = device_checksums.pop(device_abs_path, None)
         if device_checksum != host_checksum:
           to_push.append((host_abs_path, device_abs_path))
       to_delete = device_checksums.keys()
       return (to_push, to_delete)
 
+  def _ComputeDeviceChecksumsForApks(self, package_name):
+    ret = self._package_to_device_apk_checksums_cache.get(package_name)
+    if ret is None:
+      device_paths = self.GetApplicationPaths(package_name)
+      file_to_checksums = md5sum.CalculateDeviceMd5Sums(device_paths, self)
+      ret = set(file_to_checksums.values())
+      self._package_to_device_apk_checksums_cache[package_name] = ret
+    return ret
+
+  def _ComputeStaleApks(self, package_name, host_apk_paths):
+    host_checksums = md5sum.CalculateHostMd5Sums(host_apk_paths)
+    device_checksums = self._ComputeDeviceChecksumsForApks(package_name)
+    stale_apks = [k for (k, v) in host_checksums.iteritems()
+                  if v not in device_checksums]
+    return stale_apks, set(host_checksums.values())
+
   def _PushFilesImpl(self, host_device_tuples, files):
     size = sum(host_utils.GetRecursiveDiskUsage(h) for h, _ in files)
     file_count = len(files)
     dir_size = sum(host_utils.GetRecursiveDiskUsage(h)
                    for h, _ in host_device_tuples)
     dir_file_count = 0
     for h, _ in host_device_tuples:
       if os.path.isdir(h):
         dir_file_count += sum(len(f) for _r, _d, f in os.walk(h))
       else:
@@ skipping 675 matching lines @@
     """Returns client cache."""
     if client_name not in self._client_caches:
       self._client_caches[client_name] = {}
     return self._client_caches[client_name]
 
   def _ClearCache(self):
     """Clears all caches."""
     for client in self._client_caches:
       self._client_caches[client].clear()
     self._cache.clear()
+    self._package_to_device_apk_paths_cache.clear()
+    self._package_to_device_apk_checksums_cache.clear()
 
   @classmethod
   def parallel(cls, devices=None, async=False):
     """Creates a Parallelizer to operate over the provided list of devices.
 
     If |devices| is either |None| or an empty list, the Parallelizer will
     operate over all attached devices that have not been blacklisted.
 
     Args:
       devices: A list of either DeviceUtils instances or objects from
@@ skipping 20 matching lines @@
   def HealthyDevices(cls):
     blacklist = device_blacklist.ReadBlacklist()
     def blacklisted(adb):
       if adb.GetDeviceSerial() in blacklist:
         logging.warning('Device %s is blacklisted.', adb.GetDeviceSerial())
         return True
       return False
 
     return [cls(adb) for adb in adb_wrapper.AdbWrapper.Devices()
             if not blacklisted(adb)]