[vm/tool] Improvements to snapshot_analysis 'compare' command.

Introduce helper classes and methods for constructing program
hierarchy from a flat list of symbols as well as methods for
diffing and bucketing the program hierarchies.

Issue #41249

Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-try,pkg-win-release-try,pkg-mac-release-try
Change-Id: Ieb6ad1ccd8bc7fa1181d31e78ac351369ede5f81
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/150101
Commit-Queue: Vyacheslav Egorov <[email protected]>
Reviewed-by: Alexander Markov <[email protected]>

Author: mraleph

pkg/vm/lib/snapshot/compare.dart

@@ -7,12 +7,14 @@
 /// and which symbols decreased in size.
 library vm.snapshot.compare;
 
-import 'dart:convert';
 import 'dart:io';
 import 'dart:math' as math;
 
 import 'package:args/command_runner.dart';
 
+import 'package:vm/snapshot/instruction_sizes.dart';
+import 'package:vm/snapshot/program_info.dart';
+
 class CompareCommand extends Command<void> {
   @override
   final String name = 'compare';
@@ -32,10 +34,26 @@ Use --narrow flag to limit column widths.''';
       super.invocation.replaceAll('[arguments]', '<old.json> <new.json>');
 
   CompareCommand() {
-    argParser.addOption('column-width',
-        help: 'Truncate column content to the given width'
-            ' (${AsciiTable.unlimitedWidth} means do not truncate).',
-        defaultsTo: AsciiTable.unlimitedWidth.toString());
+    argParser
+      ..addOption('column-width',
+          help: 'Truncate column content to the given width'
+              ' (${AsciiTable.unlimitedWidth} means do not truncate).',
+          defaultsTo: AsciiTable.unlimitedWidth.toString())
+      ..addOption('granularity',
+          help: 'Choose the granularity of the output.',
+          allowed: ['method', 'class', 'library', 'package'],
+          defaultsTo: 'method')
+      ..addFlag('collapse-anonymous-closures', help: '''
+Collapse all anonymous closures from the same scope into a single entry.
+When comparing size of AOT snapshots for two different versions of a
+program there is no reliable way to precisely establish which two anonymous
+closures are the same and should be compared in size - so
+comparison might produce a noisy output. This option reduces confusion
+by collapsing different anonymous closures within the same scope into a
+single entry. Note that when comparing the same application compiled
+with two different versions of an AOT compiler closures can be distinguished
+precisely based on their source position (which is included in their name).
+''');
   }
 
   @override
@@ -53,110 +71,103 @@ Use --narrow flag to limit column widths.''';
 
     final oldJsonPath = _checkExists(argResults.rest[0]);
     final newJsonPath = _checkExists(argResults.rest[1]);
-    printComparison(oldJsonPath, newJsonPath, maxWidth: maxWidth);
+    printComparison(oldJsonPath, newJsonPath,
+        maxWidth: maxWidth,
+        granularity: _parseHistogramType(argResults['granularity']),
+        collapseAnonymousClosures: argResults['collapse-anonymous-closures']);
+  }
+
+  HistogramType _parseHistogramType(String value) {
+    switch (value) {
+      case 'method':
+        return HistogramType.bySymbol;
+      case 'class':
+        return HistogramType.byClass;
+      case 'library':
+        return HistogramType.byLibrary;
+      case 'package':
+        return HistogramType.byPackage;
+    }
   }
 
-  String _checkExists(String path) {
-    if (!File(path).existsSync()) {
+  File _checkExists(String path) {
+    final file = File(path);
+    if (!file.existsSync()) {
       usageException('File $path does not exist!');
     }
-    return path;
+    return file;
   }
 }
 
-void printComparison(String oldJsonPath, String newJsonPath,
-    {int maxWidth: 0}) {
-  final oldSizes = loadSymbolSizes(oldJsonPath);
-  final newSizes = loadSymbolSizes(newJsonPath);
-
+void printComparison(File oldJson, File newJson,
+    {int maxWidth: 0,
+    bool collapseAnonymousClosures = false,
+    HistogramType granularity = HistogramType.bySymbol}) async {
+  final oldSizes = await loadProgramInfo(oldJson,
+      collapseAnonymousClosures: collapseAnonymousClosures);
+  final newSizes = await loadProgramInfo(newJson,
+      collapseAnonymousClosures: collapseAnonymousClosures);
+  final diff = computeDiff(oldSizes, newSizes);
+
+  // Compute total sizes.
   var totalOld = 0;
+  oldSizes.visit((_, __, ___, size) {
+    totalOld += size;
+  });
+
   var totalNew = 0;
+  newSizes.visit((_, __, ___, size) {
+    totalNew += size;
+  });
+
   var totalDiff = 0;
-  final diffBySymbol = <String, int>{};
-
-  // Process all symbols (from both old and new results) and compute the change
-  // in size. If symbol is not present in the compilation assume its size to be
-  // zero.
-  for (var key in Set<String>()..addAll(newSizes.keys)..addAll(oldSizes.keys)) {
-    final oldSize = oldSizes[key] ?? 0;
-    final newSize = newSizes[key] ?? 0;
-    final diff = newSize - oldSize;
-    if (diff != 0) diffBySymbol[key] = diff;
-    totalOld += oldSize;
-    totalNew += newSize;
-    totalDiff += diff;
-  }
+  diff.visit((_, __, ___, size) {
+    totalDiff += size.inBytes;
+  });
 
-  // Compute the list of changed symbols sorted by difference (descending).
-  final changedSymbolsBySize = diffBySymbol.keys.toList();
-  changedSymbolsBySize.sort((a, b) => diffBySymbol[b] - diffBySymbol[a]);
+  // Compute histogram.
+  final histogram = SizesHistogram.from<SymbolDiff>(
+      diff, (diff) => diff.inBytes, granularity);
 
   // Now produce the report table.
   const numLargerSymbolsToReport = 30;
   const numSmallerSymbolsToReport = 10;
   final table = AsciiTable(header: [
-    Text.left('Library'),
-    Text.left('Method'),
+    for (var col in histogram.bucketing.nameComponents) Text.left(col),
     Text.right('Diff (Bytes)')
   ], maxWidth: maxWidth);
 
   // Report [numLargerSymbolsToReport] symbols that increased in size most.
-  for (var key in changedSymbolsBySize
-      .where((k) => diffBySymbol[k] > 0)
+  for (var key in histogram.bySize
+      .where((k) => histogram.buckets[k] > 0)
       .take(numLargerSymbolsToReport)) {
-    final name = key.split(librarySeparator);
-    table.addRow([name[0], name[1], '+${diffBySymbol[key]}']);
+    table.addRow([
+      ...histogram.bucketing.namesFromBucket(key),
+      '+${histogram.buckets[key]}'
+    ]);
   }
   table.addSeparator(Separator.Wave);
 
   // Report [numSmallerSymbolsToReport] symbols that decreased in size most.
-  for (var key in changedSymbolsBySize.reversed
-      .where((k) => diffBySymbol[k] < 0)
+  for (var key in histogram.bySize.reversed
+      .where((k) => histogram.buckets[k] < 0)
       .take(numSmallerSymbolsToReport)
       .toList()
       .reversed) {
-    final name = key.split(librarySeparator);
-    table.addRow([name[0], name[1], '${diffBySymbol[key]}']);
+    table.addRow([
+      ...histogram.bucketing.namesFromBucket(key),
+      '${histogram.buckets[key]}'
+    ]);
   }
   table.addSeparator();
 
   table.render();
-  print('Comparing ${oldJsonPath} (old) to ${newJsonPath} (new)');
+  print('Comparing ${oldJson.path} (old) to ${newJson.path} (new)');
   print('Old   : ${totalOld} bytes.');
   print('New   : ${totalNew} bytes.');
   print('Change: ${totalDiff > 0 ? '+' : ''}${totalDiff} bytes.');
 }
 
-/// A combination of characters that is unlikely to occur in the symbol name.
-const String librarySeparator = ',';
-
-/// Load --print-instructions-sizes-to output as a mapping from symbol names
-/// to their sizes.
-///
-/// Note: we produce a single symbol name from function name and library name
-/// by concatenating them with [librarySeparator].
-Map<String, int> loadSymbolSizes(String name) {
-  final symbols = jsonDecode(File(name).readAsStringSync());
-  final result = new Map<String, int>();
-  final regexp = new RegExp(r"0x[a-fA-F0-9]+");
-  for (int i = 0, n = symbols.length; i < n; i++) {
-    final e = symbols[i];
-    // Obtain a key by combining library and method name. Strip anything
-    // after the library separator to make sure we can easily decode later.
-    // For method names, also remove non-deterministic parts to avoid
-    // reporting non-existing differences against the same layout.
-    String lib = ((e['l'] ?? '').split(librarySeparator))[0];
-    String name = (e['n'].split(librarySeparator))[0]
-        .replaceAll('[Optimized] ', '')
-        .replaceAll(regexp, '');
-    String key = lib + librarySeparator + name;
-    int val = e['s'];
-    result[key] =
-        (result[key] ?? 0) + val; // add (key,val), accumulate if exists
-  }
-  return result;
-}
-
 /// A row in the [AsciiTable].
 abstract class Row {
   String render(List<int> widths, List<AlignmentDirection> alignments);

pkg/vm/lib/snapshot/instruction_sizes.dart

@@ -9,6 +9,7 @@ import 'dart:convert';
 import 'dart:io';
 
 import 'package:vm/snapshot/name.dart';
+import 'package:vm/snapshot/program_info.dart';
 
 /// Parse the output of `--print-instructions-sizes-to` saved in the given
 /// file [input].
@@ -24,6 +25,22 @@ Future<List<SymbolInfo>> load(File input) async {
       .toList(growable: false);
 }
 
+/// Parse the output of `--print-instructions-sizes-to` saved in the given
+/// file [input] into [ProgramInfo<int>] structure representing the sizes
+/// of individual functions.
+///
+/// If [collapseAnonymousClosures] is set to [true] then all anonymous closures
+/// within the same scopes are collapsed together. Collapsing closures is
+/// helpful when comparing symbol sizes between two versions of the same
+/// program because in general there is no reliable way to recognize the same
+/// anonymous closures into two independent compilations.
+Future<ProgramInfo<int>> loadProgramInfo(File input,
+    {bool collapseAnonymousClosures = false}) async {
+  final symbols = await load(input);
+  return toProgramInfo(symbols,
+      collapseAnonymousClosures: collapseAnonymousClosures);
+}
+
 /// Information about the size of the instruction object.
 class SymbolInfo {
   /// Name of the code object (`Code::QualifiedName`) owning these instructions.
@@ -51,3 +68,43 @@ class SymbolInfo {
         size: map['s']);
   }
 }
+
+/// Restore hierarchical [ProgramInfo<int>] representation from the list of
+/// symbols by parsing function names.
+///
+/// If [collapseAnonymousClosures] is set to [true] then all anonymous closures
+/// within the same scopes are collapsed together. Collapsing closures is
+/// helpful when comparing symbol sizes between two versions of the same
+/// program because in general there is no reliable way to recognize the same
+/// anonymous closures into two independent compilations.
+ProgramInfo<int> toProgramInfo(List<SymbolInfo> symbols,
+    {bool collapseAnonymousClosures = false}) {
+  final program = ProgramInfo<int>();
+  for (var sym in symbols) {
+    final scrubbed = sym.name.scrubbed;
+    if (sym.libraryUri == null) {
+      assert(sym.name.isStub);
+      assert(
+          !program.stubs.containsKey(scrubbed) || sym.name.isTypeTestingStub);
+      program.stubs[scrubbed] = (program.stubs[scrubbed] ?? 0) + sym.size;
+    } else {
+      // Split the name into components (names of individual functions).
+      final path = sym.name.components;
+
+      final lib =
+          program.libraries.putIfAbsent(sym.libraryUri, () => LibraryInfo());
+      final cls = lib.classes.putIfAbsent(sym.className, () => ClassInfo());
+      var fun = cls.functions.putIfAbsent(path.first, () => FunctionInfo());
+      for (var name in path.skip(1)) {
+        if (collapseAnonymousClosures &&
+            name.startsWith('<anonymous closure @')) {
+          name = '<anonymous closure>';
+        }
+        fun = fun.closures.putIfAbsent(name, () => FunctionInfo());
+      }
+      fun.info = (fun.info ?? 0) + sym.size;
+    }
+  }
+
+  return program;
+}

pkg/vm/lib/snapshot/name.dart

@@ -15,20 +15,45 @@ class Name {
   /// Raw textual representation of the name as it occurred in the output
   /// of the AOT compiler.
   final String raw;
+  String _scrubbed;
 
   Name(this.raw);
 
   /// Pretty version of the name, with some of the irrelevant information
   /// removed from it.
+  ///
   /// Note: we still expect this name to be unique within compilation,
   /// so we are not removing any details that are used for disambiguation.
-  String get scrubbed => raw.replaceAll(_scrubbingRe, '');
+  /// The only exception are type testing stubs, these refer to type names
+  /// and types names are not bound to be unique between compilations.
+  String get scrubbed => _scrubbed ??=
+      raw.replaceAll(isStub ? _stubScrubbingRe : _scrubbingRe, '');
 
   /// Returns true if this name refers to a stub.
   bool get isStub => raw.startsWith('[Stub] ');
 
   /// Returns true if this name refers to an allocation stub.
   bool get isAllocationStub => raw.startsWith('[Stub] Allocate ');
+
+  /// Returns ture if this name refers to a type testing stub.
+  bool get isTypeTestingStub => raw.startsWith('[Stub] Type Test ');
+
+  /// Split this name into individual '.' separated components (e.g. names of
+  /// its parent functions).
+  List<String> get components {
+    // Break the rest of the name into components.
+    final result = scrubbed.split('.');
+
+    // Constructor names look like this 'new <ClassName>.<CtorName>' so
+    // we need to concatenate the first two components back to form
+    // the constructor name.
+    if (result.first.startsWith('new ')) {
+      result[0] = '${result[0]}${result[1]}';
+      result.removeAt(1);
+    }
+
+    return result;
+  }
 }
 
 // Remove useless prefixes and private library suffixes from the raw name.
@@ -37,3 +62,7 @@ class Name {
 // still, these names are formatted as '<anonymous closure @\d+>'.
 final _scrubbingRe =
     RegExp(r'\[(Optimized|Unoptimized|Stub)\]\s*|@\d+(?![>\d])');
+
+// Remove useless prefixes and private library suffixes from the raw name
+// for stubs.
+final _stubScrubbingRe = RegExp(r'\[Stub\]\s*|@\d+|\(H[a-f\d]+\) ');