[ResidentFrontendServer] Cache the compiler options that were last used

to compile every entrypoint

This is necessary to ensure that the options used when performing
compilation during expression evaluation or hot reload match the options
that were used to compile the running program before it was started.

TEST=test case added to pkg/dartdev/test/commands/run_test.dart, CI

CoreLibraryReviewExempt: This CL does not include any core library API
changes, only VM Service implementation changes in
sdk/lib/vmservice/running_isolates.dart.
Change-Id: If11207e090d9b02d5a6a8396e09dc90d0c874f58
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/403240
Reviewed-by: Ben Konyi <[email protected]>
Commit-Queue: Derek Xu <[email protected]>

Author: derekxu16

pkg/dartdev/lib/src/generate_kernel.dart

@@ -6,7 +6,10 @@ import 'dart:io';
 
 import 'package:args/args.dart';
 import 'package:frontend_server/resident_frontend_server_utils.dart'
-    show computeCachedDillPath, ResidentCompilerInfo, sendAndReceiveResponse;
+    show
+        computeCachedDillAndCompilerOptionsPaths,
+        ResidentCompilerInfo,
+        sendAndReceiveResponse;
 import 'package:kernel/binary/tag.dart' show isValidSdkHash;
 import 'package:path/path.dart' as p;
 import 'package:pub/pub.dart';
@@ -57,7 +60,9 @@ Future<DartExecutableWithPackageConfig> generateKernel(
       packageRoot != null ? p.join(packageRoot, packageConfigName) : null;
 
   final canonicalizedExecutablePath = p.canonicalize(executable.executable);
-  final cachedDillPath = computeCachedDillPath(canonicalizedExecutablePath);
+  final cachedDillPath =
+      computeCachedDillAndCompilerOptionsPaths(canonicalizedExecutablePath)
+          .cachedDillPath;
 
   Map<String, dynamic> result;
   try {

pkg/dartdev/test/commands/run_test.dart

@@ -9,7 +9,10 @@ import 'dart:io';
 import 'package:dartdev/src/resident_frontend_constants.dart';
 import 'package:dartdev/src/resident_frontend_utils.dart';
 import 'package:frontend_server/resident_frontend_server_utils.dart'
-    show computeCachedDillPath, ResidentCompilerInfo, sendAndReceiveResponse;
+    show
+        computeCachedDillAndCompilerOptionsPaths,
+        ResidentCompilerInfo,
+        sendAndReceiveResponse;
 import 'package:kernel/binary/tag.dart' show sdkHashNull;
 import 'package:path/path.dart' as path;
 import 'package:pub_semver/pub_semver.dart';
@@ -987,8 +990,10 @@ void residentRun() {
     serverInfoDirectory = project(mainSrc: 'void main() {}');
     serverInfoFile = path.join(serverInfoDirectory.dirPath, 'info');
 
-    final cachedDillFile =
-        File(computeCachedDillPath(serverInfoDirectory.mainPath));
+    final cachedDillFile = File(
+      computeCachedDillAndCompilerOptionsPaths(serverInfoDirectory.mainPath)
+          .cachedDillPath,
+    );
     expect(cachedDillFile.existsSync(), false);
 
     final result = await serverInfoDirectory.run([
@@ -1054,7 +1059,9 @@ void residentRun() {
   test("'Hello World'", () async {
     p = project(mainSrc: "void main() { print('Hello World'); }");
 
-    final cachedDillFile = File(computeCachedDillPath(p.mainPath));
+    final cachedDillFile = File(
+      computeCachedDillAndCompilerOptionsPaths(p.mainPath).cachedDillPath,
+    );
     expect(cachedDillFile.existsSync(), false);
 
     final result = await p.run([
@@ -1081,7 +1088,9 @@ void residentRun() {
     p = project(mainSrc: "void main() { print('Hello World'); }");
     p.deleteFile('.dart_tool/package_config.json');
 
-    final cachedDillFile = File(computeCachedDillPath(p.mainPath));
+    final cachedDillFile = File(
+      computeCachedDillAndCompilerOptionsPaths(p.mainPath).cachedDillPath,
+    );
     expect(cachedDillFile.existsSync(), false);
 
     final result = await p.run([
@@ -1108,7 +1117,9 @@ void residentRun() {
       () async {
     p = project(mainSrc: "void main() { print('Hello World'); }");
 
-    final cachedDillFile = File(computeCachedDillPath(p.mainPath));
+    final cachedDillFile = File(
+      computeCachedDillAndCompilerOptionsPaths(p.mainPath).cachedDillPath,
+    );
     expect(cachedDillFile.existsSync(), false);
 
     final result = await p.run([
@@ -1135,7 +1146,9 @@ void residentRun() {
       () async {
     p = project(mainSrc: "void main() { print('Hello World'); }");
 
-    final cachedDillFile = File(computeCachedDillPath(p.mainPath));
+    final cachedDillFile = File(
+      computeCachedDillAndCompilerOptionsPaths(p.mainPath).cachedDillPath,
+    );
     expect(cachedDillFile.existsSync(), false);
 
     final result = await p.run([
@@ -1257,8 +1270,12 @@ void residentRun() {
       ),
     );
 
-    final cachedDillFile = File(computeCachedDillPath(p.mainPath));
+    final (:cachedDillPath, :cachedCompilerOptionsPath) =
+        computeCachedDillAndCompilerOptionsPaths(p.mainPath);
+    final cachedDillFile = File(cachedDillPath);
     expect(cachedDillFile.existsSync(), false);
+    final cachedCompilerOptionsFile = File(cachedCompilerOptionsPath);
+    expect(cachedCompilerOptionsFile.existsSync(), false);
 
     final result = await p.run([
       'run',
@@ -1277,8 +1294,24 @@ void residentRun() {
       ),
     );
     expect(result.exitCode, 0);
+
     expect(cachedDillFile.existsSync(), true);
     cachedDillFile.deleteSync();
+
+    expect(cachedCompilerOptionsFile.existsSync(), true);
+    final cachedCompilerOptionsFileContents =
+        cachedCompilerOptionsFile.readAsStringSync();
+    cachedCompilerOptionsFile.deleteSync();
+
+    // [cachedCompilerOptionsFileContents] should be a valid JSON list that
+    // contains the enabled experiment.
+    final cachedCompilerOptionsAsList =
+        (jsonDecode(cachedCompilerOptionsFileContents) as List<dynamic>)
+            .cast<String>();
+    expect(
+      cachedCompilerOptionsAsList,
+      contains('--enable-experiment=test-experiment'),
+    );
   });
 
   test('same server used from different directories', () async {
@@ -1320,12 +1353,18 @@ void residentRun() {
     p.file('lib/main.dart', 'void main() {}');
     p.file('bin/main.dart', 'void main() {}');
 
-    final cachedDillForLibMain =
-        File(computeCachedDillPath(path.join(p.dirPath, 'lib/main.dart')));
+    final cachedDillForLibMain = File(
+      computeCachedDillAndCompilerOptionsPaths(
+        path.join(p.dirPath, 'lib/main.dart'),
+      ).cachedDillPath,
+    );
     expect(cachedDillForLibMain.existsSync(), false);
 
-    final cachedDillForBinMain =
-        File(computeCachedDillPath(path.join(p.dirPath, 'bin/main.dart')));
+    final cachedDillForBinMain = File(
+      computeCachedDillAndCompilerOptionsPaths(
+        path.join(p.dirPath, 'bin/main.dart'),
+      ).cachedDillPath,
+    );
     expect(cachedDillForBinMain.existsSync(), false);
 
     final runResult1 = await p.run([

pkg/frontend_server/lib/resident_frontend_server_utils.dart

@@ -49,9 +49,14 @@ final class ResidentCompilerInfo {
   });
 }
 
-/// Returns the absolute path to the cached kernel file associated with
-/// [canonicalizedLibraryPath].
-String computeCachedDillPath(
+typedef CachedDillAndCompilerOptionsPaths = ({
+  String cachedDillPath,
+  String cachedCompilerOptionsPath
+});
+
+/// Returns the absolute paths to the cached kernel file and the cached compiler
+/// options file associated with [canonicalizedLibraryPath].
+CachedDillAndCompilerOptionsPaths computeCachedDillAndCompilerOptionsPaths(
   final String canonicalizedLibraryPath,
 ) {
   final String dirname = path.dirname(canonicalizedLibraryPath);
@@ -73,7 +78,14 @@ String computeCachedDillPath(
     );
   }
 
-  return path.join(cachedKernelDirectoryPath, '$basename.dill');
+  final String cachedDillPath =
+      path.join(cachedKernelDirectoryPath, '$basename.dill');
+  final String cachedCompilerOptionsPath =
+      path.join(cachedKernelDirectoryPath, '${basename}_options.json');
+  return (
+    cachedDillPath: cachedDillPath,
+    cachedCompilerOptionsPath: cachedCompilerOptionsPath
+  );
 }
 
 /// Sends a compilation [request] to the resident frontend compiler associated

pkg/frontend_server/lib/src/resident_frontend_server.dart

@@ -22,7 +22,10 @@ import 'package:kernel/kernel.dart' show Component, loadComponentFromBytes;
 import 'package:path/path.dart' as path;
 
 import '../frontend_server.dart';
-import '../resident_frontend_server_utils.dart' show computeCachedDillPath;
+import '../resident_frontend_server_utils.dart'
+    show
+        CachedDillAndCompilerOptionsPaths,
+        computeCachedDillAndCompilerOptionsPaths;
 
 /// Floor the system time by this amount in order to correctly detect modified
 /// source files on all platforms. This has no effect on correctness,
@@ -321,6 +324,8 @@ class ResidentFrontendServer {
   static const String _packageString = 'packages';
   static const String _successString = 'success';
   static const String _outputString = 'output-dill';
+  static const String _useCachedCompilerOptionsAsBaseString =
+      'useCachedCompilerOptionsAsBase';
   static const String _compileExpressionString = 'compileExpression';
   static const String _libraryUriString = 'libraryUri';
   static const String _rootLibraryUriString = 'rootLibraryUri';
@@ -351,11 +356,18 @@ class ResidentFrontendServer {
 
   /// Returns a [ResidentCompiler] that has been configured with
   /// [compileOptions] and prepared to compile the [canonicalizedLibraryPath]
-  /// entrypoint.
-  static ResidentCompiler _getResidentCompilerForEntrypoint(
-    String canonicalizedLibraryPath,
-    ArgResults compileOptions,
-  ) {
+  /// entrypoint. This function also writes [compileOptions.arguments] to
+  /// [cachedCompilerOptions] as a JSON list.
+  static ResidentCompiler _getResidentCompilerForEntrypoint({
+    required final String canonicalizedLibraryPath,
+    required final ArgResults compileOptions,
+    required final File cachedCompilerOptions,
+  }) {
+    cachedCompilerOptions.createSync();
+    cachedCompilerOptions.writeAsStringSync(
+      compileOptions.arguments.map(jsonEncode).toList().toString(),
+    );
+
     late final ResidentCompiler residentCompiler;
     if (compilers[canonicalizedLibraryPath] == null) {
       // Avoids using too much memory.
@@ -398,8 +410,14 @@ class ResidentFrontendServer {
         component.mainMethod!.enclosingLibrary.fileUri.toFilePath(),
       );
 
-      final String cachedDillPath =
-          computeCachedDillPath(canonicalizedLibraryPath);
+      final String cachedDillPath;
+      try {
+        cachedDillPath =
+            computeCachedDillAndCompilerOptionsPaths(canonicalizedLibraryPath)
+                .cachedDillPath;
+      } on Exception catch (e) {
+        return _encodeErrorMessage(e.toString());
+      }
       replacementDillFile.copySync(cachedDillPath);
     } catch (e) {
       return _encodeErrorMessage('Failed to replace cached dill');
@@ -427,21 +445,29 @@ class ResidentFrontendServer {
     final String canonicalizedExecutablePath =
         path.canonicalize(request[_executableString]);
 
-    late final String cachedDillPath;
+    final String cachedDillPath;
+    final File cachedCompilerOptions;
     try {
-      cachedDillPath = computeCachedDillPath(canonicalizedExecutablePath);
+      final CachedDillAndCompilerOptionsPaths computationResult =
+          computeCachedDillAndCompilerOptionsPaths(canonicalizedExecutablePath);
+      cachedDillPath = computationResult.cachedDillPath;
+      cachedCompilerOptions =
+          new File(computationResult.cachedCompilerOptionsPath);
     } on Exception catch (e) {
       return _encodeErrorMessage(e.toString());
     }
 
     final ArgResults options = _generateCompilerOptions(
       request: request,
+      cachedCompilerOptions: cachedCompilerOptions,
       outputDillOverride: cachedDillPath,
       initializeFromDillPath: cachedDillPath,
     );
+
     final ResidentCompiler residentCompiler = _getResidentCompilerForEntrypoint(
-      canonicalizedExecutablePath,
-      options,
+      canonicalizedLibraryPath: canonicalizedExecutablePath,
+      compileOptions: options,
+      cachedCompilerOptions: cachedCompilerOptions,
     );
     final Map<String, dynamic> response = await residentCompiler.compile();
 
@@ -488,8 +514,17 @@ class ResidentFrontendServer {
       );
     }
 
-    final String cachedDillPath =
-        computeCachedDillPath(canonicalizedLibraryPath);
+    final String cachedDillPath;
+    final File cachedCompilerOptions;
+    try {
+      final CachedDillAndCompilerOptionsPaths computationResult =
+          computeCachedDillAndCompilerOptionsPaths(canonicalizedLibraryPath);
+      cachedDillPath = computationResult.cachedDillPath;
+      cachedCompilerOptions =
+          new File(computationResult.cachedCompilerOptionsPath);
+    } on Exception catch (e) {
+      return _encodeErrorMessage(e.toString());
+    }
     // Make the [ResidentCompiler] output the compiled expression to
     // [compiledExpressionDillPath] to prevent it from overwriting the
     // cached program dill.
@@ -499,14 +534,19 @@ class ResidentFrontendServer {
       null,
       '.expr.dill',
     );
+
     final ArgResults options = _generateCompilerOptions(
       request: request,
+      cachedCompilerOptions: cachedCompilerOptions,
       outputDillOverride: compiledExpressionDillPath,
       initializeFromDillPath: cachedDillPath,
     );
 
-    final ResidentCompiler residentCompiler =
-        _getResidentCompilerForEntrypoint(canonicalizedLibraryPath, options);
+    final ResidentCompiler residentCompiler = _getResidentCompilerForEntrypoint(
+      canonicalizedLibraryPath: canonicalizedLibraryPath,
+      compileOptions: options,
+      cachedCompilerOptions: cachedCompilerOptions,
+    );
 
     final String expression = request[_expressionString];
     final List<String> definitions =
@@ -580,13 +620,32 @@ class ResidentFrontendServer {
   /// Generates the compiler options needed to handle the [request].
   static ArgResults _generateCompilerOptions({
     required Map<String, dynamic> request,
+    required File cachedCompilerOptions,
 
     /// The compiled kernel file will be stored at this path, and not at
     /// [request['--output-dill']].
     required String outputDillOverride,
     required String initializeFromDillPath,
   }) {
-    return argParser.parse(<String>[
+    final Map<String, dynamic> options = {};
+    if (request[_useCachedCompilerOptionsAsBaseString] == true &&
+        cachedCompilerOptions.existsSync()) {
+      // If [request[_useCachedCompilerOptionsAsBaseString]] is true, then we
+      // start with the cached options and apply any options specified in
+      // [request] as overrides.
+      final String cachedCompilerOptionsContents =
+          cachedCompilerOptions.readAsStringSync();
+      final List<String> cachedCompilerOptionsAsList =
+          (jsonDecode(cachedCompilerOptionsContents) as List<dynamic>)
+              .cast<String>();
+      final ArgResults cachedOptions =
+          argParser.parse(cachedCompilerOptionsAsList);
+      for (final String option in cachedOptions.options) {
+        options[option] = cachedOptions[option];
+      }
+    }
+
+    final ArgResults overrides = argParser.parse(<String>[
       '--sdk-root=${_sdkUri.toFilePath()}',
       if (!(request['aot'] ?? false)) '--incremental',
       '--platform=${_platformKernelUri.path}',
@@ -623,6 +682,28 @@ class ResidentFrontendServer {
       if (request['enable-experiment'] != null)
         for (String experiment in request['enable-experiment']) experiment,
     ]);
+
+    for (final String option in overrides.options) {
+      options[option] = overrides[option];
+    }
+
+    // Transform [options] into a list that can be passed to [argParser.parse].
+    final List<String> optionsAsList = <String>[];
+    for (final MapEntry<String, dynamic>(:key, :value) in options.entries) {
+      if (value is List<dynamic>) {
+        for (final Object multiOptionValue in value) {
+          optionsAsList.add('--$key=$multiOptionValue');
+        }
+      } else if (value is bool) {
+        if (value) {
+          optionsAsList.add('--$key');
+        }
+      } else {
+        optionsAsList.add('--$key=$value');
+      }
+    }
+
+    return argParser.parse(optionsAsList);
   }
 
   /// Encodes the [message] in JSON to be sent over the socket.