Optimize skip lists and locations

Author: srujzs

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/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,12 +152,33 @@ class Locations {
 
   Modules get modules => _modules;
 
-  void initialize(String entrypoint) {
-    _sourceToTokenPosTable.clear();
-    _sourceToLocation.clear();
-    _locationMemoizer.clear();
-    _moduleToLocations.clear();
-    _entrypoint = entrypoint;
+  Future<void> initialize(
+    String entrypoint, [
+    InvalidatedModuleReport? invalidatedModuleReport,
+  ]) async {
+    // If we know that only certain modules are deleted or added, we can only
+    // invalidate those.
+    if (invalidatedModuleReport != null) {
+      for (final module in invalidatedModuleReport.deletedModules.union(
+        invalidatedModuleReport.reloadedModules,
+      )) {
+        _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);
+          }
+        }
+      }
+    } else {
+      _sourceToTokenPosTable.clear();
+      _locationMemoizer.clear();
+      _sourceToLocation.clear();
+      _moduleToLocations.clear();
+      _entrypoint = entrypoint;
+    }
   }
 
   /// Returns all [Location] data for a provided Dart source.

dwds/lib/src/debugging/metadata/provider.dart

@@ -215,7 +215,9 @@ class MetadataProvider {
     await _metadataMemoizer.runOnce(() => _processMetadata(false));
   }
 
-  Future<void> reinitializeAfterReload(Set<String> reloadedModules) async {
+  Future<InvalidatedModuleReport> reinitializeAfterReload(
+    Map<String, List> reloadedModulesToLibraries,
+  ) async {
     final modules = (await _processMetadata(true))!;
     final invalidatedLibraries = <String>{};
     void invalidateLibrary(String libraryImportUri) {
@@ -227,10 +229,9 @@ class MetadataProvider {
     }
 
     final deletedModules = <String>{};
-    // final invalidatedModules = <String>{};
     for (final module in _moduleToLibraries.keys) {
       final deletedModule = !modules.containsKey(module);
-      final invalidatedModule = reloadedModules.contains(module);
+      final invalidatedModule = reloadedModulesToLibraries.containsKey(module);
       assert(!(deletedModule && invalidatedModule));
       // If the module was either deleted or reloaded, invalidate all previous
       // information both about the module and its libraries.
@@ -242,14 +243,27 @@ class MetadataProvider {
       }
       if (deletedModule) deletedModules.add(module);
     }
-    for (final module in reloadedModules) {
+    final reloadedModules = <String>{};
+    final reloadedLibraries = <String>{};
+    for (final module in reloadedModulesToLibraries.keys) {
+      reloadedModules.add(module);
+      reloadedLibraries.addAll(
+        reloadedModulesToLibraries[module]!.cast<String>(),
+      );
       _addMetadata(modules[module]!);
     }
     // The libraries that were removed from the program or those that we
     // invalidated but were never added again.
-    // final deletedLibraries = invalidatedLibraries.where(
-    //   (library) => !_libraries.contains(library),
-    // );
+    final deletedLibraries =
+        invalidatedLibraries
+            .where((library) => !_libraries.contains(library))
+            .toSet();
+    return InvalidatedModuleReport(
+      deletedModules: deletedModules,
+      deletedLibraries: deletedLibraries,
+      reloadedModules: reloadedModules,
+      reloadedLibraries: reloadedLibraries,
+    );
   }
 
   void _addMetadata(ModuleMetadata metadata) {
@@ -303,3 +317,18 @@ class AbsoluteImportUriException implements Exception {
   @override
   String toString() => "AbsoluteImportUriError: '$importUri'";
 }
+
+class InvalidatedModuleReport {
+  final Set<String> deletedModules;
+  final Set<String> deletedLibraries;
+  // The union of invalidated and new modules.
+  final Set<String> reloadedModules;
+  // The union of invalidated and new libraries.
+  final Set<String> reloadedLibraries;
+  InvalidatedModuleReport({
+    required this.deletedModules,
+    required this.deletedLibraries,
+    required this.reloadedModules,
+    required this.reloadedLibraries,
+  });
+}

dwds/lib/src/debugging/modules.dart

@@ -5,6 +5,7 @@
 import 'package:async/async.dart';
 import 'package:dwds/src/config/tool_configuration.dart';
 import 'package:dwds/src/debugging/debugger.dart';
+import 'package:dwds/src/debugging/metadata/provider.dart';
 import 'package:dwds/src/utilities/dart_uri.dart';
 import 'package:logging/logging.dart';
 
@@ -16,6 +17,9 @@ class Modules {
   // The Dart server path to containing module.
   final _sourceToModule = <String, String>{};
 
+  // Module to Dart server paths.
+  final _moduleToSources = <String, Set<String>>{};
+
   // The Dart server path to library import uri
   final _sourceToLibrary = <String, Uri>{};
   var _moduleMemoizer = AsyncMemoizer<void>();
@@ -31,9 +35,13 @@ class Modules {
   /// Intended to be called multiple times throughout the development workflow,
   /// e.g. after a hot-reload.
   void initialize(String entrypoint) {
-    // We only clear the source to module mapping as script IDs may persist
-    // across hot reloads.
+    // TODO(srujzs): Can we do better and only invalidate the sources/modules
+    // that were deleted/reloaded? This would require removing the
+    // deleted/reloaded libraries/sources/modules from the following maps and
+    // then only processing that set in `_initializeMapping`. It's doable, but
+    // these calculations are also not that expensive.
     _sourceToModule.clear();
+    _moduleToSources.clear();
     _sourceToLibrary.clear();
     _libraryToModule.clear();
     _moduleMemoizer = AsyncMemoizer();
@@ -46,6 +54,12 @@ class Modules {
     return _sourceToModule[serverPath];
   }
 
+  /// Returns the Dart server paths for the provided module.
+  Future<Set<String>?> sourcesForModule(String module) async {
+    await _moduleMemoizer.runOnce(_initializeMapping);
+    return _moduleToSources[module];
+  }
+
   /// Returns the containing library importUri for the provided Dart server path.
   Future<Uri?> libraryForSource(String serverPath) async {
     await _moduleMemoizer.runOnce(_initializeMapping);
@@ -69,11 +83,15 @@ class Modules {
   ) async {
     final serverPath = await globalToolConfiguration.loadStrategy
         .serverPathForModule(entrypoint, module);
+    // TODO(srujzs): We should wait until all scripts are parsed before
+    // accessing.
     return chromePathToRuntimeScriptId[serverPath];
   }
 
-  /// Initializes [_sourceToModule] and [_sourceToLibrary].
-  Future<void> _initializeMapping() async {
+  /// Initializes [_sourceToModule], [_moduleToSources], and [_sourceToLibrary].
+  Future<void> _initializeMapping([
+    InvalidatedModuleReport? invalidatedModuleReport,
+  ]) async {
     final provider = globalToolConfiguration.loadStrategy.metadataProviderFor(
       _entrypoint,
     );
@@ -92,6 +110,7 @@ class Modules {
         final module = scriptToModule[library]!;
 
         _sourceToModule[libraryServerPath] = module;
+        _moduleToSources.putIfAbsent(module, () => {}).add(libraryServerPath);
         _sourceToLibrary[libraryServerPath] = Uri.parse(library);
         _libraryToModule[library] = module;
 
@@ -102,6 +121,7 @@ class Modules {
                   : DartUri(script, _root).serverPath;
 
           _sourceToModule[scriptServerPath] = module;
+          _moduleToSources[module]!.add(scriptServerPath);
           _sourceToLibrary[scriptServerPath] = Uri.parse(library);
         }
       } else {

dwds/lib/src/debugging/skip_list.dart

@@ -2,23 +2,59 @@
 // 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 'package:dwds/src/config/tool_configuration.dart';
 import 'package:dwds/src/debugging/location.dart';
+import 'package:dwds/src/debugging/metadata/provider.dart';
+import 'package:dwds/src/utilities/dart_uri.dart';
 
 const maxValue = 2147483647;
 
 class SkipLists {
   // Map of script ID to scriptList.
   final _idToList = <String, List<Map<String, dynamic>>>{};
+  // Map of url to script ID.
+  final _urlToId = <String, String>{};
+  final String _root;
 
-  void initialize() => _idToList.clear();
+  SkipLists(this._root);
+
+  Future<void> initialize([
+    String? entrypoint,
+    InvalidatedModuleReport? invalidatedModuleReport,
+  ]) async {
+    if (invalidatedModuleReport != null) {
+      assert(entrypoint != null);
+      final invalidatedModules = invalidatedModuleReport.deletedModules.union(
+        invalidatedModuleReport.reloadedModules,
+      );
+      for (final url in _urlToId.keys) {
+        if (url.isEmpty) continue;
+
+        final dartUri = DartUri(url, _root);
+        final serverPath = dartUri.serverPath;
+        final module = await globalToolConfiguration.loadStrategy
+            .moduleForServerPath(entrypoint!, serverPath);
+        if (invalidatedModules.contains(module)) {
+          _idToList.remove(_urlToId[url]!);
+        }
+      }
+    } else {
+      _idToList.clear();
+    }
+  }
 
   /// Returns a skipList as defined by the Chrome DevTools Protocol.
   ///
   /// A `skipList` is an array of `LocationRange`s see:
   /// https://chromedevtools.github.io/devtools-protocol/tot/Debugger/#method-stepInto
   ///
   /// Can return a cached value.
-  List<Map<String, dynamic>> compute(String scriptId, Set<Location> locations) {
+  List<Map<String, dynamic>> compute(
+    String scriptId,
+    String url,
+    Set<Location> locations,
+  ) {
+    _urlToId[url] = scriptId;
     if (_idToList.containsKey(scriptId)) return _idToList[scriptId]!;
 
     final sortedLocations =

dwds/lib/src/loaders/strategy.dart

@@ -183,12 +183,12 @@ abstract class LoadStrategy {
     return Future.value();
   }
 
-  Future<void> reinitializeEntrypointAfterReload(
+  Future<InvalidatedModuleReport> reinitializeEntrypointAfterReload(
     String entrypoint,
-    Set<String> modules,
+    Map<String, List> reloadedModulesToLibraries,
   ) {
     final provider = _providers[entrypoint]!;
-    return provider.reinitializeAfterReload(modules);
+    return provider.reinitializeAfterReload(reloadedModulesToLibraries);
   }
 }
 

dwds/lib/src/services/chrome_proxy_service.dart

@@ -17,6 +17,7 @@ import 'package:dwds/src/debugging/execution_context.dart';
 import 'package:dwds/src/debugging/inspector.dart';
 import 'package:dwds/src/debugging/instance.dart';
 import 'package:dwds/src/debugging/location.dart';
+import 'package:dwds/src/debugging/metadata/provider.dart';
 import 'package:dwds/src/debugging/modules.dart';
 import 'package:dwds/src/debugging/remote_debugger.dart';
 import 'package:dwds/src/debugging/skip_list.dart';
@@ -196,7 +197,7 @@ class ChromeProxyService implements VmServiceInterface {
 
     final modules = Modules(root);
     final locations = Locations(assetReader, modules, root);
-    final skipLists = SkipLists();
+    final skipLists = SkipLists(root);
     final service = ChromeProxyService._(
       vm,
       root,
@@ -245,18 +246,20 @@ class ChromeProxyService implements VmServiceInterface {
     Map<String, List> reloadedModules,
   ) async {
     final entrypoint = inspector.appConnection.request.entrypointPath;
-    final modules = reloadedModules.keys.toSet();
-    await globalToolConfiguration.loadStrategy
-        .reinitializeEntrypointAfterReload(entrypoint, modules);
-    _initializeEntrypoint(entrypoint);
+    final invalidatedModuleReport = await globalToolConfiguration.loadStrategy
+        .reinitializeEntrypointAfterReload(entrypoint, reloadedModules);
+    await _initializeEntrypoint(entrypoint, invalidatedModuleReport);
     await inspector.initialize();
   }
 
   /// Initializes metadata in [Locations], [Modules], and [ExpressionCompiler].
-  void _initializeEntrypoint(String entrypoint) {
-    _locations.initialize(entrypoint);
+  Future<void> _initializeEntrypoint(
+    String entrypoint, [
+    InvalidatedModuleReport? invalidatedModuleReport,
+  ]) async {
     _modules.initialize(entrypoint);
-    _skipLists.initialize();
+    await _locations.initialize(entrypoint, invalidatedModuleReport);
+    await _skipLists.initialize(entrypoint, invalidatedModuleReport);
     // We do not need to wait for compiler dependencies to be updated as the
     // [ExpressionEvaluator] is robust to evaluation requests during updates.
     safeUnawaited(_updateCompilerDependencies(entrypoint));

dwds/test/debugger_test.dart

@@ -88,8 +88,8 @@ void main() async {
       FakeModules(),
       root,
     );
-    locations.initialize('fake_entrypoint');
-    skipLists = SkipLists();
+    await locations.initialize('fake_entrypoint');
+    skipLists = SkipLists(root);
     debugger = await Debugger.create(
       webkitDebugger,
       (_, __) {},

dwds/test/expression_evaluator_test.dart

@@ -54,9 +54,9 @@ void main() async {
       final root = 'fakeRoot';
       final entry = 'fake_entrypoint';
       final locations = Locations(assetReader, modules, root);
-      locations.initialize(entry);
+      await locations.initialize(entry);
 
-      final skipLists = SkipLists();
+      final skipLists = SkipLists(root);
       final debugger = await Debugger.create(
         webkitDebugger,
         (_, e) => debugEventController.sink.add(e),

dwds/test/fixtures/context.dart

@@ -412,7 +412,7 @@ class TestContext {
                   'remote-debugging-port=$debugPort',
                   if (enableDebugExtension)
                     '--load-extension=debug_extension/prod_build',
-                  if (headless) '--headless',
+                  // if (headless) '--headless',
                 ],
               },
             });