Switch FE to use the libraries.json format.

pkg/front_end/lib/src/base/processed_options.dart

Index: pkg/front_end/lib/src/base/processed_options.dart
diff --git a/pkg/front_end/lib/src/base/processed_options.dart b/pkg/front_end/lib/src/base/processed_options.dart
index ea1d6ac526b132e1c77ef3fecc461f75f8c43489..2abbd78d494e3fe0c63b2913d284a7f029cd24ac 100644
--- a/pkg/front_end/lib/src/base/processed_options.dart
+++ b/pkg/front_end/lib/src/base/processed_options.dart
@@ -28,6 +28,8 @@ import 'package:source_span/source_span.dart' show SourceSpan, SourceLocation;
 import 'package:front_end/src/fasta/command_line_reporting.dart'
     as command_line_reporting;
 
+import 'libraries_spec.dart';
+
 /// All options needed for the front end implementation.
 ///
 /// This includes: all of [CompilerOptions] in a form useful to the
@@ -76,10 +78,22 @@ class ProcessedOptions {
   /// The location of the SDK, or `null` if the location hasn't been determined
   /// yet.
   Uri _sdkRoot;
-  Uri get sdkRoot => _sdkRoot ??= _normalizeSdkRoot();
+  Uri get sdkRoot {
+    _ensureSdkDefaults();
+    return _sdkRoot;
+  }
 
   Uri _sdkSummary;
-  Uri get sdkSummary => _sdkSummary ??= _computeSdkSummaryUri();
+  Uri get sdkSummary {
+    _ensureSdkDefaults();
+    return _sdkSummary;
+  }
+
+  Uri _librariesSpecUri;
+  Uri get librariesSpecUri {
+    _ensureSdkDefaults();
+    return _librariesSpecUri;
+  }
 
   Ticker ticker;
 
@@ -274,20 +288,34 @@ class ProcessedOptions {
   /// required to locate/read the packages file as well as SDK metadata.
   Future<UriTranslatorImpl> getUriTranslator() async {
     if (_uriTranslator == null) {
-      await _getPackages();
-      // TODO(scheglov) Load SDK libraries from whatever format we decide.
-      // TODO(scheglov) Remove the field "_raw.dartLibraries".
-      var libraries = _raw.dartLibraries ?? await _parseDartLibraries();
-      _uriTranslator = new UriTranslatorImpl(
-          libraries, const <String, List<Uri>>{}, _packages);
+      var libraries = await _computeLibrarySpecification();
+      ticker.logMs("Read libraries file");

[ahe, 2017/08/03 11:58:16]

You need to make sure you know precisely when ticker.logMs is called. It doesn't
work when stuff is initialized by demand. There's an arbitrary amount of things
happening before reading the library specification.

I suggest that you compute the TargetSpecification explicitly before creating
the ProcessedOptions object.

Ideally, the flow of the code should be:

ticker.logMs("The previous phase").
TargetLibrariesSpecification specification = await
computeLibrarySpecification();
ticker.logMs("Read libraries specification");
ProcessedOptions options = ...;
ticker.logMs("Processed options");
// Invariant: all computations in processed options are done now.

[Paul Berry, 2017/08/03 17:50:41]

It seems like you're suggesting a flow where the client does some processing
(based on options) before creating the ProcessedOptions object.  I really want
to avoid that because the whole idea of ProcessedOptions is to make sure that
all the APIs in the front end deal with options in a uniform way; requiring
computation to be done before creating ProcessedOptions creates the danger of
drift between what the different implementations do before creating
ProcessedOptions.

Personally I don't have as big a concern about initializing stuff on demand.  As
long as we call ticker.logMs before and after doing things, we can safely do
things on demand because the order of operations will be clear in the log.

So I think all we need to do here is add the following line just before line
291:

ticker.logMs("Compute library specification");

[Siggi Cherem (dart-lang), 2017/08/05 00:41:02]

Done. Added the extra tick as Paul suggested.

[ahe, 2017/08/07 22:12:04]

I don't think that will work. This will print something like this:

Compute library specification in N ms.
Read libraries file in M ms.

Those N ms will apply to something else.

This is not how the ticker works, it only has one stopwatch. The alternatives
don't appeal to me:

* "measure" as known from dart2js can measure things that are on-demand and
asynchronous, but it adds a lot of overhead simply to measure.

* profiling, often statistically, and often measures short running programs
incorrectly. This makes it a poor tool for understanding start up performance
(which this code belongs to).

Unfortunately, it's not easy to make sure that the ticker is used correctly and
that all (potentially) expensive things are timed. But when we take care to do
it correctly, we see the correct timings all the time. This is important to me
as this is how I monitor performance. Golem is also nice, but not a substitute
for the in-your-face approach of Ticker.

Another option is to completely remove uses of Ticker from ProcessedOptions and
rely on the user of ProcessedOptions to initialize all the stuff that it needs
in a predicable sequence (by calling the corresponding getters).

FWIW, I wasn't suggesting that the client has to do computations before
initializing the ProcessedOptions object. The code snippet I provided could go
in a static async method on ProcessedOptions.

+      var packages = await _getPackages();
       ticker.logMs("Read packages file");
+      _uriTranslator = new UriTranslatorImpl(libraries, packages);
     }
     return _uriTranslator;
   }
 
-  Future<Map<String, Uri>> _parseDartLibraries() async {
-    Uri librariesJson = _raw.sdkRoot?.resolve("lib/libraries.json");
-    return await computeDartLibraries(fileSystem, librariesJson);
+  Future<TargetLibrariesSpecification> _computeLibrarySpecification() async {
+    var name = target.name;
+    // TODO(sigmund): eek! we should get to the point where there is no

[ahe, 2017/08/03 11:58:16]

Eek (uppercase).
We (uppercase).

[Siggi Cherem (dart-lang), 2017/08/05 00:41:02]

EEK! OK :)

+    // fasta-specific targets and the target names are meaningful.
+    if (name.endsWith('_fasta')) name = name.substring(0, name.length - 6);
+
+    if (librariesSpecUri == null ||

[ahe, 2017/08/03 11:58:15]

This field should be named librariesSpecificationUri.

[Siggi Cherem (dart-lang), 2017/08/05 00:41:02]

Done.

+        !await fileSystem.entityForUri(librariesSpecUri).exists()) {
+      if (compileSdk) {
+        reportWithoutLocation(
+            templateSdkSpecificationNotFound.withArguments(librariesSpecUri),
+            Severity.error);
+      }
+      return new TargetLibrariesSpecification(name);
+    }
+
+    var json = await fileSystem.entityForUri(librariesSpecUri).readAsString();
+    var spec = await LibrariesSpecification.parse(librariesSpecUri, json);
+    return spec.specificationFor(name);
   }
 
   /// Get the package map which maps package names to URIs.
@@ -398,32 +426,42 @@ class ProcessedOptions {
     return Packages.noPackages;
   }
 
-  /// Get the location of the SDK.
-  Uri _normalizeSdkRoot() {
-    // If an SDK summary location was provided, the SDK itself should not be
-    // needed.
-    assert(_raw.sdkSummary == null);
-    if (_raw.sdkRoot == null) {
+  bool _computedSdkDefaults = false;
+
+  /// Ensure [_sdkRoot], [_sdkSummary] and [_librarySpecUri] are initialized.
+  ///
+  /// If they are not set explicitly, they are infered based on the default
+  /// behavior described in [CompilerOptions].
+  void _ensureSdkDefaults() {
+    if (_computedSdkDefaults) return;
+    _computedSdkDefaults = true;
+    var root = _raw.sdkRoot;
+    if (root != null) {
+      // Normalize to always end in '/'
+      if (!root.path.endsWith('/')) {
+        root = root.replace(path: root.path + '/');
+      }
+      _sdkRoot = root;
+    } else if (compileSdk) {
       // TODO(paulberry): implement the algorithm for finding the SDK
       // automagically.
-      return unimplemented('infer the default sdk location', -1, null);
+      unimplemented('infer the default sdk location', -1, null);
     }
-    var root = _raw.sdkRoot;
-    if (!root.path.endsWith('/')) {
-      root = root.replace(path: root.path + '/');
-    }
-    return root;
-  }
 
-  /// Get or infer the location of the SDK summary.
-  Uri _computeSdkSummaryUri() {
-    if (_raw.sdkSummary != null) return _raw.sdkSummary;
+    if (_raw.sdkSummary != null) {
+      _sdkSummary = _raw.sdkSummary;
+    } else if (!compileSdk) {
+      // Infer based on the sdkRoot, but only when `compileSdk` is false,
+      // otherwise the default intent was to compile the sdk from sources and
+      // not to load an sdk summary file.
+      _sdkSummary = root?.resolve('outline.dill');
+    }
 
-    // Infer based on the sdkRoot, but only when `compileSdk` is false,
-    // otherwise the default intent was to compile the sdk from sources and not
-    // to load an sdk summary file.
-    if (_raw.compileSdk) return null;
-    return sdkRoot.resolve('outline.dill');
+    if (_raw.librariesSpecUri != null) {
+      _librariesSpecUri = _raw.librariesSpecUri;
+    } else if (compileSdk) {
+      _librariesSpecUri = sdkRoot.resolve('lib/libraries.json');
+    }
   }
 
   /// Create a [FileSystem] specific to the current options.
@@ -494,6 +532,8 @@ class ProcessedOptions {
 
     sb.writeln('Compile SDK: ${compileSdk}');
     sb.writeln('SDK root: ${_sdkRoot} (provided: ${_raw.sdkRoot})');
+    sb.writeln('SDK specification: ${_librariesSpecUri} '
+        '(provided: ${_raw.librariesSpecUri})');
     sb.writeln('SDK summary: ${_sdkSummary} (provided: ${_raw.sdkSummary})');
 
     sb.writeln('Strong: ${strongMode}');