resource_sizes.py: Better pakfile and dex normalization.

build/android/resource_sizes.py

Index: build/android/resource_sizes.py
diff --git a/build/android/resource_sizes.py b/build/android/resource_sizes.py
index 748775ae72d93692ea5a1b1c2f6da5962d9cf0cf..f9bb2559d63890cb95eb64b71ec164ab4474652e 100755
--- a/build/android/resource_sizes.py
+++ b/build/android/resource_sizes.py
@@ -111,6 +111,11 @@ _DUMP_STATIC_INITIALIZERS_PATH = os.path.join(
 # Pragma exists when enable_resource_whitelist_generation=true.
 _RC_HEADER_RE = re.compile(
     r'^#define (?P<name>\w+) (?:_Pragma\(.*?\) )?(?P<id>\d+)$')
+_RE_NON_LANGUAGE_PAK = re.compile(r'^assets/.*(resources|percent)\.pak$')
+_RE_LANGUAGE_PAK = re.compile(

[agrieve, 2017/06/14 14:11:12]

nit: RE_LANGUAGE_PAK -> RE_COMPRESSED_LANGUAGE_PAK

[estevenson, 2017/06/15 02:56:44]

Done.

+    r'\.lpak$|^assets/(?!stored-locales/).*(?!resources|percent)\.pak$')
+_RE_STORED_LANGUAGE_PAK = re.compile(
+    r'\.lpak$|^assets/stored-locales/.*(?!resources|percent)\.pak$')

[agrieve, 2017/06/14 14:11:13]

nit: can remove .lpak from this list. .lpak is historical, but was only ever
used to allow compression.

[estevenson, 2017/06/15 02:56:43]

Done.

 _READELF_SIZES_METRICS = {
   'text': ['.text'],
   'data': ['.data', '.rodata', '.data.rel.ro', '.data.rel.ro.local'],
@@ -165,6 +170,30 @@ def _ParseLibBuildId(so_path, tools_prefix):
   return match.group(1) if match else None
 
 
+def _ParseManifestAttributes(apk_path):
+  # Check if the manifest specifies whether or not to extract native libs.
+  skip_extract_lib = False
+  output = cmd_helper.GetCmdOutput([
+      'aapt', 'd', 'xmltree', apk_path, 'AndroidManifest.xml'])

[agrieve, 2017/06/14 14:11:12]

I don't think we can assume aapt will be in the PATH.  I think to get this, the
best way is to read build_vars.txt.

[estevenson, 2017/06/15 02:56:43]

Forgot that we already have AAPT_PATH via devil.android.sdk.build_tools. Done.

+  m = re.search(r'extractNativeLibs\(.*\)=\(.*\)(\w)', output)
+  if m:
+    skip_extract_lib = not bool(int(m.group(1)))
+
+  # Dex decompression overhead varies by Android version.
+  output = cmd_helper.GetCmdOutput(['aapt', 'd', 'badging', apk_path])

[agrieve, 2017/06/14 14:11:12]

nit: no need to run both badging and xmltree. The minSdkVersion is in the output
of xmltree already:

    E: uses-sdk (line=19)
      A: android:minSdkVersion(0x0101020c)=(type 0x10)0x18
      A: android:targetSdkVersion(0x01010270)=(type 0x10)0x1a

[estevenson, 2017/06/15 02:56:43]

Ahh missed that. Done!

+  sdk_version = int(re.search(r'sdkVersion:\'(\d+)\'', output).group(1))
+  if sdk_version < 21:
+    dex_multiplier = 1
+  elif sdk_version < 24:
+    dex_multiplier = 3
+  elif 'monochrome' in apk_path.lower():  # Extracted for WebView and Chrome.

[agrieve, 2017/06/14 14:11:13]

Worth a comment saying why this is the case.

[estevenson, 2017/06/15 02:56:44]

Done. Can you confirm that it should be a multiplier of 2 here not 3?

+    dex_multiplier = 3

[estevenson, 2017/06/12 16:56:28]

Should this be 2?

+  else:
+    dex_multiplier = 1
+
+  return dex_multiplier, skip_extract_lib
+
+
 def CountStaticInitializers(so_path, tools_prefix):
   # Static initializers expected in official builds. Note that this list is
   # built using 'nm' on libchrome.so which results from a GCC official build
@@ -207,6 +236,16 @@ def GetStaticInitializers(so_path, tools_prefix):
   return output.splitlines()[:-1], int(summary.group(1))
 
 
+def _NormalizeLanguagePaks(translations, normalized_apk_size, factor):
+  english_pak = translations.FindByPattern(r'.*/en[-_][Uu][Ss]\.l?pak')
+  num_translations = translations.GetNumEntries()
+  if english_pak:
+    normalized_apk_size -= translations.ComputeZippedSize()
+    normalized_apk_size += int(
+        english_pak.compress_size * num_translations * factor)
+  return normalized_apk_size
+
+
 def _NormalizeResourcesArsc(apk_path):
   """Estimates the expected overhead of untranslated strings in resources.arsc.
 
@@ -282,11 +321,11 @@ class _FileGroup(object):
   def __init__(self, name):
     self.name = name
     self._zip_infos = []
-    self._extracted = []
+    self._extracted_multipliers = []
 
-  def AddZipInfo(self, zip_info, extracted=False):
+  def AddZipInfo(self, zip_info, extracted_multiplier=None):

[agrieve, 2017/06/14 14:11:13]

nit: maybe make the default 0, so that you don't have to add the "if multiplier"
below?

[estevenson, 2017/06/15 02:56:44]

Done.

     self._zip_infos.append(zip_info)
-    self._extracted.append(extracted)
+    self._extracted_multipliers.append(extracted_multiplier)
 
   def AllEntries(self):
     return iter(self._zip_infos)
@@ -311,9 +350,9 @@ class _FileGroup(object):
 
   def ComputeExtractedSize(self):
     ret = 0
-    for zi, extracted in zip(self._zip_infos, self._extracted):
-      if extracted:
-        ret += zi.file_size
+    for zi, multiplier in zip(self._zip_infos, self._extracted_multipliers):
+      if multiplier:
+        ret += zi.file_size * int(multiplier)
     return ret
 
   def ComputeInstallSize(self):
@@ -333,6 +372,7 @@ def PrintApkAnalysis(apk_filename, tools_prefix, chartjson=None):
   java_code = make_group('Java code')
   native_resources_no_translations = make_group('Native resources (no l10n)')
   translations = make_group('Native resources (l10n)')
+  stored_translations = make_group('Native resources stored (l10n)')
   icu_data = make_group('ICU (i18n library) data')
   v8_snapshots = make_group('V8 Snapshots')
   png_drawables = make_group('PNG drawables')
@@ -348,22 +388,25 @@ def PrintApkAnalysis(apk_filename, tools_prefix, chartjson=None):
   finally:
     apk.close()
 
+  dex_multiplier, skip_extract_lib = _ParseManifestAttributes(apk_filename)
   total_apk_size = os.path.getsize(apk_filename)
   apk_basename = os.path.basename(apk_filename)
-
   for member in apk_contents:
     filename = member.filename
     if filename.endswith('/'):
       continue
-
     if filename.endswith('.so'):
-      native_code.AddZipInfo(member, 'crazy' not in filename)
+      should_extract_lib = not (skip_extract_lib or 'crazy' in filename)
+      native_code.AddZipInfo(member, should_extract_lib)

[agrieve, 2017/06/14 14:11:12]

nit: change to int(should_extract_lib)

[estevenson, 2017/06/15 02:56:43]

Done.

     elif filename.endswith('.dex'):
-      java_code.AddZipInfo(member, True)
-    elif re.search(r'^assets/.*(resources|percent)\.pak$', filename):
+      java_code.AddZipInfo(member, dex_multiplier)
+    elif re.search(_RE_NON_LANGUAGE_PAK, filename):
       native_resources_no_translations.AddZipInfo(member)
-    elif re.search(r'\.lpak$|^assets/.*(?!resources|percent)\.pak$', filename):
+    elif re.search(_RE_LANGUAGE_PAK, filename):
       translations.AddZipInfo(member, 'en_' in filename or 'en-' in filename)

[agrieve, 2017/06/14 14:11:13]

nit: add int() to 2nd param

[estevenson, 2017/06/15 02:56:44]

Done.

+    elif re.search(_RE_STORED_LANGUAGE_PAK, filename):
+      stored_translations.AddZipInfo(
+          member, 'en_' in filename or 'en-' in filename)

[agrieve, 2017/06/14 14:11:13]

I think 2nd param here should always be 0 (never extracted)

[estevenson, 2017/06/15 02:56:43]

Definitely. Done.

     elif filename == 'assets/icudtl.dat':
       icu_data.AddZipInfo(member)
     elif filename.endswith('.bin'):
@@ -438,14 +481,12 @@ def PrintApkAnalysis(apk_filename, tools_prefix, chartjson=None):
   normalized_apk_size += native_code.ComputeUncompressedSize()
   # Avoid noise caused when strings change and translations haven't yet been
   # updated.
-  english_pak = translations.FindByPattern(r'.*/en[-_][Uu][Ss]\.l?pak')
   num_translations = translations.GetNumEntries()
-  if english_pak and num_translations > 1:
-    normalized_apk_size -= translations.ComputeZippedSize()
-    # 1.17 found by looking at Chrome.apk and seeing how much smaller en-US.pak

[agrieve, 2017/06/14 14:11:13]

can we keep this comment?

[estevenson, 2017/06/15 02:56:44]

Done.

-    # is relative to the average locale .pak.
-    normalized_apk_size += int(
-        english_pak.compress_size * num_translations * 1.17)
+  if num_translations > 1:
+    normalized_apk_size = _NormalizeLanguagePaks(
+        translations, normalized_apk_size, 1.17)
+    normalized_apk_size = _NormalizeLanguagePaks(
+        stored_translations, normalized_apk_size, 1.43)
     normalized_apk_size += int(_NormalizeResourcesArsc(apk_filename))
 
   ReportPerfResult(chartjson, apk_basename + '_Specifics',