SuperSize: Sort padding-only symbols at the end for diffs

The "overhead" symbols are often larger than most other individual symbols in a
diff, but are completely unactionable. This moves them to the bottom of
a diff so that they draw less attention.

Bug: 702625
Change-Id: Iddb4f2a75a8807cafce62e223a64a69886f1d0b2
Reviewed-on: https://chromium-review.googlesource.com/1116071Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570860}

tools/binary_size/libsupersize/integration_test.py

@@ -330,11 +330,25 @@ class IntegrationTest(unittest.TestCase):
     size_info2 = self._CloneSizeInfo(use_elf=False)
     size_info1.metadata = {"foo": 1, "bar": [1,2,3], "baz": "yes"}
     size_info2.metadata = {"foo": 1, "bar": [1,3], "baz": "yes"}
-    size_info1.symbols -= size_info1.symbols[:2]
-    size_info2.symbols -= size_info2.symbols[-3:]
-    size_info1.symbols[1].size -= 10
+
+    size_info1.raw_symbols -= size_info1.raw_symbols[:2]
+    size_info2.raw_symbols -= size_info2.raw_symbols[-3:]
+    changed_sym = size_info1.raw_symbols.WhereNameMatches('Patcher::Name_')[0]
+    changed_sym.size -= 10
+    padding_sym = size_info2.raw_symbols.WhereNameMatches('symbol gap 0')[0]
+    padding_sym.padding += 20
+    padding_sym.size += 20
     d = diff.Diff(size_info1, size_info2)
-    d.symbols = d.symbols.Sorted()
+    d.raw_symbols = d.raw_symbols.Sorted()
+    self.assertEquals(d.raw_symbols.CountsByDiffStatus()[1:], [2, 2, 3])
+    changed_sym = d.raw_symbols.WhereNameMatches('Patcher::Name_')[0]
+    padding_sym = d.raw_symbols.WhereNameMatches('symbol gap 0')[0]
+    # Padding-only deltas should sort after all non-padding changes.
+    padding_idx = d.raw_symbols.index(padding_sym)
+    self.assertLess(d.raw_symbols.index(changed_sym), padding_idx)
+    # And before bss.
+    self.assertTrue(d.raw_symbols[padding_idx + 1].IsBss())
+
     return describe.GenerateLines(d, verbose=True)
 
   def test_Diff_Aliases1(self):

tools/binary_size/libsupersize/models.py

@@ -701,8 +701,12 @@ class SymbolGroup(BaseSymbol):
 
   def Sorted(self, cmp_func=None, key=None, reverse=False):
     if cmp_func is None and key is None:
-      cmp_func = lambda a, b: cmp((a.IsBss(), abs(b.pss), a.name),
-                                  (b.IsBss(), abs(a.pss), b.name))
+      if self.IsDelta():
+        key = lambda s: (s.diff_status == DIFF_STATUS_UNCHANGED, s.IsBss(),
+                         s.size_without_padding == 0, -abs(s.pss), s.name)
+      else:
+        key = lambda s: (
+            s.IsBss(), s.size_without_padding == 0, -abs(s.pss), s.name)
 
     after_symbols = sorted(self._symbols, cmp_func, key, reverse)
     return self._CreateTransformed(

tools/binary_size/libsupersize/testdata/Diff_Basic.golden

@@ -38,22 +38,42 @@ Other section sizes:
     .strtab: 0 bytes (0 bytes)
     .symtab: 0 bytes (0 bytes)
 
-0 symbols added (+), 1 changed (~), 0 removed (-), 40 unchanged (not shown)
-Of changed symbols, 1 grew, 0 shrank
-Number of unique symbols 46 -> 46 (+0)
-0 paths added, 0 removed, 1 changed
+2 symbols added (+), 2 changed (~), 3 removed (-), 34 unchanged (not shown)
+Of changed symbols, 4 grew, 3 shrank
+Number of unique symbols 44 -> 43 (-1)
+0 paths added, 0 removed, 2 changed
 Changed files:
+  
   third_party/container.c
 
-Showing 1 symbols (1 -> 1 unique) with total pss: 10 bytes
+Showing 7 symbols (5 -> 4 unique) with total pss: 38 bytes
 Histogram of symbols based on PSS:
-    [8,16): 1
-.text=0 bytes    .rodata=0 bytes    .data.rel.ro=0 bytes    .data=10 bytes   .bss=0 bytes    total=10 bytes
-Number of unique paths: 1
+    (-256,-128]: 1   (-32,-16]: 1   (-8,-4]: 1   [4,8): 2   [8,16): 1   [16,32): 1
+.text=0 bytes    .rodata=10 bytes   .data.rel.ro=0 bytes    .data=28 bytes   .bss=-232 bytes total=38 bytes
+Number of unique paths: 4
 
 Section Legend: t=.text, r=.rodata, R=.data.rel.ro, d=.data, b=.bss
 Index | Running Total | Section@Address | ...
 ------------------------------------------------------------
-~ 0)         10 (100.0%) d@0x2de70a0  +10 (-6->4)  num_aliases=1
+~ 0)         10 (26.3%) r@0x284e398  +10 (22->32)  num_aliases=1
+               source_path=third_party/container.c 	object_path=third_party/sub/ContiguousContainer.o
+               flags={}  name=chrome::mojom::FilePatcher::Name_
++ 1)         14 (36.8%) d@0x2de7000  +4 (0->4)  num_aliases=1
+               source_path=base/page_allocator.cc 	object_path=base/base/page_allocator.o
+               flags={}  name=google::protobuf::internal::pLinuxKernelCmpxchg
++ 2)         18 (47.4%) d@0x2de7004  +4 (0->4)  num_aliases=1
                source_path=third_party/container.c 	object_path=third_party/sub/ContiguousContainer.o
-               flags={}  name=base::android::g_renderer_histogram_code
+               flags={}  name=google::protobuf::internal::pLinuxKernelMemoryBarrier
+~ 3)         38 (100.0%) d@0x2dffd88  20 (0->0)  num_aliases=1
+               source_path= 	object_path=
+               flags={}  name=** symbol gap 0 (end of section)
+- 4)         38 (100.0%) b@0x0        -200 (4->0)  num_aliases=1
+               source_path=third_party/icu/ucnv_ext.c 	object_path=third_party/icu/icuuc/ucnv_ext.o
+               flags={gen}  name=SaveHistogram::atomic_histogram_pointer
+                    full_name=SaveHistogram(_JNIEnv*, base::android::JavaParamRef<_jobject*> const&, base::android::JavaParamRef<_jstring*> const&, base::android::JavaParamRef<_jlongArray*> const&, int)::atomic_histogram_pointer
+- 5)         38 (100.0%) b@0x0        -28 (28->0)  num_aliases=1
+               source_path=third_party/icu/ucnv_ext.c 	object_path=third_party/icu/icuuc/ucnv_ext.o
+               flags={gen}  name=g_chrome_content_browser_client
+- 6)         38 (100.0%) b@0x0        -4 (4->0)  num_aliases=1
+               source_path=third_party/icu/ucnv_ext.c 	object_path=third_party/icu/icuuc/ucnv_ext.o
+               flags={anon,gen}  name=g_AnimationFrameTimeHistogram_clazz