Adding support for non-hot-reloadable packages in DDC.

These packages can be specified with the `non-hot-reloadable-package` flag and will be compiled without their members being properly hot reloadable. In exchange, we can perform additional optimizations (such as avoiding levels of indirection/checks in anticipation of their being updated).

This additionally overhauls our memory compiler to use the incremental compiler for proper state-passing hot reload testing + adds tests for this functionality.

Change-Id: Ief65896410f32655f064bd15728703629faa4051
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428343
Reviewed-by: Nate Biggs <[email protected]>
Reviewed-by: Nicholas Shahan <[email protected]>
Commit-Queue: Mark Zhou <[email protected]>

Author: Markzipan

pkg/dev_compiler/lib/src/command/command.dart

@@ -363,7 +363,8 @@ Future<CompilerResult> _compile(List<String> args,
       final lastAcceptedComponent = Component();
       kernel.BinaryBuilder((await File(reloadLastAcceptedKernel).readAsBytes()))
           .readComponent(lastAcceptedComponent);
-      final deltaInspector = HotReloadDeltaInspector();
+      final deltaInspector = HotReloadDeltaInspector(
+          nonHotReloadablePackages: options.nonHotReloadablePackages);
       final rejectionReasons = deltaInspector.compareGenerations(
           lastAcceptedComponent, compiledLibraries);
       if (rejectionReasons.isNotEmpty) {

pkg/dev_compiler/lib/src/command/options.dart

@@ -63,6 +63,10 @@ class Options {
 
   final Map<String, String> summaryModules;
 
+  /// Packages that should not be hot reloaded. DDC can perform special
+  /// optimizations on these packages and will throw if they are hot reloaded.
+  final Set<String> nonHotReloadablePackages;
+
   final List<ModuleFormat> moduleFormats;
 
   /// The name of the module.
@@ -117,6 +121,7 @@ class Options {
       this.reloadLastAcceptedKernel,
       this.reloadDeltaKernel,
       this.summaryModules = const {},
+      this.nonHotReloadablePackages = const {},
       this.moduleFormats = const [],
       required this.moduleName,
       this.multiRootScheme = 'org-dartlang-app',

pkg/dev_compiler/lib/src/kernel/compiler_new.dart

@@ -758,6 +758,19 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
 
   bool _inFunctionExpression = false;
 
+  /// Returns whether or not [uri] can be hot reloaded.
+  ///
+  /// These packages are not updated during the runtime of a standard program,
+  /// allowing us to make the following optimizations:
+  ///  - methods, getters, and setters are emitted without additional levels
+  ///    of indirection
+  ///  - fields are emitted without additional lazy initialization checks.
+  bool _isNonHotReloadableResource(Uri uri) {
+    return (uri.isScheme('dart')) ||
+        (uri.isScheme('package') &&
+            _options.nonHotReloadablePackages.contains(uri.pathSegments[0]));
+  }
+
   /// Module can be emitted only once, and the compiler can be reused after
   /// only in incremental mode, for expression compilation only.
   js_ast.Program emitLibrary(Library library) {
@@ -2761,11 +2774,13 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     var virtualFieldSymbol = _emitFieldValueAccessor(field);
     var name = _declareMemberName(field);
     var initializer = _visitInitializer(field.initializer, field.annotations);
-    var getter = _emitLazyInitializingFunction(
-      js.call('this.#', virtualFieldSymbol),
-      initializer,
-      field,
-    );
+    var getter = _isNonHotReloadableResource(_currentLibrary!.importUri)
+        ? js.fun('function() { return this[#]; }', [virtualFieldSymbol])
+        : _emitLazyInitializingFunction(
+            js.call('this.#', virtualFieldSymbol),
+            initializer,
+            field,
+          );
     var jsGetter = js_ast.Method(name, getter, isGetter: true)
       ..sourceInformation = _nodeStart(field);
 
@@ -3124,8 +3139,21 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
 
       var initializer =
           _visitInitializer(member.initializer, member.annotations);
-      var getter = _emitLazyInitializingFunction(
-          js.call('this.#', memberValueStore), initializer, member);
+
+      js_ast.Fun getter;
+      if (_isNonHotReloadableResource(_currentLibrary!.importUri)) {
+        getter = js.fun(
+            'function() { return this[#] === void 0 ? this[#] = # : this[#]; }',
+            [
+              memberValueStore,
+              memberValueStore,
+              initializer,
+              memberValueStore
+            ]);
+      } else {
+        getter = _emitLazyInitializingFunction(
+            js.call('this.#', memberValueStore), initializer, member);
+      }
       properties.add(js_ast.Method(access, getter,
           isGetter: true,
           isStatic: member.isStatic && member.enclosingClass != null)
@@ -3134,7 +3162,6 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
           member.fileOffset,
           member.name.text.length,
         ));
-
       if (!member.isFinal && !member.isConst) {
         var body = <js_ast.Statement>[];
         var param = _emitIdentifier('v');

pkg/dev_compiler/lib/src/kernel/hot_reload_delta_inspector.dart

@@ -18,6 +18,14 @@ class HotReloadDeltaInspector {
   /// generation.
   final _rejectionMessages = <String>[];
 
+  /// A set of packages that should not be hot reloaded.
+  ///
+  /// The delta inspector will reject any component that modifies any such
+  /// packages (while allowing them to be introduced).
+  final Set<String> nonHotReloadablePackages;
+
+  HotReloadDeltaInspector({this.nonHotReloadablePackages = const {}});
+
   /// Returns all hot reload rejection errors discovered while comparing [delta]
   /// against the [lastAccepted] version.
   ///
@@ -42,6 +50,12 @@ class HotReloadDeltaInspector {
         // No previous version of the library to compare with.
         continue;
       }
+      if (_shouldNotCompileWithHotReload(deltaLibrary.importUri)) {
+        _rejectionMessages
+            .add('Attempting to hot reload a modified library from a package '
+                'marked as non-hot-reloadable: '
+                "Library: '${deltaLibrary.importUri}'");
+      }
       var libraryMetadata = metadataRepo.mapping
           .putIfAbsent(deltaLibrary, HotReloadLibraryMetadata.new);
       // TODO(60281): Handle members when an entire library has been deleted
@@ -158,6 +172,16 @@ class HotReloadDeltaInspector {
           acceptedProcedure.name.text,
     ];
   }
+
+  /// Returns `true` if the resource at [uri] should not be compiled with hot
+  /// reload.
+  ///
+  /// No 'dart:' libraries will be compiled with hot reload support.
+  bool _shouldNotCompileWithHotReload(Uri uri) {
+    return (uri.isScheme('dart')) ||
+        (uri.isScheme('package') &&
+            nonHotReloadablePackages.contains(uri.pathSegments[0]));
+  }
 }
 
 const hotReloadLibraryMetadataTag = 'ddc.hot-reload-library.metadata';

pkg/dev_compiler/test/hot_reload_delta_inspector_test.dart

@@ -348,24 +348,150 @@ Future<void> main() async {
           orderedEquals(['deletedSetter']));
     });
   });
+
+  group('Non-hot-reloadable packages ', () {
+    final packageName = 'test_package';
+
+    final deltaInspector =
+        HotReloadDeltaInspector(nonHotReloadablePackages: {packageName});
+    test('reject reloads when a member is added.', () async {
+      final initialAndDeltaSource = '''
+          import 'package:$packageName/file.dart';
+          main() {}
+          ''';
+      final initialPackageSource = 'class Foo {}';
+      final deltaPackageSource = 'class Foo { int member = 100; }';
+      final (:initial, :delta) = await compileComponents(
+        initialAndDeltaSource,
+        initialAndDeltaSource,
+        initialPackageSource: initialPackageSource,
+        deltaPackageSource: deltaPackageSource,
+        packageName: packageName,
+      );
+      expect(
+          deltaInspector.compareGenerations(initial, delta),
+          unorderedEquals([
+            'Attempting to hot reload a modified library from a package '
+                'marked as non-hot-reloadable: '
+                "Library: 'package:$packageName/file.dart'"
+          ]));
+    });
+    test('reject reloads when a member is removed.', () async {
+      final initialAndDeltaSource = '''
+          import 'package:$packageName/file.dart';
+          main() {}
+          ''';
+      final initialPackageSource = 'class Foo { int member = 100; }';
+      final deltaPackageSource = 'class Foo {}';
+      final (:initial, :delta) = await compileComponents(
+        initialAndDeltaSource,
+        initialAndDeltaSource,
+        initialPackageSource: initialPackageSource,
+        deltaPackageSource: deltaPackageSource,
+        packageName: packageName,
+      );
+      expect(
+          deltaInspector.compareGenerations(initial, delta),
+          unorderedEquals([
+            'Attempting to hot reload a modified library from a package '
+                'marked as non-hot-reloadable: '
+                "Library: 'package:$packageName/file.dart'"
+          ]));
+    });
+    test('accept reloads when introduced but not modified.', () async {
+      final initialSource = '''
+          main() {}
+          ''';
+      final initialAndDeltaPackageSource = 'class Foo { int member = 100; }';
+      final deltaSource = '''
+          import 'package:$packageName/file.dart';
+          main() {}
+          ''';
+      final (:initial, :delta) = await compileComponents(
+        initialSource,
+        deltaSource,
+        initialPackageSource: initialAndDeltaPackageSource,
+        deltaPackageSource: initialAndDeltaPackageSource,
+        packageName: packageName,
+      );
+      expect(() => deltaInspector.compareGenerations(initial, delta),
+          returnsNormally);
+    });
+  });
 }
 
 /// Test only helper compiles [initialSource] and [deltaSource] and returns two
 /// kernel components.
+///
+/// Auto-generates a fake package_config.json if [packageName] is provided.
+/// Supports a single package named [packageName] containing a single file
+/// whose source contents across one generation are [initialPackageSource] and
+/// [deltaPackageSource].
 Future<({Component initial, Component delta})> compileComponents(
-    String initialSource, String deltaSource) async {
+    String initialSource, String deltaSource,
+    {Uri? baseUri,
+    String? packageName,
+    String initialPackageSource = '',
+    String deltaPackageSource = ''}) async {
+  baseUri ??= memoryDirectory;
+
   final fileName = 'main.dart';
-  final fileUri = Uri(scheme: 'memory', host: '', path: fileName);
+  final packageFileName = 'lib/file.dart';
+  final fileUri = Uri(scheme: baseUri.scheme, host: '', path: fileName);
   final memoryFileMap = {fileName: initialSource};
-  final initialResult = await componentFromMemory(memoryFileMap, fileUri);
+
+  // Generate a fake package_config.json and package.
+  Uri? packageConfigUri;
+  if (packageName != null) {
+    packageConfigUri = baseUri.resolve('package_config.json');
+    memoryFileMap['package_config.json'] =
+        generateFakePackagesFile(packageName: packageName);
+    memoryFileMap[packageFileName] = initialPackageSource;
+  }
+  final initialResult = await incrementalComponentFromMemory(
+    memoryFileMap,
+    fileUri,
+    baseUri: baseUri,
+    packageConfigUri: packageConfigUri,
+  );
   expect(initialResult.errors, isEmpty,
       reason: 'Initial source produced compile time errors.');
+
   memoryFileMap[fileName] = deltaSource;
-  final deltaResult = await componentFromMemory(memoryFileMap, fileUri);
+  if (packageName != null) {
+    memoryFileMap[packageFileName] = initialPackageSource;
+  }
+  final deltaResult = await incrementalComponentFromMemory(
+    memoryFileMap,
+    fileUri,
+    baseUri: baseUri,
+    packageConfigUri: packageConfigUri,
+    initialCompilerState: initialResult.initialCompilerState,
+  );
   expect(deltaResult.errors, isEmpty,
       reason: 'Delta source produced compile time errors.');
   return (
     initial: initialResult.ddcResult.component,
     delta: deltaResult.ddcResult.component
   );
 }
+
+String generateFakePackagesFile({
+  required String packageName,
+  String rootUri = '/',
+  String packageUri = 'lib/',
+}) {
+  return '''
+{
+  "configVersion": 0,
+  "packages": [
+    {
+      "name": "$packageName",
+      "rootUri": "$rootUri",
+      "packageUri": "$packageUri",
+      "languageVersion": "3.4"
+    }
+  ]
+}
+''';
+}