Invalidate caches after a hot reload (#2641)

Many of the caches we computed on the original load need to be invalidated. Previously, we pessimistically tore down the caches and reinitialized them. With this PR, a ModifiedModuleReport is computed that allows us to only invalidate and recompute information for modules/libraries that were either deleted or reloaded.

Specifically, the caches in MetadataProvider, Modules, Locations, SkipLists, AppInspector, and LibraryHelper are now specially invalidated and recomputed.

Removes an existing option to use the module URI instead of the module name in the MetadataProvider. This option is not relevant until we can unify this change across the ecosystem.

Fixes #2628.

Author: srujzs

dwds/CHANGELOG.md

@@ -1,6 +1,7 @@
-## 24.4.0-wip
+## 24.4.0
 
 - Added support for breakpoint registering on a hot reload with the DDC library bundle format using PausePostRequests.
+- `FrontendServerDdcLibraryBundleStrategy.hotReloadSourceUri` is now expected to also provide the reloaded modules.
 
 ## 24.3.11
 

dwds/lib/src/debugging/debugger.dart

@@ -594,6 +594,7 @@ class Debugger extends Domain {
             params: {
               'skipList': _skipLists.compute(
                 scriptId,
+                url,
                 await _locations.locationsForUrl(url),
               ),
             },

dwds/lib/src/debugging/inspector.dart

@@ -13,6 +13,7 @@ import 'package:dwds/src/debugging/execution_context.dart';
 import 'package:dwds/src/debugging/instance.dart';
 import 'package:dwds/src/debugging/libraries.dart';
 import 'package:dwds/src/debugging/location.dart';
+import 'package:dwds/src/debugging/metadata/provider.dart';
 import 'package:dwds/src/debugging/remote_debugger.dart';
 import 'package:dwds/src/loaders/ddc_library_bundle.dart';
 import 'package:dwds/src/readers/asset_reader.dart';
@@ -34,7 +35,9 @@ import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
 class AppInspector implements AppInspectorInterface {
   var _scriptCacheMemoizer = AsyncMemoizer<List<ScriptRef>>();
 
-  Future<List<ScriptRef>> get scriptRefs => _populateScriptCaches();
+  Future<List<ScriptRef>> getScriptRefs({
+    ModifiedModuleReport? modifiedModuleReport,
+  }) => _populateScriptCaches(modifiedModuleReport: modifiedModuleReport);
 
   final _logger = Logger('AppInspector');
 
@@ -103,24 +106,35 @@ class AppInspector implements AppInspectorInterface {
 
   /// Reset all caches and recompute any mappings.
   ///
-  /// Should be called across hot reloads.
-  Future<void> initialize() async {
+  /// Should be called across hot reloads with a valid [ModifiedModuleReport].
+  Future<void> initialize({ModifiedModuleReport? modifiedModuleReport}) async {
     _scriptCacheMemoizer = AsyncMemoizer<List<ScriptRef>>();
-    _scriptRefsById.clear();
-    _serverPathToScriptRef.clear();
-    _scriptIdToLibraryId.clear();
-    _libraryIdToScriptRefs.clear();
 
-    _libraryHelper = LibraryHelper(this);
+    // TODO(srujzs): We can invalidate these in a smarter way instead of
+    // reinitializing when doing a hot reload, but these helpers recompute info
+    // on demand later and therefore are not in the critical path.
     _classHelper = ClassHelper(this);
     _instanceHelper = InstanceHelper(this);
 
+    if (modifiedModuleReport != null) {
+      // Invalidate `_libraryHelper` as we use it populate any script caches.
+      _libraryHelper.initialize(modifiedModuleReport: modifiedModuleReport);
+    } else {
+      _libraryHelper = LibraryHelper(this)..initialize();
+      _scriptRefsById.clear();
+      _serverPathToScriptRef.clear();
+      _scriptIdToLibraryId.clear();
+      _libraryIdToScriptRefs.clear();
+    }
+
     final libraries = await _libraryHelper.libraryRefs;
     isolate.rootLib = await _libraryHelper.rootLib;
     isolate.libraries?.clear();
     isolate.libraries?.addAll(libraries);
 
-    final scripts = await scriptRefs;
+    final scripts = await getScriptRefs(
+      modifiedModuleReport: modifiedModuleReport,
+    );
 
     await DartUri.initialize();
     DartUri.recordAbsoluteUris(libraries.map((lib) => lib.uri).nonNulls);
@@ -583,7 +597,7 @@ class AppInspector implements AppInspectorInterface {
   /// All the scripts in the isolate.
   @override
   Future<ScriptList> getScripts() async {
-    return ScriptList(scripts: await scriptRefs);
+    return ScriptList(scripts: await getScriptRefs());
   }
 
   /// Calls the Chrome Runtime.getProperties API for the object with [objectId].
@@ -714,19 +728,50 @@ class AppInspector implements AppInspectorInterface {
   ///
   /// This will get repopulated on restarts and reloads.
   ///
+  /// If [modifiedModuleReport] is provided, only invalidates and
+  /// recalculates caches for the modified libraries.
+  ///
   /// Returns the list of scripts refs cached.
-  Future<List<ScriptRef>> _populateScriptCaches() {
+  Future<List<ScriptRef>> _populateScriptCaches({
+    ModifiedModuleReport? modifiedModuleReport,
+  }) {
     return _scriptCacheMemoizer.runOnce(() async {
       final scripts =
           await globalToolConfiguration.loadStrategy
               .metadataProviderFor(appConnection.request.entrypointPath)
               .scripts;
+      if (modifiedModuleReport != null) {
+        // Invalidate any script caches that were computed for the now invalid
+        // libraries. They will get repopulated below.
+        for (final libraryUri in modifiedModuleReport.modifiedLibraries) {
+          final libraryRef = await _libraryHelper.libraryRefFor(libraryUri);
+          final libraryId = libraryRef?.id;
+          // If this was not a pre-existing library, nothing to invalidate.
+          if (libraryId == null) continue;
+          final scriptRefs = _libraryIdToScriptRefs.remove(libraryId);
+          if (scriptRefs == null) continue;
+          for (final scriptRef in scriptRefs) {
+            final scriptId = scriptRef.id;
+            final scriptUri = scriptRef.uri;
+            if (scriptId != null && scriptUri != null) {
+              _scriptRefsById.remove(scriptId);
+              _scriptIdToLibraryId.remove(scriptId);
+              _serverPathToScriptRef.remove(
+                DartUri(scriptUri, _root).serverPath,
+              );
+            }
+          }
+        }
+      }
       // For all the non-dart: libraries, find their parts and create scriptRefs
       // for them.
       final userLibraries = _userLibraryUris(
         isolate.libraries ?? <LibraryRef>[],
       );
       for (final uri in userLibraries) {
+        if (modifiedModuleReport?.modifiedLibraries.contains(uri) == false) {
+          continue;
+        }
         final parts = scripts[uri];
         final scriptRefs = [
           ScriptRef(uri: uri, id: createId()),

dwds/lib/src/debugging/libraries.dart

@@ -5,6 +5,7 @@
 import 'package:collection/collection.dart';
 import 'package:dwds/src/config/tool_configuration.dart';
 import 'package:dwds/src/debugging/metadata/class.dart';
+import 'package:dwds/src/debugging/metadata/provider.dart';
 import 'package:dwds/src/services/chrome_debug_exception.dart';
 import 'package:dwds/src/utilities/domain.dart';
 import 'package:logging/logging.dart';
@@ -27,6 +28,29 @@ class LibraryHelper extends Domain {
     inspector = appInspector;
   }
 
+  /// Initialize any caches.
+  ///
+  /// If [modifiedModuleReport] is not null, invalidates only modified libraries
+  /// from the cache and recomputes values for any eager caches.
+  void initialize({ModifiedModuleReport? modifiedModuleReport}) {
+    _rootLib = null;
+    if (modifiedModuleReport != null) {
+      for (final library in modifiedModuleReport.modifiedLibraries) {
+        // These will later be initialized by `libraryFor` if needed.
+        _librariesById.remove(library);
+        _libraryRefsById.remove(library);
+      }
+      for (final library in modifiedModuleReport.reloadedLibraries) {
+        // These need to be recomputed here as `libraryRefs` only checks if this
+        // map is empty before returning.
+        _libraryRefsById[library] = _createLibraryRef(library);
+      }
+      return;
+    }
+    _librariesById.clear();
+    _libraryRefsById.clear();
+  }
+
   Future<LibraryRef> get rootLib async {
     if (_rootLib != null) return _rootLib!;
     final libraries = await libraryRefs;
@@ -51,21 +75,21 @@ class LibraryHelper extends Domain {
     return _rootLib!;
   }
 
+  LibraryRef _createLibraryRef(String library) =>
+      LibraryRef(id: library, name: library, uri: library);
+
   /// Returns all libraryRefs in the app.
   ///
-  /// Note this can return a cached result.
+  /// Note this can return a cached result that can be selectively reinitialized
+  /// using [initialize].
   Future<List<LibraryRef>> get libraryRefs async {
     if (_libraryRefsById.isNotEmpty) return _libraryRefsById.values.toList();
     final libraries =
         await globalToolConfiguration.loadStrategy
             .metadataProviderFor(inspector.appConnection.request.entrypointPath)
             .libraries;
     for (final library in libraries) {
-      _libraryRefsById[library] = LibraryRef(
-        id: library,
-        name: library,
-        uri: library,
-      );
+      _libraryRefsById[library] = _createLibraryRef(library);
     }
     return _libraryRefsById.values.toList();
   }

dwds/lib/src/debugging/location.dart

@@ -4,6 +4,7 @@
 
 import 'package:async/async.dart';
 import 'package:dwds/src/config/tool_configuration.dart';
+import 'package:dwds/src/debugging/metadata/provider.dart';
 import 'package:dwds/src/debugging/modules.dart';
 import 'package:dwds/src/readers/asset_reader.dart';
 import 'package:dwds/src/utilities/dart_uri.dart';
@@ -151,11 +152,34 @@ class Locations {
 
   Modules get modules => _modules;
 
-  void initialize(String entrypoint) {
-    _sourceToTokenPosTable.clear();
-    _sourceToLocation.clear();
+  /// Initialize any caches.
+  ///
+  /// If [modifiedModuleReport] is not null, only invalidates the caches for the
+  /// modified modules instead.
+  Future<void> initialize(
+    String entrypoint, {
+    ModifiedModuleReport? modifiedModuleReport,
+  }) async {
+    // If we know that only certain modules are deleted or added, we can only
+    // invalidate those.
+    if (modifiedModuleReport != null) {
+      for (final module in modifiedModuleReport.modifiedModules) {
+        _locationMemoizer.remove(module);
+        _moduleToLocations.remove(module);
+        final sources = await _modules.sourcesForModule(module);
+        if (sources != null) {
+          for (final serverPath in sources) {
+            _sourceToTokenPosTable.remove(serverPath);
+            _sourceToLocation.remove(serverPath);
+          }
+        }
+      }
+      return;
+    }
     _locationMemoizer.clear();
     _moduleToLocations.clear();
+    _sourceToTokenPosTable.clear();
+    _sourceToLocation.clear();
     _entrypoint = entrypoint;
   }