diagnose_bloat.py: Grouped diffs.

tools/binary_size/diagnose_bloat.py

Index: tools/binary_size/diagnose_bloat.py
diff --git a/tools/binary_size/diagnose_bloat.py b/tools/binary_size/diagnose_bloat.py
index 17a49b3935d66e9192c07d65c8b0dd5cedc2ab50..6c1fc377230be43c1a729e34827a7e9fcc117a22 100755
--- a/tools/binary_size/diagnose_bloat.py
+++ b/tools/binary_size/diagnose_bloat.py
@@ -29,7 +29,6 @@ import zipfile
 
 _COMMIT_COUNT_WARN_THRESHOLD = 15
 _ALLOWED_CONSECUTIVE_FAILURES = 2
-_DIFF_DETAILS_LINES_THRESHOLD = 100
 _SRC_ROOT = os.path.abspath(
     os.path.join(os.path.dirname(__file__), os.pardir, os.pardir))
 _DEFAULT_ARCHIVE_DIR = os.path.join(_SRC_ROOT, 'out', 'binary-size-results')
@@ -50,10 +49,11 @@ class BaseDiff(object):
   def AppendResults(self, logfile):
     """Print and write diff results to an open |logfile|."""
     _PrintAndWriteToFile(logfile, self.banner)
-    _PrintAndWriteToFile(logfile, 'Summary:')
-    _PrintAndWriteToFile(logfile, self.Summary())
-    _PrintAndWriteToFile(logfile, '\nDetails:')
-    _PrintAndWriteToFile(logfile, self.DetailedResults())
+    for s in self.Summary():
+      print s
+    print
+    for s in self.DetailedResults():
+      logfile.write(s + '\n')
 
   @property
   def summary_stat(self):
@@ -78,7 +78,6 @@ class BaseDiff(object):
 
 
 class NativeDiff(BaseDiff):
-  _RE_SUMMARY = re.compile(r'Section Sizes .*?\n\n.*?(?=\n\n)', flags=re.DOTALL)
   _RE_SUMMARY_STAT = re.compile(
       r'Section Sizes \(Total=(?P<value>\d+) (?P<units>\w+)\)')
   _SUMMARY_STAT_NAME = 'Native Library Delta'
@@ -101,7 +100,7 @@ class NativeDiff(BaseDiff):
     return self._diff.splitlines()
 
   def Summary(self):
-    return NativeDiff._RE_SUMMARY.search(self._diff).group()
+    return self.DetailedResults()[:100]
 
   def ProduceDiff(self, before_dir, after_dir):
     before_size = os.path.join(before_dir, self._size_name)
@@ -113,6 +112,9 @@ class NativeDiff(BaseDiff):
 class ResourceSizesDiff(BaseDiff):
   _RESOURCE_SIZES_PATH = os.path.join(
       _SRC_ROOT, 'build', 'android', 'resource_sizes.py')
+  _SUMMARY_SECTIONS = ('Breakdown', 'Specifics')
+  _AGGREGATE_SECTIONS = (
+      'InstallBreakdown', 'Breakdown', 'MainLibInfo', 'Uncompressed')
 
   def __init__(self, apk_name, slow_options=False):
     self._apk_name = apk_name
@@ -122,23 +124,24 @@ class ResourceSizesDiff(BaseDiff):
 
   @property
   def summary_stat(self):
-    for s in self._diff:
-      if 'normalized' in s.name:
-        return s
+    for section_name, results in self._diff.iteritems():
+      for sub_section_name, value, units in results:
+        if 'normalized' in sub_section_name:
+          full_name = '{} {}'.format(section_name, sub_section_name)
+          return _DiffResult(full_name, value, units)
     return None
 
   def DetailedResults(self):
-    return ['{:>+10,} {} {}'.format(value, units, name)
-            for name, value, units in self._diff]
+    return self._ResultLines()
 
   def Summary(self):
-    return 'Normalized APK size: {:+,} {}'.format(
-        self.summary_stat.value, self.summary_stat.units)
+    return self._ResultLines(
+        include_sections=ResourceSizesDiff._SUMMARY_SECTIONS)
 
   def ProduceDiff(self, before_dir, after_dir):
     before = self._RunResourceSizes(before_dir)
     after = self._RunResourceSizes(after_dir)
-    diff = []
+    self._diff = collections.defaultdict(list)
     for section, section_dict in after.iteritems():
       for subsection, v in section_dict.iteritems():
         # Ignore entries when resource_sizes.py chartjson format has changed.
@@ -149,12 +152,33 @@ class ResourceSizesDiff(BaseDiff):
               'Found differing dict structures for resource_sizes.py, '
               'skipping %s %s', section, subsection)
         else:
-          diff.append(
-              _DiffResult(
-                  '%s %s' % (section, subsection),
-                  v['value'] - before[section][subsection]['value'],
-                  v['units']))
-    self._diff = sorted(diff, key=lambda x: abs(x.value), reverse=True)
+          self._diff[section].append(_DiffResult(
+              subsection,
+              v['value'] - before[section][subsection]['value'],
+              v['units']))
+
+  def _ResultLines(self, include_sections=None):
+    ret = []
+    for section_name, section_results in self._diff.iteritems():
+      section_no_target = re.sub(r'^.*_', '', section_name)
+      if not include_sections or section_no_target in include_sections:
+        sub_section_lines = []
+        section_sum = 0
+        units = ''
+        for name, value, units in section_results:
+          if value == 0 and include_sections:
+            continue
+          section_sum += value
+          sub_section_lines.append('{:>+10,} {} {}'.format(value, units, name))
+        section_header = section_name
+        if section_no_target in ResourceSizesDiff._AGGREGATE_SECTIONS:
+          section_header += ' ({:+,} {})'.format(section_sum, units)
+        if sub_section_lines:
+          ret.append(section_header)
+          ret.extend(sub_section_lines)
+    if not ret:
+      ret = ['Empty ' + self.name]
+    return ret
 
   def _RunResourceSizes(self, archive_dir):
     apk_path = os.path.join(archive_dir, self._apk_name)
@@ -662,10 +686,8 @@ def _PrintAndWriteToFile(logfile, s, *args, **kwargs):
     print s
     logfile.write('%s\n' % s)
   else:
-    for l in s[:_DIFF_DETAILS_LINES_THRESHOLD]:
+    for l in s:
       print l
-    if len(s) > _DIFF_DETAILS_LINES_THRESHOLD:
-      print '\nOutput truncated, see %s for more.' % logfile.name
     logfile.write('\n'.join(s))

[agrieve, 2017/06/23 02:40:30]

nit: might be simpler to do:

if isinstance(s, basestring):
  data = s.format(*args, **kwargs) + '\n'
else:
  data = '\n'.join(s) + '\n'
sys.stdout.write(data)
logfile.write(data)


That way you'll ensure to write the same thing to the log as you do to stdout
(right now logfile.write is missing a trailing \n in the 2nd case I think.

[estevenson, 2017/06/23 17:23:27]

Done.