Remove zipalign overhead from normalized apk size metric

Hoping that this will help multiple trybot runs with the same changes
yield the same size result.

Bug: 1130754
Change-Id: Ie4376919f7f18e177d5106c86a3f429389333cd7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2422718Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809139}

build/android/resource_sizes.py

@@ -469,6 +469,12 @@ def _DoApkAnalysis(apk_filename, apks_path, tool_prefix, out_dir, report_func):
   # Use a constant compression factor to account for fluctuations.
   normalized_apk_size -= java_code.ComputeZippedSize()
   normalized_apk_size += java_code.ComputeUncompressedSize()
+  # Don't include zipalign overhead in normalized size, since it effectively
+  # causes size changes files that proceed aligned files to be rounded.
+  # For APKs where classes.dex directly proceeds libchrome.so, this causes
+  # small dex size changes to disappear into libchrome.so alignment.
+  normalized_apk_size -= sum(len(i.extra) for i in apk_contents)
+
   # Unaligned size should be ~= uncompressed size or something is wrong.
   # As of now, padding_fraction ~= .007
   padding_fraction = -_PercentageDifference(

docs/speed/binary_size/metrics.md

@@ -38,6 +38,7 @@ For Googlers, more information available at [go/chrome-apk-size](https://goto.go
    * The size of an APK
    * With all native code as the sum of section sizes (except .bss), uncompressed.
    * With all dex code as if it were stored uncompressed.
+   * With all zipalign padding removed.
    * With all translations as if they were not missing (estimates size of missing translations based on size of english strings).
      * Without translation-normalization, translation dumps cause jumps.
      * Translation-normalization applies only to apks (not to Android App Bundles).

tools/binary_size/libsupersize/archive.py

@@ -1246,9 +1246,11 @@ def _ParseApkOtherSymbols(section_ranges, apk_path, apk_so_path,
   apk_symbols = []
   dex_size = 0
   zip_info_total = 0
+  zipalign_total = 0
   with zipfile.ZipFile(apk_path) as z:
     for zip_info in z.infolist():
       zip_info_total += zip_info.compress_size
+      zipalign_total += len(zip_info.extra)
       # Skip main shared library, pak, and dex files as they are accounted for.
       if (zip_info.filename == apk_so_path
           or zip_info.filename.endswith('.pak')):
@@ -1268,11 +1270,16 @@ def _ParseApkOtherSymbols(section_ranges, apk_path, apk_so_path,
               zip_info.compress_size,
               source_path=source_path,
               full_name=resource_filename))  # Full name must disambiguate
-  overhead_size = os.path.getsize(apk_path) - zip_info_total
+  overhead_size = os.path.getsize(apk_path) - zip_info_total - zipalign_total
   assert overhead_size >= 0, 'Apk overhead must be non-negative'
   zip_overhead_symbol = models.Symbol(
       models.SECTION_OTHER, overhead_size, full_name='Overhead: APK file')
   apk_symbols.append(zip_overhead_symbol)
+  if zipalign_total > 0:
+    zipalign_symbol = models.Symbol(models.SECTION_OTHER,
+                                    zipalign_total,
+                                    full_name='Overhead: zipalign')
+    apk_symbols.append(zipalign_symbol)
   _ExtendSectionRange(section_ranges, models.SECTION_OTHER,
                       sum(s.size for s in apk_symbols))
   return dex_size, apk_symbols

tools/binary_size/libsupersize/integration_test.py

@@ -125,9 +125,10 @@ class IntegrationTest(unittest.TestCase):
 
     with zipfile.ZipFile(_TEST_APK_PATH, 'w') as apk_file:
       apk_file.write(_TEST_ELF_PATH, _TEST_APK_SO_PATH)
-      # Exactly 4MB of data (2^22).
-      apk_file.writestr(
-          _TEST_APK_SMALL_SO_PATH, IntegrationTest._CreateBlankData(22))
+      # Exactly 4MB of data (2^22), with some zipalign overhead.
+      info = zipfile.ZipInfo(_TEST_APK_SMALL_SO_PATH)
+      info.extra = b'\x00' * 16
+      apk_file.writestr(info, IntegrationTest._CreateBlankData(22))
       # Exactly 1MB of data (2^20).
       apk_file.writestr(
           _TEST_APK_OTHER_FILE_PATH, IntegrationTest._CreateBlankData(20))

tools/binary_size/libsupersize/testdata/Archive_Apk.golden

 apk_file_name=test.apk
-apk_size=147858879
+apk_size=147858911
 elf_arch=arm
 elf_build_id=WhatAnAmazingBuildId
 elf_file_name=elf
@@ -74,8 +74,8 @@ Section .pak.nontranslated: has 100.0% of 737 bytes accounted for from 3 symbols
 * 0 have source paths. Accounts for 0 bytes (0.0%).
 * 0 have a component assigned. Accounts for 0 bytes (0.0%).
 * 0 symbols have shared ownership.
-Section .other: has 100.0% of 95595797 bytes accounted for from 27 symbols. 0 bytes are unaccounted for.
-* Padding accounts for 33903399 bytes (35.5%)
+Section .other: has 100.0% of 95595829 bytes accounted for from 28 symbols. 0 bytes are unaccounted for.
+* Padding accounts for 33903431 bytes (35.5%)
 * 3 have source paths. Accounts for 5243904 bytes (5.5%).
 * 1 have a component assigned. Accounts for 1048576 bytes (1.1%).
 * 22 placeholders exist (symbols that start with **). Accounts for 56448494 bytes (59.0%).
@@ -308,7 +308,8 @@ Section .other: has 100.0% of 95595797 bytes accounted for from 27 symbols. 0 by
 .other@0(size_without_padding=4194304,padding=0,full_name=smalltest.so,object_path=,source_path=$APK/smalltest.so,flags={gen},num_aliases=1,component=)
 .other@0(size_without_padding=1048576,padding=0,full_name=assets/icudtl.dat,object_path=,source_path=third_party/icu/android/icudtl.dat,flags={},num_aliases=1,component=Internal>Android)
 .other@0(size_without_padding=1024,padding=0,full_name=res/drawable-v13/test.xml,object_path=,source_path=chrome/android/res/drawable/test.xml,flags={},num_aliases=1,component=)
-.other@0(size_without_padding=0,padding=764,full_name=Overhead: APK file,object_path=,source_path=,flags={},num_aliases=1,component=)
+.other@0(size_without_padding=0,padding=780,full_name=Overhead: APK file,object_path=,source_path=,flags={},num_aliases=1,component=)
+.other@0(size_without_padding=0,padding=16,full_name=Overhead: zipalign,object_path=,source_path=,flags={},num_aliases=1,component=)
 .other@0(size_without_padding=0,padding=33902635,full_name=Overhead: ELF file,object_path=,source_path=,flags={},num_aliases=1,component=)
 .other@154(size_without_padding=19,padding=0,full_name=** ELF Section: .interp,object_path=,source_path=,flags={},num_aliases=1,component=)
 .other@168(size_without_padding=36,padding=0,full_name=** ELF Section: .note.gnu.build-id,object_path=,source_path=,flags={},num_aliases=1,component=)

tools/binary_size/libsupersize/testdata/Archive_MinimalApks.golden

 apk_file_name=Bundle.minimal.apks
 apk_size-vr=20
-apk_size=147858879
+apk_size=147858911
 elf_arch=arm
 elf_build_id=WhatAnAmazingBuildId
 elf_file_name=elf
@@ -75,8 +75,8 @@ Section .pak.nontranslated: has 100.0% of 737 bytes accounted for from 3 symbols
 * 0 have source paths. Accounts for 0 bytes (0.0%).
 * 0 have a component assigned. Accounts for 0 bytes (0.0%).
 * 0 symbols have shared ownership.
-Section .other: has 100.0% of 95595797 bytes accounted for from 27 symbols. 0 bytes are unaccounted for.
-* Padding accounts for 33903399 bytes (35.5%)
+Section .other: has 100.0% of 95595829 bytes accounted for from 28 symbols. 0 bytes are unaccounted for.
+* Padding accounts for 33903431 bytes (35.5%)
 * 3 have source paths. Accounts for 5243904 bytes (5.5%).
 * 1 have a component assigned. Accounts for 1048576 bytes (1.1%).
 * 22 placeholders exist (symbols that start with **). Accounts for 56448494 bytes (59.0%).
@@ -309,7 +309,8 @@ Section .other: has 100.0% of 95595797 bytes accounted for from 27 symbols. 0 by
 .other@0(size_without_padding=4194304,padding=0,full_name=smalltest.so,object_path=,source_path=$APK/smalltest.so,flags={gen},num_aliases=1,component=)
 .other@0(size_without_padding=1048576,padding=0,full_name=assets/icudtl.dat,object_path=,source_path=third_party/icu/android/icudtl.dat,flags={},num_aliases=1,component=Internal>Android)
 .other@0(size_without_padding=1024,padding=0,full_name=res/drawable-v13/test.xml,object_path=,source_path=chrome/android/res/drawable/test.xml,flags={},num_aliases=1,component=)
-.other@0(size_without_padding=0,padding=764,full_name=Overhead: APK file,object_path=,source_path=,flags={},num_aliases=1,component=)
+.other@0(size_without_padding=0,padding=780,full_name=Overhead: APK file,object_path=,source_path=,flags={},num_aliases=1,component=)
+.other@0(size_without_padding=0,padding=16,full_name=Overhead: zipalign,object_path=,source_path=,flags={},num_aliases=1,component=)
 .other@0(size_without_padding=0,padding=33902635,full_name=Overhead: ELF file,object_path=,source_path=,flags={},num_aliases=1,component=)
 .other@154(size_without_padding=19,padding=0,full_name=** ELF Section: .interp,object_path=,source_path=,flags={},num_aliases=1,component=)
 .other@168(size_without_padding=36,padding=0,full_name=** ELF Section: .note.gnu.build-id,object_path=,source_path=,flags={},num_aliases=1,component=)