SuperSize: Better support for class merging

 * source_path, object_path, template_name, and "name" all now
   use the class name that a symbol was merged from rather than
   the one it was merged into.
 * Made lambda name normalization work for class merging
 * Changed object_path to be the same whether or not source_path
   exists (because why would it differ!?)
 * Added more tests :).

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

tools/binary_size/libsupersize/apkanalyzer.py

@@ -10,6 +10,7 @@
 import collections
 import logging
 import os
+import posixpath
 import subprocess
 import zipfile
 
@@ -143,6 +144,80 @@ def _TruncateFrom(value, delimiter, rfind=False):
   return value
 
 
+# Visible for testing.
+class LambdaNormalizer:
+  def __init__(self):
+    self._lambda_by_class_counter = collections.defaultdict(int)
+    self._lambda_name_to_nested_number = {}
+
+  def Normalize(self, package, name):
+    # Make d8 desugared lambdas look the same as Desugar ones.
+    # D8 lambda: org.-$$Lambda$Promise$Nested1$kjevdDQ8V2zqCrdieLqWLHzk.dex
+    # Desugar lambda: org.Promise$Nested1$$Lambda$0
+    # 1) Need to prefix with proper class name so that they will show as nested.
+    # 2) Need to suffix with number so that they diff better.
+    # Original name will be kept as "object_path".
+    lambda_start_idx = package.find('-$$Lambda$')
+    class_path = package
+    if lambda_start_idx != -1:
+      lambda_end_idx = package.find('.dex') + len('.dex')
+      old_lambda_name = package[lambda_start_idx:lambda_end_idx]
+      class_path = package.replace('-$$Lambda$', '')
+      base_name = _TruncateFrom(class_path, '$', rfind=True)
+      # Map all methods of the lambda class to the same nested number.
+      lambda_number = self._lambda_name_to_nested_number.get(class_path)
+      if lambda_number is None:
+        # First time we've seen this lambda, increment nested class count.
+        lambda_number = self._lambda_by_class_counter[base_name]
+        self._lambda_name_to_nested_number[class_path] = lambda_number
+        self._lambda_by_class_counter[base_name] = lambda_number + 1
+
+      new_lambda_name = '{}$$Lambda${}'.format(base_name[lambda_start_idx:],
+                                               lambda_number)
+      name = name.replace(old_lambda_name, new_lambda_name)
+
+    # Map nested classes to outer class.
+    outer_class = _TruncateFrom(class_path, '$')
+    return outer_class, name
+
+
+# Visible for testing.
+def CreateDexSymbol(name, size, source_map, lambda_normalizer):
+  parts = name.split(' ')  # (class_name, return_type, method_name)
+  new_package = parts[0]
+
+  if new_package == _TOTAL_NODE_NAME:
+    return None
+
+  # Make d8 desugared lambdas look the same as Desugar ones.
+  outer_class, name = lambda_normalizer.Normalize(new_package, name)
+
+  # Look for class merging.
+  old_package = new_package
+  # len(parts) == 2 for class nodes.
+  if len(parts) > 2:
+    method = parts[2]
+    # last_idx == -1 for fields, which is fine.
+    last_idx = method.find('(')
+    last_idx = method.rfind('.', 0, last_idx)
+    if last_idx != -1:
+      old_package = method[:last_idx]
+      outer_class, name = lambda_normalizer.Normalize(old_package, name)
+
+  source_path = source_map.get(outer_class, '')
+  object_path = posixpath.join(models.APK_PREFIX_PATH, *old_package.split('.'))
+  if name.endswith(')'):
+    section_name = models.SECTION_DEX_METHOD
+  else:
+    section_name = models.SECTION_DEX
+
+  return models.Symbol(section_name,
+                       size,
+                       full_name=name,
+                       object_path=object_path,
+                       source_path=source_path)
+
+
 def CreateDexSymbols(apk_path, mapping_path, size_info_prefix):
   source_map = _ParseJarInfoFile(size_info_prefix + '.jar.info')
 
@@ -161,50 +236,12 @@ def CreateDexSymbols(apk_path, mapping_path, size_info_prefix):
       total_node_size)
   # Use (DEX_METHODS, DEX) buckets to speed up sorting.
   symbols = ([], [])
-  lambda_by_class_counter = collections.defaultdict(int)
-  lambda_name_to_nested_number = {}
+  lambda_normalizer = LambdaNormalizer()
   for _, name, node_size in nodes:
-    package = _TruncateFrom(name, ' ')
-    # Make d8 desugared lambdas look the same as Desugar ones.
-    # D8 lambda: -$$Lambda$Promise$Nested1$kjevdDQ8V2zqCrdieLqWLHzk.dex
-    # Desugar lambda: Promise$Nested1$$Lambda$0
-    # 1) Need to prefix with proper class name so that they will show as nested.
-    # 2) Need to suffix with number so that they diff better.
-    # Original name will be kept as "object_path".
-    is_d8_lambda = '-$$Lambda$' in package
-    class_path = package
-    if is_d8_lambda:
-      class_path = package.replace('-$$Lambda$', '')
-      base_name = _TruncateFrom(class_path, '$', rfind=True)
-      # Map all methods of the lambda class to the same nested number.
-      lambda_number = lambda_name_to_nested_number.get(class_path)
-      if lambda_number is None:
-        # First time we've seen this lambda, increment nested class count.
-        lambda_number = lambda_by_class_counter[base_name]
-        lambda_name_to_nested_number[class_path] = lambda_number
-        lambda_by_class_counter[base_name] = lambda_number + 1
-
-      name = '{}$$Lambda${}{}'.format(base_name, lambda_number,
-                                      name[len(package):])
-    # Map all nested classes to outer class.
-    source_path = source_map.get(_TruncateFrom(class_path, '$'), '')
-    if source_path:
-      object_path = package
-    elif package == _TOTAL_NODE_NAME:
-      # Unattributed size is handled outside of this function.
-      continue
-    else:
-      object_path = os.path.join(models.APK_PREFIX_PATH, *package.split('.'))
-    if name.endswith(')'):
-      section_name = models.SECTION_DEX_METHOD
-    else:
-      section_name = models.SECTION_DEX
-    symbols[int(section_name is models.SECTION_DEX)].append(
-        models.Symbol(section_name,
-                      node_size,
-                      full_name=name,
-                      object_path=object_path,
-                      source_path=source_path))
+    symbol = CreateDexSymbol(name, node_size, source_map, lambda_normalizer)
+    if symbol:
+      symbols[int(symbol.section_name is models.SECTION_DEX)].append(symbol)
+
   symbols[0].sort(key=lambda s: s.full_name)
   symbols[1].sort(key=lambda s: s.full_name)
   symbols[0].extend(symbols[1])

tools/binary_size/libsupersize/apkanalyzer_test.py

@@ -93,6 +93,107 @@ def testUndoHierarchicalSizing_PackageAndClassSameName(self):
       ('C', 'name', 2),
     ], nodes)
 
+  def testLambdaNormalizer_wholeString(self):
+    lambda_normalizer = apkanalyzer.LambdaNormalizer()
+    name = 'org.-$$Lambda$StackAnimation$Nested1$kjevdDQ8V2zqCrdieLqWLHzk.dex'
+    package = name
+    expected_outer_class = 'org.StackAnimation'
+    expected_name = 'org.StackAnimation$Nested1$$Lambda$0'
+    self.assertEqual((expected_outer_class, expected_name),
+                     lambda_normalizer.Normalize(package, name))
+
+  def testLambdaNormalizer_prefix(self):
+    lambda_normalizer = apkanalyzer.LambdaNormalizer()
+    name = 'org.-$$Lambda$StackAnimation$Nested1$kjevdeLqWLHzk.dex foo bar'
+    package = name.split(' ')[0]
+    expected_outer_class = 'org.StackAnimation'
+    expected_name = 'org.StackAnimation$Nested1$$Lambda$0 foo bar'
+    self.assertEqual((expected_outer_class, expected_name),
+                     lambda_normalizer.Normalize(package, name))
+
+  def testLambdaNormalizer_lambdaCounting(self):
+    lambda_normalizer = apkanalyzer.LambdaNormalizer()
+    name = 'org.-$$Lambda$StackAnimation$Nested1$kjevdDQ8V2zqCrdieLqWLHzk.dex'
+    expected_outer_class = 'org.StackAnimation'
+    expected_name = 'org.StackAnimation$Nested1$$Lambda$0'
+    # Ensure multiple calls to the same class maps to same number.
+    self.assertEqual((expected_outer_class, expected_name),
+                     lambda_normalizer.Normalize(name, name))
+    self.assertEqual((expected_outer_class, expected_name),
+                     lambda_normalizer.Normalize(name, name))
+    name = 'org.-$$Lambda$StackAnimation$Nested1$kjevdDQ8V2zqCrdieLqWLHzk2.dex'
+    expected_name = 'org.StackAnimation$Nested1$$Lambda$1'
+    self.assertEqual((expected_outer_class, expected_name),
+                     lambda_normalizer.Normalize(name, name))
+
+  def testLambdaNormalizer_multiSameLine(self):
+    lambda_normalizer = apkanalyzer.LambdaNormalizer()
+    name = ('org.-$$Lambda$StackAnimation$Nested1$kevdDQ8V2zqCrdieLqWLHzk.dex '
+            'org.-$$Lambda$Other$kjevdDQ8V2zqCrdieLqWLHzk.dex.foo bar')
+    package = name.split(' ')[0]
+    expected_outer_class = 'org.StackAnimation'
+    expected_name = ('org.StackAnimation$Nested1$$Lambda$0 '
+                     'org.-$$Lambda$Other$kjevdDQ8V2zqCrdieLqWLHzk.dex.foo bar')
+    self.assertEqual((expected_outer_class, expected_name),
+                     lambda_normalizer.Normalize(package, name))
+
+    name = expected_name
+    package = name.split(' ')[1]
+    expected_outer_class = 'org.Other'
+    expected_name = ('org.StackAnimation$Nested1$$Lambda$0 '
+                     'org.Other$$Lambda$0.foo bar')
+    self.assertEqual((expected_outer_class, expected_name),
+                     lambda_normalizer.Normalize(package, name))
+
+  def testCreateDexSymbol_normal(self):
+    name = ('org.StackAnimation org.ChromeAnimation '
+            'createReachTopAnimatorSet(org.StackTab[],float)')
+    size = 1
+    source_map = {}
+    lambda_normalizer = apkanalyzer.LambdaNormalizer()
+    symbol = apkanalyzer.CreateDexSymbol(name, size, source_map,
+                                         lambda_normalizer)
+    self.assertEqual('$APK/org/StackAnimation', symbol.object_path)
+
+  def testCreateDexSymbol_classMerged_noSource(self):
+    name = ('org.NewClass org.ChromeAnimation '
+            'org.OldClass.createReachTopAnimatorSet(org.StackTab[],float)')
+    size = 1
+    source_map = {}
+    lambda_normalizer = apkanalyzer.LambdaNormalizer()
+    symbol = apkanalyzer.CreateDexSymbol(name, size, source_map,
+                                         lambda_normalizer)
+    self.assertEqual('$APK/org/OldClass', symbol.object_path)
+
+  def testCreateDexSymbol_classMerged_withSource(self):
+    name = ('org.NewClass org.ChromeAnimation '
+            'org.OldClass.createReachTopAnimatorSet(org.StackTab[],float)')
+    size = 1
+    source_map = {'org.OldClass': 'old_path.java'}
+    lambda_normalizer = apkanalyzer.LambdaNormalizer()
+    symbol = apkanalyzer.CreateDexSymbol(name, size, source_map,
+                                         lambda_normalizer)
+    self.assertEqual('$APK/org/OldClass', symbol.object_path)
+    self.assertEqual('old_path.java', symbol.source_path)
+
+  def testCreateDexSymbol_classMerged_field(self):
+    name = 'org.NewClass int org.OldClass.createReachTopAnimatorSet'
+    size = 1
+    source_map = {}
+    lambda_normalizer = apkanalyzer.LambdaNormalizer()
+    symbol = apkanalyzer.CreateDexSymbol(name, size, source_map,
+                                         lambda_normalizer)
+    self.assertEqual('$APK/org/OldClass', symbol.object_path)
+
+  def testCreateDexSymbol_total(self):
+    name = '<TOTAL>'
+    size = 1
+    source_map = {}
+    lambda_normalizer = apkanalyzer.LambdaNormalizer()
+    symbol = apkanalyzer.CreateDexSymbol(name, size, source_map,
+                                         lambda_normalizer)
+    self.assertIsNone(symbol)
+
 
 if __name__ == '__main__':
   unittest.main()

tools/binary_size/libsupersize/caspian/README.md

@@ -16,7 +16,7 @@ https://emscripten.org/docs/getting_started/downloads.html
 
 ```sh
 git apply -3 tools/binary_size/libsupersize/caspian/wasmbuild.patch
-gn gen out/caspian --args='is_official_build=true treat_warnings_as_errors=false fatal_linker_warnings=false'
+gn gen out/caspian --args='is_official_build=true treat_warnings_as_errors=false fatal_linker_warnings=false chrome_pgo_phase=0'
 ( cd out/caspian; autoninja tools/binary_size:caspian_web && cp wasm/caspian_web.* ../../tools/binary_size/libsupersize/static/ )
 ```
 

tools/binary_size/libsupersize/caspian/function_signature.cc

@@ -52,13 +52,13 @@ std::tuple<std::string_view, std::string_view, std::string_view> ParseJava(
   // elements can be arbitrary.
   std::string maybe_member_type;
   size_t hash_idx = full_name.find('#');
-  std::string_view full_class_name;
+  std::string_view full_new_class_name;
   std::string_view member;
   std::string_view member_type;
   if (hash_idx != std::string_view::npos) {
     // Parse an already parsed full_name.
     // Format: Class#symbol: type
-    full_class_name = full_name.substr(0, hash_idx);
+    full_new_class_name = full_name.substr(0, hash_idx);
     size_t colon_idx = full_name.find(':');
     member = Slice(full_name, hash_idx + 1, colon_idx);
     if (colon_idx != std::string_view::npos) {
@@ -67,7 +67,7 @@ std::tuple<std::string_view, std::string_view, std::string_view> ParseJava(
   } else {
     // Format: Class [returntype] functionName()
     std::vector<std::string_view> parts = SplitBy(full_name, ' ');
-    full_class_name = parts[0];
+    full_new_class_name = parts[0];
     if (parts.size() >= 2) {
       member = parts.back();
     }
@@ -77,23 +77,35 @@ std::tuple<std::string_view, std::string_view, std::string_view> ParseJava(
     }
   }
 
-  std::vector<std::string_view> split = SplitBy(full_class_name, '.');
-  std::string_view short_class_name = split.back();
-
   if (member.empty()) {
+    std::vector<std::string_view> split = SplitBy(full_new_class_name, '.');
+    std::string_view short_class_name = split.back();
     return std::make_tuple(full_name, full_name, short_class_name);
   }
-  owned_strings->push_back(std::string(full_class_name) + std::string("#") +
+  owned_strings->push_back(std::string(full_new_class_name) + std::string("#") +
                            std::string(member) + std::string(member_type));
   full_name = owned_strings->back();
 
   member = member.substr(0, member.find('('));
 
+  // Class merging.
+  std::string_view full_old_class_name = full_new_class_name;
+  size_t dot_idx = member.rfind('.');
+  if (dot_idx != std::string_view::npos) {
+    full_old_class_name = Slice(member, 0, dot_idx);
+    member = member.substr(dot_idx + 1);
+  }
+
+  std::string_view short_class_name = full_old_class_name;
+  dot_idx = full_old_class_name.rfind('.');
+  if (dot_idx != std::string_view::npos)
+    short_class_name = short_class_name.substr(dot_idx + 1);
+
   owned_strings->push_back(std::string(short_class_name) + std::string("#") +
                            std::string(member));
   std::string_view name = owned_strings->back();
 
-  owned_strings->push_back(std::string(full_class_name) + std::string("#") +
+  owned_strings->push_back(std::string(full_old_class_name) + std::string("#") +
                            std::string(member));
   std::string_view template_name = owned_strings->back();
 

tools/binary_size/libsupersize/caspian/function_signature_test.cc

@@ -124,6 +124,16 @@ TEST(AnalyzeTest, ParseJavaFunctionSignature) {
   // Java field
   do_test("org.ClassName some.Type mField", "org.ClassName#mField: some.Type",
           "org.ClassName#mField", "ClassName#mField");
+
+  // Class merging: Method
+  do_test("org.NewClass int org.OldClass.readShort(int,int)",
+          "org.NewClass#org.OldClass.readShort(int,int): int",
+          "org.OldClass#readShort", "OldClass#readShort");
+
+  // Class merging: Field
+  do_test("org.NewClass some.Type org.OldClass.mField",
+          "org.NewClass#org.OldClass.mField: some.Type", "org.OldClass#mField",
+          "OldClass#mField");
 }
 
 TEST(AnalyzeTest, ParseFunctionSignature) {