Switch FE to use the libraries.json format.

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

 // Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:async';
 
 import 'package:front_end/compilation_message.dart';
 import 'package:front_end/compiler_options.dart';
 import 'package:front_end/file_system.dart';
 import 'package:front_end/src/base/performace_logger.dart';
@@ skipping 10 matching lines @@
 import 'package:kernel/target/targets.dart';
 import 'package:kernel/target/vm_fasta.dart';
 import 'package:package_config/packages.dart' show Packages;
 import 'package:package_config/src/packages_impl.dart'
     show NonFilePackagesDirectoryPackages, MapPackages;
 import 'package:package_config/packages_file.dart' as package_config;
 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
 /// implementation, default values for options that were not provided,
 /// and information derived from how the compiler was invoked (like the
 /// entry-points given to the compiler and whether a modular or whole-program
 /// API was used).
 ///
 /// The intent is that the front end should immediately wrap any incoming
 /// [CompilerOptions] object in this class before doing further processing, and
@@ skipping 28 matching lines @@
   /// includes inferred types.
   List<Program> _inputSummariesPrograms;
 
   /// Other programs that are meant to be linked and compiled with the input
   /// sources.
   List<Program> _linkedDependencies;
 
   /// 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;
 
   bool get verbose => _raw.verbose;
 
   bool get verify => _raw.verify;
 
   bool get debugDump => _raw.debugDump;
 
   bool get setExitCodeOnProblem => _raw.setExitCodeOnProblem;
@@ skipping 174 matching lines @@
   Program loadProgram(List<int> bytes, CanonicalName nameRoot) {
     return loadProgramFromBytes(bytes, new Program(nameRoot: nameRoot));
   }
 
   /// Get the [UriTranslator] which resolves "package:" and "dart:" URIs.
   ///
   /// This is an asynchronous method since file system operations may be
   /// 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.
   ///
   /// This is an asynchronous getter since file system operations may be
   /// required to locate/read the packages file.
   Future<Packages> _getPackages() async {
     if (_packages != null) return _packages;
     if (_raw.packagesFileUri != null) {
       return _packages = await createPackagesFromFile(_raw.packagesFileUri);
@@ skipping 90 matching lines @@
       candidate = await checkInDir(parentDir);
       if (candidate != null) break;
       dir = parentDir;
       parentDir = dir.resolve('..');
     }
 
     if (candidate != null) return createPackagesFromFile(candidate);
     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 + '/');
+
+    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');
     }
-    return root;
-  }
 
-  /// Get or infer the location of the SDK summary.
-  Uri _computeSdkSummaryUri() {
-    if (_raw.sdkSummary != null) return _raw.sdkSummary;
-
-    // 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.
   ///
   /// If `_raw.multiRoots` is not empty, the file-system will implement the
   /// semantics of multiple roots. If [chaseDependencies] is false, the
   /// resulting file system will be hermetic.
   FileSystem _createFileSystem() {
     var result = _raw.fileSystem;
     // Note: hermetic checks are done before translating multi-root URIs, so
@@ skipping 50 matching lines @@
     writeList('Multiroots', _raw.multiRoots);
 
     sb.writeln('Modular: ${_modularApi}');
     sb.writeln('Hermetic: ${!chaseDependencies}'
         ' (provided: ${!_raw.chaseDependencies})');
     sb.writeln('Packages uri: ${_raw.packagesFileUri}');
     sb.writeln('Packages: ${_packages}');
 
     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}');
     sb.writeln('Target: ${_target?.name} (provided: ${_raw.target?.name})');
 
     sb.writeln('throwOnErrorsAreFatal: ${throwOnErrors}');
     sb.writeln('throwOnWarningsAreFatal: ${throwOnWarnings}');
     sb.writeln('throwOnNits: ${throwOnNits}');
     sb.writeln('exit on problem: ${setExitCodeOnProblem}');
     sb.writeln('Embed sources: ${embedSourceText}');
@@ skipping 44 matching lines @@
   String get dart2jsCode => _original.code.dart2jsCode;
 
   SourceSpan get span =>
       new SourceLocation(_original.charOffset, sourceUrl: _original.uri)
           .pointSpan();
 
   _CompilationMessage(this._original, this.severity);
 
   String toString() => message;
 }