First pass at asynchronous input loading in dart2js.

sdk/lib/_internal/compiler/implementation/compiler.dart

Index: sdk/lib/_internal/compiler/implementation/compiler.dart
diff --git a/sdk/lib/_internal/compiler/implementation/compiler.dart b/sdk/lib/_internal/compiler/implementation/compiler.dart
index 1aa47b763707039c52ea28dda80d0bcf9c61a5b0..7d8bde006c8d4858670604453952d378c4b83545 100644
--- a/sdk/lib/_internal/compiler/implementation/compiler.dart
+++ b/sdk/lib/_internal/compiler/implementation/compiler.dart
@@ -81,6 +81,17 @@ class PostProcessTask {
   PostProcessTask(this.element, this.action);
 }
 
+// TODO(rnystrom): Now that file reading is asynchronous, this task is no longer
+// used and the time spent reading files isn't easily measurable (or even that
+// well-defined). As Peter says:
+//
+// As far as I can tell, we can only count how much time is being spent reading
+// files by having a global counter that covers the time from the beginning of
+// the compiler until it reaches a point when all libraries are scanned, and
+// subtract the time spent in other tasks.
+//
+// I think you should drop measureAsync. I think we can measure the time spent
+// reading files in dart2js.dart (you can just add a TODO).
 class ReadingFilesTask extends CompilerTask {

[ahe, 2013/08/06 16:29:17]

Can you remove this task?

[Johnni Winther, 2013/08/27 11:05:00]

Done.

   ReadingFilesTask(Compiler compiler) : super(compiler);
   String get name => 'Reading input files';
@@ -668,14 +679,17 @@ abstract class Compiler implements DiagnosticListener {
     reportDiagnostic(null, message, api.Diagnostic.VERBOSE_INFO);
   }
 
-  bool run(Uri uri) {
+  Future<bool> run(Uri uri) {
     totalCompileTime.start();
-    try {
-      runCompiler(uri);
-    } on CompilerCancelledException catch (exception) {
-      log('Error: $exception');
-      return false;
-    } catch (exception) {
+
+    return runCompiler(uri).then((_) {
+      return true;

[ahe, 2013/08/06 16:29:17]

Can't you eliminate this call to then?

[Johnni Winther, 2013/08/27 11:05:00]

Done.

+    }).catchError((error) {
+      if (error is CompilerCancelledException) {
+        log('Error: $error');
+        return false;
+      }
+
       try {
         if (!hasCrashed) {
           hasCrashed = true;
@@ -687,12 +701,13 @@ abstract class Compiler implements DiagnosticListener {
       } catch (doubleFault) {
         // Ignoring exceptions in exception handling.
       }
-      rethrow;
-    } finally {
+      throw error;
+    }).whenComplete(() {
       tracer.close();
       totalCompileTime.stop();
-    }
-    return !compilationFailed;
+    }).then((_) {
+      return !compilationFailed;

[ahe, 2013/08/06 16:29:17]

As it should be handled by this.

+    });
   }
 
   bool hasIsolateSupport() => isolateLibrary != null;
@@ -749,7 +764,7 @@ abstract class Compiler implements DiagnosticListener {
     }
   }
 
-  LibraryElement scanBuiltinLibrary(String filename);
+  Future<LibraryElement> scanBuiltinLibrary(String filename);
 
   void initializeSpecialClasses() {
     final List missingCoreClasses = [];
@@ -825,29 +840,38 @@ abstract class Compiler implements DiagnosticListener {
         listClass.lookupConstructor(callConstructor);
   }
 
-  void scanBuiltinLibraries() {
-    jsHelperLibrary = scanBuiltinLibrary('_js_helper');
-    interceptorsLibrary = scanBuiltinLibrary('_interceptors');
-    foreignLibrary = scanBuiltinLibrary('_foreign_helper');
-    isolateHelperLibrary = scanBuiltinLibrary('_isolate_helper');
-
-    assertMethod = jsHelperLibrary.find(const SourceString('assertHelper'));
-    identicalFunction = coreLibrary.find(const SourceString('identical'));
-
-    initializeSpecialClasses();
-
-    functionClass.ensureResolved(this);
-    functionApplyMethod =
-        functionClass.lookupLocalMember(const SourceString('apply'));
-    jsInvocationMirrorClass.ensureResolved(this);
-    invokeOnMethod = jsInvocationMirrorClass.lookupLocalMember(INVOKE_ON);
-
-    if (preserveComments) {
-      var uri = new Uri(scheme: 'dart', path: 'mirrors');
-      LibraryElement libraryElement =
-          libraryLoader.loadLibrary(uri, null, uri);
-      documentClass = libraryElement.find(const SourceString('Comment'));
-    }
+  Future scanBuiltinLibraries() {
+    return scanBuiltinLibrary('_js_helper').then((LibraryElement library) {
+      jsHelperLibrary = library;
+      return scanBuiltinLibrary('_interceptors');
+    }).then((LibraryElement library) {
+      interceptorsLibrary = library;
+      return scanBuiltinLibrary('_foreign_helper');
+    }).then((LibraryElement library) {
+      foreignLibrary = library;
+      return scanBuiltinLibrary('_isolate_helper');
+    }).then((LibraryElement library) {
+      isolateHelperLibrary = library;
+
+      assertMethod = jsHelperLibrary.find(const SourceString('assertHelper'));
+      identicalFunction = coreLibrary.find(const SourceString('identical'));
+
+      initializeSpecialClasses();
+
+      functionClass.ensureResolved(this);
+      functionApplyMethod =
+          functionClass.lookupLocalMember(const SourceString('apply'));
+      jsInvocationMirrorClass.ensureResolved(this);
+      invokeOnMethod = jsInvocationMirrorClass.lookupLocalMember(INVOKE_ON);
+
+      if (preserveComments) {
+        var uri = new Uri(scheme: 'dart', path: 'mirrors');
+        return libraryLoader.loadLibrary(uri, null, uri).then(
+            (LibraryElement libraryElement) {
+          documentClass = libraryElement.find(const SourceString('Comment'));
+        });
+      }
+    });
   }
 
   void importHelperLibrary(LibraryElement library) {
@@ -862,131 +886,138 @@ abstract class Compiler implements DiagnosticListener {
    */
   Uri resolvePatchUri(String dartLibraryPath);
 
-  void runCompiler(Uri uri) {
+  Future runCompiler(Uri uri) {
     // TODO(ahe): This prevents memory leaks when invoking the compiler
     // multiple times.  Implement a better mechanism where StringWrapper
     // instances are shared on a per library basis.
     SourceString.canonicalizedValues.clear();
 
     assert(uri != null || analyzeOnly);
-    scanBuiltinLibraries();
-    if (librariesToAnalyzeWhenRun != null) {
-      for (Uri libraryUri in librariesToAnalyzeWhenRun) {
-        log('analyzing $libraryUri ($buildId)');
-        libraryLoader.loadLibrary(libraryUri, null, libraryUri);
-      }
-    }
-    if (uri != null) {
-      if (analyzeOnly) {
-        log('analyzing $uri ($buildId)');
-      } else {
-        log('compiling $uri ($buildId)');
+    return scanBuiltinLibraries().then((_) {
+      if (librariesToAnalyzeWhenRun != null) {
+        return Future.forEach(librariesToAnalyzeWhenRun, (libraryUri) {
+          log('analyzing $libraryUri ($buildId)');
+          return libraryLoader.loadLibrary(libraryUri, null, libraryUri);
+        });
       }
-      mainApp = libraryLoader.loadLibrary(uri, null, uri);
-    }
-    Element main = null;
-    if (mainApp != null) {
-      main = mainApp.find(MAIN);
-      if (main == null) {
-        if (!analyzeOnly) {
-          // Allow analyze only of libraries with no main.
-          reportFatalError(
-              mainApp,
-              MessageKind.GENERIC,
-              {'text': 'Error: Could not find "${MAIN.slowToString()}".'});
-        } else if (!analyzeAll) {
-          reportFatalError(
-              mainApp,
-              MessageKind.GENERIC,
-              {'text': 'Error: Could not find "${MAIN.slowToString()}". '
-                  'No source will be analyzed. '
-                  'Use "--analyze-all" to analyze all code in the library.'});
-        }
-      } else {
-        if (!main.isFunction()) {
-          reportFatalError(
-              main,
-              MessageKind.GENERIC,
-              {'text': 'Error: "${MAIN.slowToString()}" is not a function.'});
+    }).then((_) {
+      if (uri != null) {
+        if (analyzeOnly) {
+          log('analyzing $uri ($buildId)');
+        } else {
+          log('compiling $uri ($buildId)');
         }
-        FunctionElement mainMethod = main;
-        FunctionSignature parameters = mainMethod.computeSignature(this);
-        parameters.forEachParameter((Element parameter) {
-          reportError(
-              parameter,
-              MessageKind.GENERIC,
-              {'text':
-                  'Error: "${MAIN.slowToString()}" cannot have parameters.'});
+        return libraryLoader.loadLibrary(uri, null, uri)
+            .then((LibraryElement library) {
+          mainApp = library;
         });
       }
+    }).then((_) {
+      Element main = null;

[ahe, 2013/08/06 16:29:17]

I think it would make sense to move the rest of this method into its own method.

[Johnni Winther, 2013/08/27 11:05:00]

Done.

+      if (mainApp != null) {
+        main = mainApp.find(MAIN);
+        if (main == null) {
+          if (!analyzeOnly) {
+            // Allow analyze only of libraries with no main.
+            reportFatalError(
+                mainApp,
+                MessageKind.GENERIC,
+                {'text': 'Error: Could not find "${MAIN.slowToString()}".'});
+          } else if (!analyzeAll) {
+            reportFatalError(
+                mainApp,
+                MessageKind.GENERIC,
+                {'text': 'Error: Could not find "${MAIN.slowToString()}". '
+                    'No source will be analyzed. '
+                    'Use "--analyze-all" to analyze all code in the library.'});
+          }
+        } else {
+          if (!main.isFunction()) {
+            reportFatalError(
+                main,
+                MessageKind.GENERIC,
+                {'text': 'Error: "${MAIN.slowToString()}" is not a function.'});
+          }
+          FunctionElement mainMethod = main;
+          FunctionSignature parameters = mainMethod.computeSignature(this);
+          parameters.forEachParameter((Element parameter) {
+            reportError(
+                parameter,
+                MessageKind.GENERIC,
+                {'text':
+                    'Error: "${MAIN.slowToString()}" cannot have parameters.'});
+          });
+        }
 
-      mirrorUsageAnalyzerTask.analyzeUsage(mainApp);
-
-      // In order to see if a library is deferred, we must compute the
-      // compile-time constants that are metadata.  This means adding
-      // something to the resolution queue.  So we cannot wait with
-      // this until after the resolution queue is processed.
-      // TODO(ahe): Clean this up, for example, by not enqueueing
-      // classes only used for metadata.
-      deferredLoadTask.findDeferredLibraries(mainApp);
-    }
+        mirrorUsageAnalyzerTask.analyzeUsage(mainApp);
 
-    log('Resolving...');
-    phase = PHASE_RESOLVING;
-    if (analyzeAll) {
-      libraries.forEach(
-          (_, lib) => fullyEnqueueLibrary(lib, enqueuer.resolution));
-    }
-    // Elements required by enqueueHelpers are global dependencies
-    // that are not pulled in by a particular element.
-    backend.enqueueHelpers(enqueuer.resolution, globalDependencies);
-    processQueue(enqueuer.resolution, main);
-    enqueuer.resolution.logSummary(log);
+        // In order to see if a library is deferred, we must compute the
+        // compile-time constants that are metadata.  This means adding
+        // something to the resolution queue.  So we cannot wait with
+        // this until after the resolution queue is processed.
+        // TODO(ahe): Clean this up, for example, by not enqueueing
+        // classes only used for metadata.
+        deferredLoadTask.findDeferredLibraries(mainApp);
+      }
 
-    if (compilationFailed) return;
-    if (analyzeOnly) return;
-    assert(main != null);
-    phase = PHASE_DONE_RESOLVING;
-
-    // TODO(ahe): Remove this line. Eventually, enqueuer.resolution
-    // should know this.
-    world.populate();
-    // Compute whole-program-knowledge that the backend needs. (This might
-    // require the information computed in [world.populate].)
-    backend.onResolutionComplete();
-
-    deferredLoadTask.onResolutionComplete(main);
-
-    log('Inferring types...');
-    typesTask.onResolutionComplete(main);
-
-    log('Compiling...');
-    phase = PHASE_COMPILING;
-    // TODO(johnniwinther): Move these to [CodegenEnqueuer].
-    if (hasIsolateSupport()) {
-      enqueuer.codegen.addToWorkList(
-          isolateHelperLibrary.find(Compiler.START_ROOT_ISOLATE));
-      enqueuer.codegen.registerGetOfStaticFunction(mainApp.find(MAIN));
-    }
-    if (enabledNoSuchMethod) {
-      enqueuer.codegen.registerInvocation(noSuchMethodSelector);
-      enqueuer.codegen.addToWorkList(createInvocationMirrorElement);
-    }
-    if (compileAll) {
-      libraries.forEach((_, lib) => fullyEnqueueLibrary(lib, enqueuer.codegen));
-    }
-    processQueue(enqueuer.codegen, main);
-    enqueuer.codegen.logSummary(log);
+      log('Resolving...');
+      phase = PHASE_RESOLVING;
+      if (analyzeAll) {
+        libraries.forEach(
+            (_, lib) => fullyEnqueueLibrary(lib, enqueuer.resolution));
+      }
+      // Elements required by enqueueHelpers are global dependencies
+      // that are not pulled in by a particular element.
+      backend.enqueueHelpers(enqueuer.resolution, globalDependencies);
+      processQueue(enqueuer.resolution, main);
+      enqueuer.resolution.logSummary(log);
+
+      if (compilationFailed) return;
+      if (analyzeOnly) return;
+      assert(main != null);
+      phase = PHASE_DONE_RESOLVING;
+
+      // TODO(ahe): Remove this line. Eventually, enqueuer.resolution
+      // should know this.
+      world.populate();
+      // Compute whole-program-knowledge that the backend needs. (This might
+      // require the information computed in [world.populate].)
+      backend.onResolutionComplete();
+
+      deferredLoadTask.onResolutionComplete(main);
+
+      log('Inferring types...');
+      typesTask.onResolutionComplete(main);
+
+      log('Compiling...');
+      phase = PHASE_COMPILING;
+      // TODO(johnniwinther): Move these to [CodegenEnqueuer].
+      if (hasIsolateSupport()) {
+        enqueuer.codegen.addToWorkList(
+            isolateHelperLibrary.find(Compiler.START_ROOT_ISOLATE));
+        enqueuer.codegen.registerGetOfStaticFunction(mainApp.find(MAIN));
+      }
+      if (enabledNoSuchMethod) {
+        enqueuer.codegen.registerInvocation(noSuchMethodSelector);
+        enqueuer.codegen.addToWorkList(createInvocationMirrorElement);
+      }
+      if (compileAll) {
+        libraries.forEach((_, lib) => fullyEnqueueLibrary(lib,
+            enqueuer.codegen));
+      }
+      processQueue(enqueuer.codegen, main);
+      enqueuer.codegen.logSummary(log);
 
-    if (compilationFailed) return;
+      if (compilationFailed) return;
 
-    backend.assembleProgram();
+      backend.assembleProgram();
 
-    checkQueues();
+      checkQueues();
 
-    if (compilationFailed) {
-      assembledCode = null; // Signals failure.
-    }
+      if (compilationFailed) {
+        assembledCode = null; // Signals failure.
+      }
+    });
   }
 
   void fullyEnqueueLibrary(LibraryElement library, Enqueuer world) {
@@ -1318,7 +1349,7 @@ abstract class Compiler implements DiagnosticListener {
    *
    * See [LibraryLoader] for terminology on URIs.
    */
-  Script readScript(Uri readableUri, [Node node]) {
+  Future<Script> readScript(Uri readableUri, [Element element, Node node]) {
     unimplemented('Compiler.readScript');
   }
 
@@ -1462,10 +1493,14 @@ class SourceSpan {
 bool invariant(Spannable spannable, var condition, {var message: null}) {
   // TODO(johnniwinther): Use [spannable] and [message] to provide better
   // information on assertion errors.
+  if (spannable == null) {
+    throw new SpannableAssertionFailure(CURRENT_ELEMENT_SPANNABLE, 
+        "Spannable was null for invariant. Use CURRENT_ELEMENT_SPANNABLE.");
+  }
   if (condition is Function){
     condition = condition();
   }
-  if (spannable == null || !condition) {
+  if (!condition) {
     if (message is Function) {
       message = message();
     }