[android] Push only updated files in PushIfNeeded when few files have changed.

build/android/pylib/android_commands.py

Index: build/android/pylib/android_commands.py
diff --git a/build/android/pylib/android_commands.py b/build/android/pylib/android_commands.py
index 4cf070d0711a9626edf1ba6eae35efe0f7bb7e40..8687d5401c1d11279ae6de50f5d934b7e71804b9 100644
--- a/build/android/pylib/android_commands.py
+++ b/build/android/pylib/android_commands.py
@@ -738,15 +738,15 @@ class AndroidCommands(object):
     """
     self.RunShellCommand('input keyevent %d' % keycode)
 
-  def CheckMd5Sum(self, local_path, device_path):
-    """Compares the md5sum of a local path against a device path.
+  def _RunMd5Sum(self, local_path, device_path):
+    """Gets the md5sum of a local path and device path.
 
     Args:
       local_path: Path (file or directory) on the host.
       device_path: Path on the device.
 
     Returns:
-      True if the md5sums match.
+      A tuple containing lists of the local and device md5sum results.

[frankf, 2013/07/31 22:58:19]

Define exactly what tuple is.

[craigdh, 2013/08/05 23:55:43]

Done.

     """
     if not self._md5sum_build_dir:
       default_build_type = os.environ.get('BUILD_TYPE', 'Debug')
@@ -769,6 +769,22 @@ class AndroidCommands(object):
     md5sum_output = cmd_helper.GetCmdOutput(
         ['%s/md5sum_bin_host' % self._md5sum_build_dir, local_path])
     host_hash_tuples = _ComputeFileListHash(md5sum_output.splitlines())
+    return (host_hash_tuples, device_hash_tuples)
+
+  def GetFileDiff(self, local_path, device_path):

[frankf, 2013/07/31 22:58:19]

GetFileDiff -> GetFilesChanged

[craigdh, 2013/08/05 23:55:43]

Done.

+    """Compares the md5sum of a local path against a device path.
+
+    Note: Ignores extra files on the device.
+
+    Args:
+      local_path: Path (file or directory) on the host.
+      device_path: Path on the device.
+
+    Returns:
+      A list of files whose md5sums do not match.
+    """
+    host_hash_tuples, device_hash_tuples = self._RunMd5Sum(
+        local_path, device_path)
 
     # Ignore extra files on the device.
     if len(device_hash_tuples) > len(host_hash_tuples):
@@ -778,16 +794,30 @@ class AndroidCommands(object):
       def _host_has(fname):

[frankf, 2013/07/31 22:58:19]

naming issue

[craigdh, 2013/08/05 23:55:43]

Done.

         return any(path in fname for path in host_files)
 
-      hashes_on_device = [h.hash for h in device_hash_tuples if
-                         _host_has(h.path)]
-    else:
-      hashes_on_device = [h.hash for h in device_hash_tuples]
+      device_hash_tuples = [h for h in device_hash_tuples if _host_has(h.path)]
+
+    # Constructs the target device path from a given host path. Don't use when
+    # only a single file is given as the device_path may specify a rename.

[frankf, 2013/07/31 22:58:19]

What rename?

[craigdh, 2013/08/05 23:55:43]

If the local_path and device_path refer to a file but have a different base name
then the file is being renamed.

+    def _h2d_path(host_path):

[frankf, 2013/07/31 22:58:19]

Use our naming convention

[craigdh, 2013/08/05 23:55:43]

Done.

+      return os.path.join(os.path.dirname(device_path), os.path.relpath(
+          host_path, os.path.dirname(os.path.normpath(local_path))))
+
+    device_hashes = [h.hash for h in device_hash_tuples]
+    return [(t.path,
+             _h2d_path(t.path) if os.path.isdir(local_path) else device_path)

[frankf, 2013/07/31 22:58:19]

You're returning a list of tuples, this doesn't match the docstring

[craigdh, 2013/08/05 23:55:43]

Done.

+            for t in host_hash_tuples if t.hash not in device_hashes]
+
+  def CheckMd5Sum(self, local_path, device_path):
+    """Compares the md5sum of a local path against a device path.
 
-    # Compare md5sums between host and device files.
-    hashes_on_host = [h.hash for h in host_hash_tuples]
-    hashes_on_device.sort()
-    hashes_on_host.sort()
-    return hashes_on_device == hashes_on_host
+    Args:
+      local_path: Path (file or directory) on the host.
+      device_path: Path on the device.
+
+    Returns:
+      True if the md5sums match.
+    """
+    return not self.GetFileDiff(local_path, device_path)
 
   def PushIfNeeded(self, local_path, device_path):
     """Pushes |local_path| to |device_path|.

[frankf, 2013/07/31 22:58:19]

rename local to host everywhere

[craigdh, 2013/08/05 23:55:43]

Done.

@@ -798,38 +828,51 @@ class AndroidCommands(object):
     All pushed files can be removed by calling RemovePushedFiles().
     """
     assert os.path.exists(local_path), 'Local path not found %s' % local_path
-    size = int(cmd_helper.GetCmdOutput(['du', '-sb', local_path]).split()[0])
+
+    def _get_local_size(path):
+      return int(cmd_helper.GetCmdOutput(['du', '-sb', path]).split()[0])
+
+    size = _get_local_size(local_path)
     self._pushed_files.append(device_path)
     self._potential_push_size += size
 
-    if self.CheckMd5Sum(local_path, device_path):
+    missing_files = self.GetFileDiff(local_path, device_path)
+    if not missing_files:
       return
 
-    self._actual_push_size += size
-    # They don't match, so remove everything first and then create it.
-    if os.path.isdir(local_path):
-      self.RunShellCommand('rm -r %s' % device_path, timeout_time=2 * 60)
-      self.RunShellCommand('mkdir -p %s' % device_path)
-
-    # NOTE: We can't use adb_interface.Push() because it hardcodes a timeout of
-    # 60 seconds which isn't sufficient for a lot of users of this method.
-    push_command = 'push %s %s' % (local_path, device_path)
-    self._LogShell(push_command)
-
-    # Retry push with increasing backoff if the device is busy.
-    retry = 0
-    while True:
-      output = self._adb.SendCommand(push_command, timeout_time=30 * 60)
-      if _HasAdbPushSucceeded(output):
-        return
-      if 'resource busy' in output and retry < 3:
-        retry += 1
-        wait_time = 5 * retry
-        logging.error('Push failed, retrying in %d seconds: %s' %
-                      (wait_time, output))
-        time.sleep(wait_time)
-      else:
-        raise Exception('Push failed: %s' % output)
+    def _push(local, device):

[frankf, 2013/07/31 22:58:19]

Naming

[craigdh, 2013/08/05 23:55:43]

Done.

+      # NOTE: We can't use adb_interface.Push() because it hardcodes a timeout
+      # of 60 seconds which isn't sufficient for a lot of users of this method.
+      push_command = 'push %s %s' % (local, device)
+      self._LogShell(push_command)
+
+      # Retry push with increasing backoff if the device is busy.
+      retry = 0
+      while True:
+        output = self._adb.SendCommand(push_command, timeout_time=30 * 60)
+        if _HasAdbPushSucceeded(output):
+          return
+        if 'resource busy' in output and retry < 3:
+          retry += 1
+          wait_time = 5 * retry
+          logging.error('Push failed, retrying in %d seconds: %s' %
+                        (wait_time, output))
+          time.sleep(wait_time)
+        else:
+          raise Exception('Push failed: %s' % output)
+
+    # Above a small number of files it's likely faster to push everything.
+    if len(missing_files) > 20:

[frankf, 2013/07/31 22:58:19]

This is very arbitrary. Why number of files instead of size? Why 20? Do you have
empirical data?

[craigdh, 2013/08/05 23:55:43]

Based on size for now with a cutoff and added a TODO. I plan on working out the
heuristic for a follow up cl.

+      # We're pushing everything, remove everything first and then create it.
+      self._actual_push_size += size
+      if os.path.isdir(local_path):
+        self.RunShellCommand('rm -r %s' % device_path, timeout_time=2 * 60)
+        self.RunShellCommand('mkdir -p %s' % device_path)
+      _push(local_path, device_path)
+    else:
+      for f in missing_files:
+        self._actual_push_size += _get_local_size(f[0])
+        _push(f[0], f[1])
 
   def GetPushSizeInfo(self):
     """Get total size of pushes to the device done via PushIfNeeded()