Add support for caching results of shared modules.

This is important as we will soon add support for compiling the sdk as a
module and we would like to only compile it once when running a suite of
tests.

+ also enable caching in the dart2js pipeline test.

Change-Id: Ic9043f868123164f3ab425ba73f7428416b05fc0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/103485
Commit-Queue: Sigmund Cherem <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>

Author: sigmundch

pkg/modular_test/lib/src/io_pipeline.dart

@@ -26,49 +26,103 @@ abstract class IOModularStep extends ModularStep {
 }
 
 class IOPipeline extends Pipeline<IOModularStep> {
-  /// A folder per step. The key is the data id produced by a specific step.
+  /// Folder that holds the results of each step during the run of the pipeline.
   ///
-  /// This contains internal state used during the run of the pipeline, but is
-  /// expected to be null before and after the pipeline is executed.
-  Map<DataId, Uri> _tmpFolders;
-  Map<DataId, Uri> get tmpFoldersForTesting => _tmpFolders;
-  bool saveFoldersForTesting;
+  /// This value is usually null before and after the pipeline runs, but will be
+  /// non-null in two cases:
+  ///
+  ///  * for testing purposes when using [saveIntermediateResultsForTesting].
+  ///
+  ///  * to share results across pipeline runs when using [cacheSharedModules].
+  ///
+  /// When using [cacheSharedModules] the pipeline will only reuse data for
+  /// modules that are known to be shared (e.g. shared packages and sdk
+  /// libraries), and not modules that are test specific. File names will be
+  /// specific enough so that we can keep separate the artifacts created from
+  /// running tools under different configurations (with different flags).
+  Uri _resultsFolderUri;
+  Uri get resultFolderUriForTesting => _resultsFolderUri;
+
+  /// A unique number to denote the current modular test configuration.
+  ///
+  /// When using [cacheSharedModules], a test can resuse the output of a
+  /// previous run of this pipeline if that output was generated with the same
+  /// configuration.
+  int _currentConfiguration;
+
+  final ConfigurationRegistry _registry;
 
-  IOPipeline(List<IOModularStep> steps, {this.saveFoldersForTesting: false})
-      : super(steps);
+  /// Whether to keep alive the temporary folder used to store intermediate
+  /// results in order to inspect it later in test.
+  final bool saveIntermediateResultsForTesting;
+
+  IOPipeline(List<IOModularStep> steps,
+      {this.saveIntermediateResultsForTesting: false,
+      bool cacheSharedModules: false})
+      : _registry = cacheSharedModules ? new ConfigurationRegistry() : null,
+        super(steps, cacheSharedModules);
 
   @override
   Future<void> run(ModularTest test) async {
-    assert(_tmpFolders == null);
-    _tmpFolders = {};
+    var resultsDir = null;
+    if (_resultsFolderUri == null) {
+      resultsDir = await Directory.systemTemp.createTemp('modular_test_res-');
+      _resultsFolderUri = resultsDir.uri;
+    }
+    if (cacheSharedModules) {
+      _currentConfiguration = _registry.computeConfigurationId(test);
+    }
     await super.run(test);
-    if (!saveFoldersForTesting) {
-      for (var folder in _tmpFolders.values) {
-        await Directory.fromUri(folder).delete(recursive: true);
-      }
-      _tmpFolders = null;
+    if (resultsDir != null &&
+        !saveIntermediateResultsForTesting &&
+        !cacheSharedModules) {
+      await resultsDir.delete(recursive: true);
+      _resultsFolderUri = null;
+    }
+    if (!saveIntermediateResultsForTesting) {
+      _currentConfiguration = null;
+    }
+  }
+
+  /// Delete folders that were kept around either because of
+  /// [saveIntermediateResultsForTesting] or because of [cacheSharedModules].
+  Future<void> cleanup() async {
+    if (saveIntermediateResultsForTesting || cacheSharedModules) {
+      await Directory.fromUri(_resultsFolderUri).delete(recursive: true);
     }
   }
 
   @override
   Future<void> runStep(IOModularStep step, Module module,
       Map<Module, Set<DataId>> visibleData) async {
-    // Since data ids are unique throughout the pipeline, we use the first
-    // result data id as a hint for the name of the temporary folder of a step.
-    var stepFolder;
-    for (var dataId in step.resultData) {
-      stepFolder ??=
-          await Directory.systemTemp.createTemp('modular_test_${dataId}-');
-      _tmpFolders[dataId] ??=
-          (await Directory.systemTemp.createTemp('modular_test_${dataId}_res-'))
-              .uri;
+    if (cacheSharedModules && module.isShared) {
+      // If all expected outputs are already available, skip the step.
+      bool allCachedResultsFound = true;
+      for (var dataId in step.resultData) {
+        var cachedFile = File.fromUri(_resultsFolderUri
+            .resolve(_toFileName(module, dataId, configSpecific: true)));
+        if (!await cachedFile.exists()) {
+          allCachedResultsFound = false;
+          break;
+        }
+      }
+      if (allCachedResultsFound) {
+        step.notifyCached(module);
+        return;
+      }
     }
+
+    // Each step is executed in a separate folder.  To make it easier to debug
+    // issues, we include one of the step data ids in the name of the folder.
+    var stepId = step.resultData.first;
+    var stepFolder =
+        await Directory.systemTemp.createTemp('modular_test_${stepId}-');
     for (var module in visibleData.keys) {
       for (var dataId in visibleData[module]) {
-        var filename = "${module.name}.${dataId.name}";
-        var assetUri = _tmpFolders[dataId].resolve(filename);
-        await File.fromUri(assetUri)
-            .copy(stepFolder.uri.resolve(filename).toFilePath());
+        var assetUri = _resultsFolderUri
+            .resolve(_toFileName(module, dataId, configSpecific: true));
+        await File.fromUri(assetUri).copy(
+            stepFolder.uri.resolve(_toFileName(module, dataId)).toFilePath());
       }
     }
     if (step.needsSources) {
@@ -81,19 +135,31 @@ class IOPipeline extends Pipeline<IOModularStep> {
     }
 
     await step.execute(module, stepFolder.uri,
-        (Module m, DataId id) => Uri.parse("${m.name}.${id.name}"));
+        (Module m, DataId id) => Uri.parse(_toFileName(m, id)));
 
     for (var dataId in step.resultData) {
       var outputFile =
-          File.fromUri(stepFolder.uri.resolve("${module.name}.${dataId.name}"));
+          File.fromUri(stepFolder.uri.resolve(_toFileName(module, dataId)));
       if (!await outputFile.exists()) {
         throw StateError(
             "Step '${step.runtimeType}' didn't produce an output file");
       }
-      await outputFile.copy(_tmpFolders[dataId]
-          .resolve("${module.name}.${dataId.name}")
+      await outputFile.copy(_resultsFolderUri
+          .resolve(_toFileName(module, dataId, configSpecific: true))
           .toFilePath());
     }
     await stepFolder.delete(recursive: true);
   }
+
+  String _toFileName(Module module, DataId dataId,
+      {bool configSpecific: false}) {
+    var prefix =
+        cacheSharedModules && configSpecific && _currentConfiguration != null
+            ? _currentConfiguration
+            : '';
+    return "$prefix${module.name}.${dataId.name}";
+  }
+
+  String configSpecificResultFileNameForTesting(Module module, DataId dataId) =>
+      _toFileName(module, dataId, configSpecific: true);
 }

pkg/modular_test/lib/src/loader.dart

@@ -17,11 +17,11 @@
 ///       [defaultPackagesInput]. The list of packages provided is expected to
 ///       be disjoint with those in [defaultPackagesInput].
 ///   * a modules.yaml file: a specification of dependencies between modules.
-///     The format is described in `dependencies_parser.dart`.
+///     The format is described in `test_specification_parser.dart`.
 import 'dart:io';
 import 'dart:convert';
 import 'suite.dart';
-import 'dependency_parser.dart';
+import 'test_specification_parser.dart';
 import 'find_sdk_root.dart';
 
 import 'package:package_config/packages_file.dart' as package_config;
@@ -40,7 +40,7 @@ Future<ModularTest> loadTest(Uri uri) async {
   Map<String, Uri> defaultPackages =
       package_config.parse(_defaultPackagesInput, root);
   Map<String, Module> modules = {};
-  String spec;
+  String specString;
   Module mainModule;
   Map<String, Uri> packages = {};
   await for (var entry in folder.list(recursive: false)) {
@@ -69,7 +69,7 @@ Future<ModularTest> loadTest(Uri uri) async {
         List<int> packagesBytes = await entry.readAsBytes();
         packages = package_config.parse(packagesBytes, entryUri);
       } else if (fileName == 'modules.yaml') {
-        spec = await entry.readAsString();
+        specString = await entry.readAsString();
       }
     } else {
       assert(entry is Directory);
@@ -88,7 +88,7 @@ Future<ModularTest> loadTest(Uri uri) async {
           packageBase: Uri.parse('$moduleName/'));
     }
   }
-  if (spec == null) {
+  if (specString == null) {
     return _invalidTest("modules.yaml file is missing");
   }
   if (mainModule == null) {
@@ -97,12 +97,15 @@ Future<ModularTest> loadTest(Uri uri) async {
 
   _addDefaultPackageEntries(packages, defaultPackages);
   await _addModulePerPackage(packages, modules);
-  _attachDependencies(parseDependencyMap(spec), modules);
-  _attachDependencies(parseDependencyMap(_defaultPackagesSpec), modules);
+  TestSpecification spec = parseTestSpecification(specString);
+  _attachDependencies(spec.dependencies, modules);
+  _attachDependencies(
+      parseTestSpecification(_defaultPackagesSpec).dependencies, modules);
   _detectCyclesAndRemoveUnreachable(modules, mainModule);
   var sortedModules = modules.values.toList()
     ..sort((a, b) => a.name.compareTo(b.name));
-  return new ModularTest(sortedModules, mainModule);
+  var sortedFlags = spec.flags.toList()..sort();
+  return new ModularTest(sortedModules, mainModule, sortedFlags);
 }
 
 /// Returns all source files recursively found in a folder as relative URIs.
@@ -169,7 +172,7 @@ Future<void> _addModulePerPackage(
       // module that is part of the test (package name and module name should
       // match).
       modules[packageName] = Module(packageName, [], rootUri, sources,
-          isPackage: true, packageBase: Uri.parse('lib/'));
+          isPackage: true, packageBase: Uri.parse('lib/'), isShared: true);
     }
   }
 }

pkg/modular_test/lib/src/memory_pipeline.dart

@@ -27,20 +27,60 @@ class MemoryPipeline extends Pipeline<MemoryModularStep> {
   /// A copy of [_result] at the time the pipeline last finished running.
   Map<Module, Map<DataId, Object>> resultsForTesting;
 
-  MemoryPipeline(this._sources, List<MemoryModularStep> steps) : super(steps);
+  final ConfigurationRegistry _registry;
+
+  /// Cache of results when [cacheSharedModules] is true
+  final List<Map<Module, Map<DataId, Object>>> _resultCache;
+
+  MemoryPipeline(this._sources, List<MemoryModularStep> steps,
+      {bool cacheSharedModules: false})
+      : _registry = cacheSharedModules ? new ConfigurationRegistry() : null,
+        _resultCache = cacheSharedModules ? [] : null,
+        super(steps, cacheSharedModules);
 
   @override
   Future<void> run(ModularTest test) async {
-    assert(_results == null);
     _results = {};
+    Map<Module, Map<DataId, Object>> cache = null;
+    if (cacheSharedModules) {
+      int id = _registry.computeConfigurationId(test);
+      if (id < _resultCache.length) {
+        cache = _resultCache[id];
+      } else {
+        assert(id == _resultCache.length);
+        _resultCache.add(cache = {});
+      }
+      _results.addAll(cache);
+    }
     await super.run(test);
     resultsForTesting = _results;
+    if (cacheSharedModules) {
+      for (var module in _results.keys) {
+        if (module.isShared) {
+          cache[module] = _results[module];
+        }
+      }
+    }
     _results = null;
   }
 
   @override
   Future<void> runStep(MemoryModularStep step, Module module,
       Map<Module, Set<DataId>> visibleData) async {
+    if (cacheSharedModules && module.isShared) {
+      bool allCachedResultsFound = true;
+      for (var dataId in step.resultData) {
+        if (_results[module] == null || _results[module][dataId] == null) {
+          allCachedResultsFound = false;
+          break;
+        }
+      }
+      if (allCachedResultsFound) {
+        step.notifyCached(module);
+        return;
+      }
+    }
+
     Map<Module, Map<DataId, Object>> inputData = {};
     visibleData.forEach((module, dataIdSet) {
       inputData[module] = {};

pkg/modular_test/lib/src/pipeline.dart

@@ -48,6 +48,9 @@ class ModularStep {
       this.moduleDataNeeded: const [],
       this.resultData,
       this.onlyOnMain: false});
+
+  /// Notifies that the step was not executed, but cached instead.
+  void notifyCached(Module module) {}
 }
 
 /// An object to uniquely identify modular data produced by a modular step.
@@ -63,9 +66,13 @@ class DataId {
 }
 
 abstract class Pipeline<S extends ModularStep> {
+  /// Whether to cache the result of shared modules (e.g. shard packages and sdk
+  /// libraries) when multiple tests are run by this pipeline.
+  final bool cacheSharedModules;
+
   final List<S> steps;
 
-  Pipeline(this.steps) {
+  Pipeline(this.steps, this.cacheSharedModules) {
     _validate();
   }