fe: minor change to benchmarks to match what the runners expect

pkg/front_end/tool/perf.dart

Index: pkg/front_end/tool/perf.dart
diff --git a/pkg/front_end/tool/perf.dart b/pkg/front_end/tool/perf.dart
index 7cd3a49126cf372e330c749a62a0dfbdf9d91f00..c994047d6fe6a564b73552df9c2256e2b8346b08 100644
--- a/pkg/front_end/tool/perf.dart
+++ b/pkg/front_end/tool/perf.dart
@@ -37,95 +37,61 @@ main(List<String> args) async {
     print('usage: perf.dart <bench-id> <entry.dart>');
     exit(1);
   }
-  var totalTimer = new Stopwatch()..start();
 
   var bench = args[0];
   var entryUri = Uri.base.resolve(args[1]);
 
   await setup(entryUri);
 
+  Set<Source> files = scanReachableFiles(entryUri);
   var handlers = {
-    'scan': () async {
-      Set<Source> files = scanReachableFiles(entryUri);
-      // TODO(sigmund): replace the warmup with instrumented snapshots.
-      for (int i = 0; i < 10; i++) scanFiles(files);
-    },
-    'parse': () async {
-      Set<Source> files = scanReachableFiles(entryUri);
-      // TODO(sigmund): replace the warmup with instrumented snapshots.
-      for (int i = 0; i < 10; i++) parseFiles(files);
-    },
+    'scan': () async => scanFiles(files),
+    'parse': () async => parseFiles(files),
     'kernel_gen_e2e': () async {
-      // TODO(sigmund): remove. This is used to compute the input size, we
-      // should extract input size from frontend instead.
-      scanReachableFiles(entryUri);
-      // TODO(sigmund): replace this warmup. Note that for very large programs,
-      // the GC pressure on the VM seems to make this worse with time (maybe we
-      // are leaking memory?). That's why we run it twice and not 10 times.
-      for (int i = 0; i < 2; i++) {
-        await generateKernel(entryUri, useSdkSummary: false);
-      }
+      await generateKernel(entryUri, useSdkSummary: false);
     },
     'kernel_gen_e2e_sum': () async {
-      // TODO(sigmund): remove. This is incorrect since it includes sizes for
-      // files that will not be loaded when using summaries. We need to extract
-      // input size from frontend instead.
-      scanReachableFiles(entryUri);
-      // TODO(sigmund): replace this warmup. Note that for very large programs,
-      // the GC pressure on the VM seems to make this worse with time (maybe we
-      // are leaking memory?). That's why we run it twice and not 10 times.
-      for (int i = 0; i < 2; i++) {
-        await generateKernel(entryUri, useSdkSummary: true, compileSdk: false);
-      }
-    },
-    'unlinked_summarize': () async {
-      Set<Source> files = scanReachableFiles(entryUri);
-      // TODO(sigmund): replace the warmup with instrumented snapshots.
-      for (int i = 0; i < 10; i++) unlinkedSummarizeFiles(files);
-    },
-    'prelinked_summarize': () async {
-      Set<Source> files = scanReachableFiles(entryUri);
-      // TODO(sigmund): replace the warmup with instrumented snapshots.
-      for (int i = 0; i < 10; i++) prelinkedSummarizeFiles(files);
+      await generateKernel(entryUri, useSdkSummary: true, compileSdk: false);
     },
-    'linked_summarize': () async {
-      Set<Source> files = scanReachableFiles(entryUri);
-      // TODO(sigmund): replace the warmup with instrumented snapshots.
-      for (int i = 0; i < 10; i++) linkedSummarizeFiles(files);
-    }
+    'unlinked_summarize': () async => summarize(files),
+    'prelinked_summarize': () async => summarize(files, prelink: true),
+    'linked_summarize': () async => summarize(files, link: true),
   };
 
   var handler = handlers[bench];
   if (handler == null) {
-    // TODO(sigmund): implement the remaining benchmarks.
     print('unsupported bench-id: $bench. Please specify one of the following: '
         '${handlers.keys.join(", ")}');
     exit(1);
   }
-  await handler();
 
-  totalTimer.stop();
-  report("total", totalTimer.elapsedMicroseconds);
+  // TODO(sigmund): replace the warmup with instrumented snapshots.
+  int iterations = bench.contains('kernel_gen') ? 2 : 10;
+  for (int i = 0; i < iterations; i++) {
+    var totalTimer = new Stopwatch()..start();
+    print('== iteration $i');
+    await handler();
+    totalTimer.stop();
+    report("total", totalTimer.elapsedMicroseconds);
+  }
+
 }
 
 /// Cumulative time spent parsing.
 Stopwatch parseTimer = new Stopwatch();
 
-/// Cumulative time spent prelinking summaries.
-Stopwatch prelinkSummaryTimer = new Stopwatch();
+/// Cumulative time spent building unlinked summaries.
+Stopwatch unlinkedSummarizeTimer = new Stopwatch();
 
 /// Cumulative time spent scanning.
 Stopwatch scanTimer = new Stopwatch();
 
-/// Cumulative total number of chars scanned.
-int scanTotalChars = 0;
+/// Size of all sources.
+int inputSize = 0;
 
 /// Factory to load and resolve app, packages, and sdk sources.
 SourceFactory sources;
 
-/// Cumulative time spent building unlinked summaries.
-Stopwatch unlinkedSummarizeTimer = new Stopwatch();
-
 /// Add to [files] all sources reachable from [start].
 void collectSources(Source start, Set<Source> files) {
   if (!files.add(start)) return;
@@ -140,6 +106,10 @@ void collectSources(Source start, Set<Source> files) {
 
 Future<Program> generateKernel(Uri entryUri,
     {bool useSdkSummary: false, bool compileSdk: true}) async {
+  // TODO(sigmund): this is here only to compute the input size,
+  // we should extract the input size from the frontend instead.
+  scanReachableFiles(entryUri);
+
   var dartkTimer = new Stopwatch()..start();
   // TODO(sigmund): add a constructor with named args to compiler options.
   var options = new CompilerOptions()
@@ -163,9 +133,9 @@ Future<Program> generateKernel(Uri entryUri,
 }
 
 /// Generates unlinkmed summaries for all files in [files], and returns them in
-/// an [UnlinkedSummaries] container.
-UnlinkedSummaries generateUnlinkedSummaries(Set<Source> files) {
-  var unlinkedSummaries = new UnlinkedSummaries();
+/// an [_UnlinkedSummaries] container.
+_UnlinkedSummaries generateUnlinkedSummaries(Set<Source> files) {
+  var unlinkedSummaries = new _UnlinkedSummaries();
   for (var source in files) {
     unlinkedSummaries.summariesByUri[source.uri.toString()] =
         unlinkedSummarize(source);
@@ -175,42 +145,40 @@ UnlinkedSummaries generateUnlinkedSummaries(Set<Source> files) {
 
 /// Produces linked summaries for every file in [files] and reports the time

[Paul Berry, 2017/01/03 17:58:34]

Doc comment is no longer accurate.

[Siggi Cherem (dart-lang), 2017/01/04 00:01:04]

Done.

 /// spent doing so.
-void linkedSummarizeFiles(Set<Source> files) {
-  // The code below will record again how many chars are scanned and how long it
-  // takes to scan them, even though we already did so in [scanReachableFiles].
-  // Recording and reporting this twice is unnecessary, but we do so for now to
-  // validate that the results are consistent.
+void summarize(Set<Source> files, {bool prelink: false, bool link: false}) {
   scanTimer = new Stopwatch();
-  var old = scanTotalChars;
-  scanTotalChars = 0;
   parseTimer = new Stopwatch();
   unlinkedSummarizeTimer = new Stopwatch();
   var unlinkedSummaries = generateUnlinkedSummaries(files);
-  prelinkSummaryTimer = new Stopwatch();
-  Map<String, LinkedLibraryBuilder> prelinkedLibraries =
-      prelinkSummaries(files, unlinkedSummaries);
-  var linkTimer = new Stopwatch()..start();
-  LinkedLibrary getDependency(String uri) {
-    // getDependency should never be called because all dependencies are present
-    // in [prelinkedLibraries].
-    print('Warning: getDependency called for: $uri');
-    return null;
-  }
-
-  bool strong = true;
-  relink(prelinkedLibraries, getDependency, unlinkedSummaries.getUnit, strong);
-  linkTimer.stop();
-
-  if (old != scanTotalChars) print('input size changed? ${old} chars');
   report("scan", scanTimer.elapsedMicroseconds);
   report("parse", parseTimer.elapsedMicroseconds);
-  report('unlinked summarize', unlinkedSummarizeTimer.elapsedMicroseconds);
+  report('unlink extract', unlinkedSummarizeTimer.elapsedMicroseconds);

[Paul Berry, 2017/01/03 17:58:34]

I don't really have strong feelings about nomenclature here, but can you help me
understand why "extract" is a meaningful name for this benchmark?

[Siggi Cherem (dart-lang), 2017/01/04 00:01:04]

In all honestly, I'm not convinced about "extract" either. The main requirement
I have is that the runner relies on matching the benchmark name (which is an
argument to this script) and the name given to `report`. It's ok to report extra
data, and the runner will ignore it. I believe we will mostly use the data of
"unlinked summarize + parse" as a benchmark, which is why I made that be the one
with the benchmark name "unlinked_summarize" and gave this a different name.

Some ideas:
 (a) do not report the "extract" result at all
 (b) create 2 benchmarks:
    - unlinked_summarize: summarize alone (like you had before my change, aka
"extract")
    - unlinked_summarize_from_source: includes parse time (and maybe scan time
too?)
 (c) change the runner script to aggregate data itself. 

It is much easier to maintain things if we don't do (c). I'm totally fine with
either (a) or (b).

[Paul Berry, 2017/01/04 13:55:43]

In that case I'd prefer (b), on the theory that tracking more information is
better than less :)

   report(
-      'unlinked summarize + parse',
+      'unlinked_summarize',
       unlinkedSummarizeTimer.elapsedMicroseconds +
           parseTimer.elapsedMicroseconds);
-  report('prelink', prelinkSummaryTimer.elapsedMicroseconds);
-  report('link', linkTimer.elapsedMicroseconds);
+
+  if (prelink || link) {
+    var prelinkTimer = new Stopwatch()..start();
+    var prelinkedLibraries = prelinkSummaries(files, unlinkedSummaries);
+    prelinkTimer.stop();
+    report('prelinked_summarize', prelinkTimer.elapsedMicroseconds);
+
+    if (link) {
+      var linkTimer = new Stopwatch()..start();
+      LinkedLibrary getDependency(String uri) {
+        // getDependency should never be called because all dependencies are
+        // present in [prelinkedLibraries].
+        print('Warning: getDependency called for: $uri');
+        return null;
+      }
+
+      relink(prelinkedLibraries, getDependency, unlinkedSummaries.getUnit,
+          true /*strong*/);
+      linkTimer.stop();
+      report('linked_summarize', linkTimer.elapsedMicroseconds);
+    }
+  }
 }
 
 /// Uses the diet-parser to parse only directives in [source].
@@ -222,20 +190,12 @@ CompilationUnit parseDirectives(Source source) {
 
 /// Parses every file in [files] and reports the time spent doing so.
 void parseFiles(Set<Source> files) {
-  // The code below will record again how many chars are scanned and how long it
-  // takes to scan them, even though we already did so in [scanReachableFiles].
-  // Recording and reporting this twice is unnecessary, but we do so for now to
-  // validate that the results are consistent.
   scanTimer = new Stopwatch();
-  var old = scanTotalChars;
-  scanTotalChars = 0;
   parseTimer = new Stopwatch();
   for (var source in files) {
     parseFull(source);
   }
 
-  // Report size and scanning time again. See discussion above.
-  if (old != scanTotalChars) print('input size changed? ${old} chars');
   report("scan", scanTimer.elapsedMicroseconds);
   report("parse", parseTimer.elapsedMicroseconds);
 }
@@ -250,74 +210,40 @@ CompilationUnit parseFull(Source source) {
   return unit;
 }
 
-/// Produces prelinked summaries for every file in [files] and reports the time
-/// spent doing so.
-void prelinkedSummarizeFiles(Set<Source> files) {
-  // The code below will record again how many chars are scanned and how long it
-  // takes to scan them, even though we already did so in [scanReachableFiles].
-  // Recording and reporting this twice is unnecessary, but we do so for now to
-  // validate that the results are consistent.
-  scanTimer = new Stopwatch();
-  var old = scanTotalChars;
-  scanTotalChars = 0;
-  parseTimer = new Stopwatch();
-  unlinkedSummarizeTimer = new Stopwatch();
-  var unlinkedSummaries = generateUnlinkedSummaries(files);
-  prelinkSummaryTimer = new Stopwatch();
-  prelinkSummaries(files, unlinkedSummaries);
-
-  if (old != scanTotalChars) print('input size changed? ${old} chars');
-  report("scan", scanTimer.elapsedMicroseconds);
-  report("parse", parseTimer.elapsedMicroseconds);
-  report('unlinked summarize', unlinkedSummarizeTimer.elapsedMicroseconds);
-  report(
-      'unlinked summarize + parse',
-      unlinkedSummarizeTimer.elapsedMicroseconds +
-          parseTimer.elapsedMicroseconds);
-  report('prelink', prelinkSummaryTimer.elapsedMicroseconds);
-}
-
 /// Prelinks all the summaries for [files], using [unlinkedSummaries] to obtain
 /// their unlinked summaries.
 ///
 /// The return value is suitable for passing to the summary linker.
 Map<String, LinkedLibraryBuilder> prelinkSummaries(
-    Set<Source> files, UnlinkedSummaries unlinkedSummaries) {
-  prelinkSummaryTimer.start();
-  Set<String> libraryUris =
-      files.map((source) => source.uri.toString()).toSet();
+    Set<Source> files, _UnlinkedSummaries unlinkedSummaries) {
+  Set<String> libraryUris = files.map((source) => '${source.uri}').toSet();
 
   String getDeclaredVariable(String s) => null;
   var prelinkedLibraries =
       setupForLink(libraryUris, unlinkedSummaries.getUnit, getDeclaredVariable);
-  prelinkSummaryTimer.stop();
   return prelinkedLibraries;
 }
 
 /// Report that metric [name] took [time] micro-seconds to process
-/// [scanTotalChars] characters.
+/// [inputSize] characters.
 void report(String name, int time) {
   var sb = new StringBuffer();
-  sb.write('$name: $time us, ${time ~/ 1000} ms');
-  sb.write(', ${scanTotalChars * 1000 ~/ time} chars/ms');
+  var padding = " " * (20 - name.length);
+  sb.write('$name:$padding $time us, ${time ~/ 1000} ms');
+  sb.write(', ${time * 1000 ~/ inputSize} ns/char');
   print('$sb');
 }
 
 /// Scans every file in [files] and reports the time spent doing so.
 void scanFiles(Set<Source> files) {
-  // The code below will record again how many chars are scanned and how long it
-  // takes to scan them, even though we already did so in [scanReachableFiles].
-  // Recording and reporting this twice is unnecessary, but we do so for now to
-  // validate that the results are consistent.
+  // `tokenize` records how many chars are scanned and how long it takes to scan
+  // them. As this function is called repeatedly when running as a benchmark, we
+  // make sure to clear the data and compute it again every time.
   scanTimer = new Stopwatch();
-  var old = scanTotalChars;
-  scanTotalChars = 0;
   for (var source in files) {
     tokenize(source);
   }
 
-  // Report size and scanning time again. See discussion above.
-  if (old != scanTotalChars) print('input size changed? ${old} chars');
   report("scan", scanTimer.elapsedMicroseconds);
 }
 
@@ -348,7 +274,8 @@ Set<Source> scanReachableFiles(Uri entryUri) {
 
   loadTimer.stop();
 
-  print('input size: ${scanTotalChars} chars');
+  for (var s in files) inputSize += s.contents.data.length;
+  print('input size: ${inputSize} chars');
   var loadTime = loadTimer.elapsedMicroseconds - scanTimer.elapsedMicroseconds;
   report("load", loadTime);
   report("scan", scanTimer.elapsedMicroseconds);
@@ -372,11 +299,9 @@ Future setup(Uri entryUri) async {
 /// Scan [source] and return the first token produced by the scanner.
 Token tokenize(Source source) {
   scanTimer.start();
-  var contents = source.contents.data;
-  scanTotalChars += contents.length;
   // TODO(sigmund): is there a way to scan from a random-access-file without
   // first converting to String?
-  var scanner = new _Scanner(contents);
+  var scanner = new _Scanner(source.contents.data);
   var token = scanner.tokenize();
   scanTimer.stop();
   return token;
@@ -390,32 +315,8 @@ UnlinkedUnitBuilder unlinkedSummarize(Source source) {
   return unlinkedUnit;
 }
 
-/// Produces unlinked summaries for every file in [files] and reports the time
-/// spent doing so.
-void unlinkedSummarizeFiles(Set<Source> files) {
-  // The code below will record again how many chars are scanned and how long it
-  // takes to scan them, even though we already did so in [scanReachableFiles].
-  // Recording and reporting this twice is unnecessary, but we do so for now to
-  // validate that the results are consistent.
-  scanTimer = new Stopwatch();
-  var old = scanTotalChars;
-  scanTotalChars = 0;
-  parseTimer = new Stopwatch();
-  unlinkedSummarizeTimer = new Stopwatch();
-  generateUnlinkedSummaries(files);
-
-  if (old != scanTotalChars) print('input size changed? ${old} chars');
-  report("scan", scanTimer.elapsedMicroseconds);
-  report("parse", parseTimer.elapsedMicroseconds);
-  report('unlinked summarize', unlinkedSummarizeTimer.elapsedMicroseconds);
-  report(
-      'unlinked summarize + parse',
-      unlinkedSummarizeTimer.elapsedMicroseconds +
-          parseTimer.elapsedMicroseconds);
-}
-
 /// Simple container for a mapping from URI string to an unlinked summary.
-class UnlinkedSummaries {
+class _UnlinkedSummaries {
   final summariesByUri = <String, UnlinkedUnit>{};
 
   /// Get the unlinked summary for the given URI, and report a warning if it