[Android] Tune DeviceUtils commands that are prone to large outputs.

build/android/pylib/device/device_utils.py

Index: build/android/pylib/device/device_utils.py
diff --git a/build/android/pylib/device/device_utils.py b/build/android/pylib/device/device_utils.py
index f7167cea13a20e9401ff03556d1b4e49b8bddfb2..419740b7883955f578101f52ebc594dbcbb5cb86 100644
--- a/build/android/pylib/device/device_utils.py
+++ b/build/android/pylib/device/device_utils.py
@@ -593,7 +593,7 @@ class DeviceUtils(object):
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
-    pids = self._GetPidsImpl(process_name)
+    pids = self.GetPids(process_name)
     if not pids:
       raise device_errors.CommandFailedError(
           'No process "%s"' % process_name, str(self))
@@ -603,7 +603,7 @@ class DeviceUtils(object):
 
     if blocking:
       wait_period = 0.1
-      while self._GetPidsImpl(process_name):
+      while self.GetPids(process_name):
         time.sleep(wait_period)
 
     return len(pids)
@@ -1020,7 +1020,8 @@ class DeviceUtils(object):
 
     if size is None or size <= self._MAX_ADB_OUTPUT_LENGTH:
       return _JoinLines(self.RunShellCommand(
-          ['cat', device_path], as_root=as_root, check_return=True))
+          ['cat', device_path], as_root=as_root, check_return=True,
+          large_output=(not bool(size))))

[perezju, 2015/04/13 09:58:32]

I don't think this makes much sense, and introduces a cyclic dependency that
makes me worry.

We are using "cat" as an optimization to avoid having to pull the file from the
device. But with large_output=True we'll instead cat the file in the device to
_another_ file which is then read using ... ReadFile!

I would prefer:
- If we can't determine the size of the file, play it safe and just do a pull.
- If the file of the size is 0, don't event try to read anything from the
device, just return '' (or '\n'?).
- Only use cat if we know that the size is small enough so that large_output
would not be needed (otherwise the "optimization" is pointless).

[rnephew (Wrong account), 2015/04/13 17:21:04]

In at least the /proc/ fs on the device, and probably elsewhere, some files
return 0 size even though they are not 0 size. One example is /proc/wakelocks:
adb shell ls -l /proc/wakelocks ; adb shell cat /proc/wakelocks | wc
-r--r--r-- root     root            0 2015-04-13 13:19 wakelocks
     64     576    3729

[jbudorick, 2015/04/13 17:44:29]

This doesn't make much sense because what it's reading doesn't really make much
sense.

e.g. telemetry reads /proc/timer_list here:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

/proc/timer_list reports zero size, but actually will dump quite a bit of data
when catting:

jbudorick@jbudorick:~$ adb shell ls -l /proc/timer_list && adb pull
/proc/timer_list timer_list.txt && ls -l timer_list.txt
-r--r--r-- root     root            0 2015-04-13 08:18 timer_list
70 KB/s (5670 bytes in 0.078s)
-rw-r----- 1 jbudorick eng 5670 Apr 13 06:18 timer_list.txt
I had thought that pull didn't work with files that report zero size, but it
certainly seems to do so. I'll check this again on K and then switch so long as
it works.
This is wrong, see above.

     elif as_root and self.NeedsSU():
       with device_temp_file.DeviceTempFile(self.adb) as device_temp:
         self.RunShellCommand(['cp', device_path, device_temp.name],
@@ -1353,11 +1354,9 @@ class DeviceUtils(object):
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
-    return self._GetPidsImpl(process_name)
-
-  def _GetPidsImpl(self, process_name):
     procs_pids = {}
-    for line in self.RunShellCommand('ps', check_return=True):
+    for line in self.RunShellCommand('ps', check_return=True,
+                                     large_output=True):

[perezju, 2015/04/13 09:58:32]

This is precisely one of the things I really think we shouldn't do.

I think that, instead of large_output=True, we should change the command to 'ps
| grep -F {{process_name}}' instead.

[jbudorick, 2015/04/13 17:44:29]

Done.

       try:
         ps_data = line.split()
         if process_name in ps_data[-1]:
@@ -1430,7 +1429,8 @@ class DeviceUtils(object):
         'Private_Dirty')
 
     showmap_out = self.RunShellCommand(
-        ['showmap', str(pid)], as_root=True, check_return=True)
+        ['showmap', str(pid)], as_root=True, check_return=True,
+        large_output=True)

[perezju, 2015/04/13 09:58:32]

How about: 'showmap {{pid}} | grep TOTAL'

[jbudorick, 2015/04/13 17:44:29]

Done.

     if not showmap_out:
       raise device_errors.CommandFailedError('No output from showmap')