Start actually adding incrementality into incremental kernel generator.

pkg/front_end/lib/src/incremental_kernel_generator_impl.dart

Index: pkg/front_end/lib/src/incremental_kernel_generator_impl.dart
diff --git a/pkg/front_end/lib/src/incremental_kernel_generator_impl.dart b/pkg/front_end/lib/src/incremental_kernel_generator_impl.dart
index f2bdc118182ad52af5c5062c38a4186cf9add2e7..ad86ecad33a76270ed55117786b3ca8a4e3f4f45 100644
--- a/pkg/front_end/lib/src/incremental_kernel_generator_impl.dart
+++ b/pkg/front_end/lib/src/incremental_kernel_generator_impl.dart
@@ -3,24 +3,39 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:io';
 
 import 'package:front_end/file_system.dart';
 import 'package:front_end/incremental_kernel_generator.dart';
 import 'package:front_end/incremental_resolved_ast_generator.dart';
+import 'package:front_end/src/base/api_signature.dart';
+import 'package:front_end/src/base/performace_logger.dart';
 import 'package:front_end/src/base/processed_options.dart';
 import 'package:front_end/src/fasta/dill/dill_target.dart';
 import 'package:front_end/src/fasta/kernel/kernel_target.dart';
 import 'package:front_end/src/fasta/ticker.dart';
 import 'package:front_end/src/fasta/translate_uri.dart';
+import 'package:front_end/src/incremental/byte_store.dart';
 import 'package:front_end/src/incremental/file_state.dart';
+import 'package:kernel/binary/ast_from_binary.dart';
+import 'package:kernel/binary/ast_to_binary.dart';
 import 'package:kernel/kernel.dart' hide Source;
-import 'package:kernel/target/vm.dart';
 
 dynamic unimplemented() {
   // TODO(paulberry): get rid of this.
   throw new UnimplementedError();
 }
 
+class ByteSink implements Sink<List<int>> {
+  final BytesBuilder builder = new BytesBuilder();
+
+  void add(List<int> data) {
+    builder.add(data);
+  }
+
+  void close() {}
+}
+
 /// Implementation of [IncrementalKernelGenerator].
 ///
 /// TODO(scheglov) Update the documentation.
@@ -36,9 +51,15 @@ class IncrementalKernelGeneratorImpl implements IncrementalKernelGenerator {
   /// The object that knows how to resolve "package:" and "dart:" URIs.
   final TranslateUri _uriTranslator;
 
+  /// The logger to report compilation progress.
+  final PerformanceLog _logger;
+
   /// The current file system state.
   final FileSystemState _fsState;
 
+  /// The byte storage to get and put serialized data.
+  final ByteStore _byteStore;
+
   /// The URI of the program entry point.
   final Uri _entryPoint;
 
@@ -46,38 +67,47 @@ class IncrementalKernelGeneratorImpl implements IncrementalKernelGenerator {
   /// and not checked for actual changes yet.
   final Set<Uri> _invalidatedFiles = new Set<Uri>();
 
-  /// The cached SDK kernel.
-  DillTarget _sdkDillTarget;
-
   IncrementalKernelGeneratorImpl(
       this._options, this._uriTranslator, this._entryPoint)
-      : _fsState = new FileSystemState(_options.fileSystem, _uriTranslator);
+      : _logger = _options.logger,
+        _fsState = new FileSystemState(_options.fileSystem, _uriTranslator),
+        _byteStore = _options.byteStore;
 
   @override
   Future<DeltaProgram> computeDelta(
       {Future<Null> watch(Uri uri, bool used)}) async {
-    await _ensureVmLibrariesLoaded();
-    await _refreshInvalidatedFiles();
+    return await _logger.runAsync('Compute delta', () async {
+      await _refreshInvalidatedFiles();
 
-    // Ensure that the graph starting at the entry point is ready.
-    await _fsState.getFile(_entryPoint);
+      // Ensure that the graph starting at the entry point is ready.
+      FileState entryLibrary = await _fsState.getFile(_entryPoint);
 
-    DillTarget sdkTarget = await _getSdkDillTarget();
-    // TODO(scheglov) Use it to also serve other package kernels.
+      List<LibraryCycle> cycles = _logger.run('Compute library cycles', () {
+        List<LibraryCycle> cycles = entryLibrary.topologicalOrder;
+        _logger.writeln('Computed ${cycles.length} cycles.');
+        return cycles;
+      });
 
-    KernelTarget kernelTarget = new KernelTarget(_fsState.fileSystemView,
-        sdkTarget, _uriTranslator, _options.strongMode, null);
-    kernelTarget.read(_entryPoint);
-
-    // TODO(scheglov) Replace with a better API.
-    // Firstly, we don't "write" anything here.
-    // Secondly, it catches all the exceptions and write them to `stderr`.
-    // This is too interactive and not API-clients friendly.
-    await kernelTarget.writeOutline(null);
+      CanonicalName nameRoot = new CanonicalName.root();
+      DillTarget dillTarget =
+          new DillTarget(new Ticker(isVerbose: false), _uriTranslator);
 
-    // TODO(scheglov) Replace with a better API.
-    Program program = await kernelTarget.writeProgram(null);
-    return new DeltaProgram(program);
+      List<_LibraryCycleResult> results = [];
+      await _logger.runAsync('Compute results for cycles', () async {
+        for (LibraryCycle cycle in cycles) {
+          _LibraryCycleResult result =
+              await _compileCycle(nameRoot, dillTarget, cycle);
+          results.add(result);
+        }
+      });
+
+      Program program = new Program(nameRoot: nameRoot);
+      for (_LibraryCycleResult result in results) {
+        program.libraries.addAll(result.kernelLibraries);
+      }
+
+      return new DeltaProgram(program);
+    });
   }
 
   @override
@@ -88,40 +118,122 @@ class IncrementalKernelGeneratorImpl implements IncrementalKernelGenerator {
   @override
   void invalidateAll() => unimplemented();
 
-  /// Fasta unconditionally loads all VM libraries.  In order to be able to
-  /// serve them using the file system view, we need to ask [_fsState] for
-  /// the corresponding files.
-  Future<Null> _ensureVmLibrariesLoaded() async {
-    List<String> extraLibraries = new VmTarget(null).extraRequiredLibraries;
-    for (String absoluteUriStr in extraLibraries) {
-      Uri absoluteUri = Uri.parse(absoluteUriStr);
-      Uri fileUri = _uriTranslator.translate(absoluteUri);
-      await _fsState.getFile(fileUri);
-    }
-  }
-
-  /// Return the [DillTarget] that is used inside of [KernelTarget] to
-  /// resynthesize SDK libraries.
-  Future<DillTarget> _getSdkDillTarget() async {
-    if (_sdkDillTarget == null) {
-      _sdkDillTarget =
-          new DillTarget(new Ticker(isVerbose: false), _uriTranslator);
-      // TODO(scheglov) Read the SDK kernel.
-//      _sdkDillTarget.read(options.sdkSummary);
-//      await _sdkDillTarget.writeOutline(null);
-    } else {
-//      Program sdkProgram = _sdkDillTarget.loader.program;
-//      sdkProgram.visitChildren(new _ClearCanonicalNamesVisitor());
-    }
-    return _sdkDillTarget;
+  /// Ensure that [dillTarget] includes the [cycle] libraries.  It already
+  /// contains all the libraries that sorted before the given [cycle] in
+  /// topological order.  Return the result with the cycle libraries.
+  Future<_LibraryCycleResult> _compileCycle(
+      CanonicalName nameRoot, DillTarget dillTarget, LibraryCycle cycle) async {
+    return _logger.runAsync('Compile cycle $cycle', () async {
+      String signature;
+      {
+        var signatureBuilder = new ApiSignature();
+        // TODO(scheglov) add salt

[Siggi Cherem (dart-lang), 2017/05/16 21:37:01]

might be worth expanding what this means :) - I have no idea what you mean by
salt here.

[scheglov, 2017/05/16 22:03:38]

Yeah, it's more a reminder for myself. Paul and I are familiar with
AnalysisDriver internals.

Salt something we mix into the signatures and it is a combination of the
compiler version, compilation options, etc.

+        //    signature.addUint32List(_fsState._salt);
+        Set<FileState> transitiveFiles = cycle.libraries
+            .map((library) => library.transitiveFiles)
+            .expand((files) => files)
+            .toSet();
+        signatureBuilder.addInt(transitiveFiles.length);
+        for (FileState file in transitiveFiles) {

[Siggi Cherem (dart-lang), 2017/05/16 21:37:01]

nit: `var` (here it does seem clear from context)

[scheglov, 2017/05/16 22:03:38]

Done.

+          signatureBuilder.addString(file.uri.toString());
+          // TODO(scheglov) use API signature
+          signatureBuilder.addBytes(file.contentHash);
+        }
+        signature = signatureBuilder.toHex();
+      }
+
+      _logger.writeln('Signature: $signature.');
+      String kernelKey = '$signature.kernel';

[Siggi Cherem (dart-lang), 2017/05/16 21:37:01]

ditto (`var`)

[scheglov, 2017/05/16 22:03:38]

Done.

+
+      /// We need kernel libraries for these URIs.
+      Set<Uri> libraryUris = new Set<Uri>();

[Siggi Cherem (dart-lang), 2017/05/16 21:37:01]

here too

[scheglov, 2017/05/16 22:03:38]

Done.

We probably should write a tool to remove all types for all local variables
then.
I see a lot of well typed code in Fasta.
It would be nice to have similar code style in all places of the package.

+      for (FileState library in cycle.libraries) {
+        libraryUris.add(library.uri);
+      }
+
+      /// Check if there is already a bundle with these libraries.
+      List<int> bytes = _byteStore.get(kernelKey);
+      if (bytes != null) {
+        return _logger.run('Read serialized libraries', () {
+          var program = new Program(nameRoot: nameRoot);
+          var reader = new BinaryBuilder(bytes);
+          reader.readProgram(program);
+          dillTarget.loader
+              .appendLibraries(program, (uri) => libraryUris.contains(uri));
+          return new _LibraryCycleResult(cycle, signature, program.libraries);
+        });
+      }
+
+      // Ask DILL to fill outlines using loaded libraries.
+      await dillTarget.writeOutline(null);

[Siggi Cherem (dart-lang), 2017/05/16 21:37:01]

We might need to do something to make `writeOutline` a noop for libraries that
have already been filled before. In particular, we will call writeOutline once
per cycle that we are compiling, so if you have:

   A   -- imports -->   B   -- imports -->   C

you might be filling the outline of C twice by the time you compile A. This
might be an issue (the builders might have the contents of C twice), or if
that's ignored, it appears to be wasteful. So it's probably worth taking a
closer look.

[scheglov, 2017/05/16 22:03:38]

It works fine, we fill only once.

Loader.buildOutlines() removes from unparsedLibraries.
We fill outlines only for new libraries for which read() was invoked.

+
+      // Create KernelTarget and configure it for compiling the cycle URIs.
+      KernelTarget kernelTarget = new KernelTarget(_fsState.fileSystemView,
+          dillTarget, _uriTranslator, _options.strongMode);
+      for (FileState library in cycle.libraries) {
+        kernelTarget.read(library.uri);
+      }
+
+      // Compile the cycle libraries into a new full program.
+      Program program = await _logger.runAsync(
+          'Compile ${cycle.libraries.length} cycle libraries', () async {
+        await kernelTarget.writeOutline(null, nameRoot: nameRoot);
+        return await kernelTarget.writeProgram(null);
+      });
+
+      // Add newly compiled libraries into DILL.
+      List<Library> kernelLibraries = program.libraries
+          .where((library) => libraryUris.contains(library.importUri))
+          .toList();
+      dillTarget.loader
+          .appendLibraries(program, (uri) => libraryUris.contains(uri));
+
+      _logger.run('Serialize ${kernelLibraries.length} libraries', () {
+        program.unbindCanonicalNames();
+        List<int> bytes = _writeProgramBytes(program, kernelLibraries.contains);
+        _byteStore.put(kernelKey, bytes);
+        _logger.writeln('Stored ${bytes.length} bytes.');
+      });
+
+      return new _LibraryCycleResult(cycle, signature, kernelLibraries);
+    });
   }
 
   /// Refresh all the invalidated files and update dependencies.
   Future<Null> _refreshInvalidatedFiles() async {
-    for (Uri fileUri in _invalidatedFiles) {
-      FileState file = await _fsState.getFile(fileUri);
-      await file.refresh();
-    }
-    _invalidatedFiles.clear();
+    await _logger.runAsync('Refresh invalidated files', () async {
+      for (Uri fileUri in _invalidatedFiles) {

[Siggi Cherem (dart-lang), 2017/05/16 21:37:01]

here too (`var`)

[scheglov, 2017/05/16 22:03:38]

Done.

+        FileState file = await _fsState.getFile(fileUri);

[Siggi Cherem (dart-lang), 2017/05/16 21:37:01]

and here.

[scheglov, 2017/05/16 22:03:38]

Done.

+        await file.refresh();

[Siggi Cherem (dart-lang), 2017/05/16 21:37:01]

do you need the refresh to happen one file at a time or is parallelism allowed
here?

If parallelism is OK, you could run each of these iterations separately and
merge them with Future.wait, or you could even do them in 2 stages:

   List<FileState> files = await
Future.wait(_invalidatedFiles.map(_fsState.getFile).toList());
   await Future.wait(files.map((f) => f.refresh()).toList());

[scheglov, 2017/05/16 22:03:38]

I think it will work.
But I'd like to see how it works on real app to measure performance difference.

+      }
+      _invalidatedFiles.clear();
+    });
+  }
+
+  List<int> _writeProgramBytes(Program program, bool filter(Library library)) {
+    ByteSink byteSink = new ByteSink();
+    new LibraryFilteringBinaryPrinter(byteSink, filter)
+        .writeProgramFile(program);
+    return byteSink.builder.takeBytes();
   }
 }
+
+/// Compilation result for a library cycle.
+class _LibraryCycleResult {
+  final LibraryCycle cycle;
+
+  /// The signature of the result.
+  ///
+  /// Currently it is based on the full content of the transitive closure of
+  /// the [cycle] files and all its dependencies.
+  /// TODO(scheglov) Not used yet.
+  /// TODO(scheglov) Use API signatures.
+  /// TODO(scheglov) Or use tree shaking and compute signatures of outlines.
+  final String signature;
+
+  /// Kernel libraries for libraries in the [cycle].  Dependencies are not
+  /// included, they were returned as results for preceding cycles.

[Siggi Cherem (dart-lang), 2017/05/16 21:37:01]

minor dartdoc suggestion: consider rephrasing this a bit to clarify that the
*body* of dependencies is not included, but references to those dependencies are
included. 

[scheglov, 2017/05/16 22:03:38]

Done.

+  final List<Library> kernelLibraries;
+
+  _LibraryCycleResult(this.cycle, this.signature, this.kernelLibraries);
+}