[Telemetry] Speed up page_cycler on android.

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 04b73dec612698288d0312daa787dc388970fc83..b5fab46ff63b6065a4161c4a48540a836ca82e04 100644
--- a/build/android/pylib/android_commands.py
+++ b/build/android/pylib/android_commands.py
@@ -55,10 +55,6 @@ LOCAL_PROPERTIES_PATH = constants.DEVICE_LOCAL_PROPERTIES_PATH
 # Property in /data/local.prop that controls Java assertions.
 JAVA_ASSERT_PROPERTY = 'dalvik.vm.enableassertions'
 
-MEMORY_INFO_RE = re.compile('^(?P<key>\w+):\s+(?P<usage_kb>\d+) kB$')
-NVIDIA_MEMORY_INFO_RE = re.compile('^\s*(?P<user>\S+)\s*(?P<name>\S+)\s*'
-                                   '(?P<pid>\d+)\s*(?P<usage_bytes>\d+)$')
-
 # Keycode "enum" suitable for passing to AndroidCommands.SendKey().
 KEYCODE_HOME = 3
 KEYCODE_BACK = 4
@@ -1598,7 +1594,7 @@ class AndroidCommands(object):
       [0]: Dict of {metric:usage_kb}, for the process which has specified pid.
       The metric keys which may be included are: Size, Rss, Pss, Shared_Clean,
       Shared_Dirty, Private_Clean, Private_Dirty, Referenced, Swap,
-      KernelPageSize, MMUPageSize, Nvidia (tablet only), VmHWM.
+      KernelPageSize, MMUPageSize, VmHWM.
       [1]: Detailed /proc/[PID]/smaps information.
     """
     usage_dict = collections.defaultdict(int)
@@ -1612,10 +1608,10 @@ class AndroidCommands(object):
         current_smap = ' '.join(items[5:])
       elif len(items) > 3:
         current_smap = ' '.join(items[3:])
-      match = re.match(MEMORY_INFO_RE, line)
-      if match:
-        key = match.group('key')
-        usage_kb = int(match.group('usage_kb'))
+      if line.endswith('kB'):
+        key, value = line.split(':')
+        key = key.strip()
+        usage_kb = int(value.strip().split()[0])
         usage_dict[key] += usage_kb
         if key not in smaps[current_smap]:
           smaps[current_smap][key] = 0
@@ -1624,18 +1620,12 @@ class AndroidCommands(object):
       # Presumably the process died between ps and calling this method.
       logging.warning('Could not find memory usage for pid ' + str(pid))
 
-    for line in self.GetProtectedFileContents('/d/nvmap/generic-0/clients'):

[tonyg, 2014/05/13 14:37:50]

Are you sure we don't care about this any more for any nexus on the bots? If it
is at all useful, we could also perhaps check for the file once, cache whether
it exists and then shortcut reading the file if it didn't exist.

[bulach, 2014/05/13 14:45:57]

well... :)
way up in telemetry, this is not being used at all:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

I'm not aware of any bots looking into this.
ok to nuke it? git log would show anyways, should we ever want to bring this
back...

-      match = re.match(NVIDIA_MEMORY_INFO_RE, line)
-      if match and match.group('pid') == pid:
-        usage_bytes = int(match.group('usage_bytes'))
-        usage_dict['Nvidia'] = int(round(usage_bytes / 1000.0))  # kB
-        break
-
     peak_value_kb = 0
     for line in self.GetProtectedFileContents('/proc/%s/status' % pid):
       if not line.startswith('VmHWM:'):  # Format: 'VmHWM: +[0-9]+ kB'
         continue
       peak_value_kb = int(line.split(':')[1].strip().split(' ')[0])

[picksi, 2014/05/14 16:32:18]

It looks like the code to read the peak_value_kb is almost the same as the new
(much nicer!) code you added above to read the key & usage_kb. Is there a way to
avoid this (almost) duplication? [... long discussion about functions returning
keys & ints that aren't always used]

[bulach, 2014/05/14 16:52:56]

hehe :) there was another follow up on this:
https://codereview.chromium.org/280703010/diff/20001/build/android/pylib/andr...

so this duplication is no longer happening: the bit above is now using "showmap"
command, which faster but has a slightly different format..

having said that, thanks for highlighting this! there's no need to use a
"defaultdict" anymore, patch on its way:
https://codereview.chromium.org/285693008

+      break
     usage_dict['VmHWM'] = peak_value_kb
     if not peak_value_kb:
       logging.warning('Could not find memory peak value for pid ' + str(pid))

[picksi, 2014/05/14 16:32:18]

This relies on a zero peak value meaning it couldn't find a peak value. I assume
that a zero-peak is not possible, but this feels like using one variable for two
separate jobs. Especially as 'if not peak_value_kb' is treating peak_value_kb as
a boolean, when we all know its really an int!

Now the 'break' exists we could write the peak value directly into the
usage_dict (unless writing 0 in is necessary?), and then use it's absence from
the dictionary to trigger the logging warning... or set a 'key found' flag to
true if efficiency is an issue...

I've gone too far down this rabbit hole for a single line of code now ...

[bulach, 2014/05/14 16:52:56]

yeah, this is a good use for python's "for..else" construction!


(having said that, afaict the styleguide allows using boolean coercion for ints
and None on "if" clauses, so it wasn't truly wrong given that it can't have a 0
peak... but yes, it'll be clearer now anyways :)