Skip to content

Commit c09a432

Browse files
srujzssrujzs
committed
More optimizations
1 parent a15ad81 commit c09a432

File tree

5 files changed

+146
-44
lines changed

5 files changed

+146
-44
lines changed

‎dwds/lib/src/debugging/inspector.dart‎

Lines changed: 56 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'package:dwds/src/debugging/execution_context.dart';
1313
import 'package:dwds/src/debugging/instance.dart';
1414
import 'package:dwds/src/debugging/libraries.dart';
1515
import 'package:dwds/src/debugging/location.dart';
16+
import 'package:dwds/src/debugging/metadata/provider.dart';
1617
import 'package:dwds/src/debugging/remote_debugger.dart';
1718
import 'package:dwds/src/loaders/ddc_library_bundle.dart';
1819
import 'package:dwds/src/readers/asset_reader.dart';
@@ -34,7 +35,9 @@ import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
3435
class AppInspector implements AppInspectorInterface {
3536
var _scriptCacheMemoizer = AsyncMemoizer<List<ScriptRef>>();
3637

37-
Future<List<ScriptRef>> get scriptRefs => _populateScriptCaches();
38+
Future<List<ScriptRef>> getScriptRefs([
39+
InvalidatedModuleReport? invalidatedModuleReport,
40+
]) => _populateScriptCaches(invalidatedModuleReport);
3841

3942
final _logger = Logger('AppInspector');
4043

@@ -103,24 +106,35 @@ class AppInspector implements AppInspectorInterface {
103106

104107
/// Reset all caches and recompute any mappings.
105108
///
106-
/// Should be called across hot reloads.
107-
Future<void> initialize() async {
109+
/// Should be called across hot reloads with a valid [invalidatedModuleReport].
110+
Future<void> initialize([
111+
InvalidatedModuleReport? invalidatedModuleReport,
112+
]) async {
108113
_scriptCacheMemoizer = AsyncMemoizer<List<ScriptRef>>();
109-
_scriptRefsById.clear();
110-
_serverPathToScriptRef.clear();
111-
_scriptIdToLibraryId.clear();
112-
_libraryIdToScriptRefs.clear();
113114

114-
_libraryHelper = LibraryHelper(this);
115+
// TODO(srujzs): We can invalidate these in a smarter way instead of
116+
// reinitializing when doing a hot reload, but these helpers recompute info
117+
// on demand and therefore are not in the critical path.
115118
_classHelper = ClassHelper(this);
116119
_instanceHelper = InstanceHelper(this);
117120

121+
if (invalidatedModuleReport != null) {
122+
// Invalidate `_libraryHelper` as we use it populate any script caches.
123+
_libraryHelper.invalidate(invalidatedModuleReport);
124+
} else {
125+
_libraryHelper = LibraryHelper(this);
126+
_scriptRefsById.clear();
127+
_serverPathToScriptRef.clear();
128+
_scriptIdToLibraryId.clear();
129+
_libraryIdToScriptRefs.clear();
130+
}
131+
118132
final libraries = await _libraryHelper.libraryRefs;
119133
isolate.rootLib = await _libraryHelper.rootLib;
120134
isolate.libraries?.clear();
121135
isolate.libraries?.addAll(libraries);
122136

123-
final scripts = await scriptRefs;
137+
final scripts = await getScriptRefs(invalidatedModuleReport);
124138

125139
await DartUri.initialize();
126140
DartUri.recordAbsoluteUris(libraries.map((lib) => lib.uri).nonNulls);
@@ -583,7 +597,7 @@ class AppInspector implements AppInspectorInterface {
583597
/// All the scripts in the isolate.
584598
@override
585599
Future<ScriptList> getScripts() async {
586-
return ScriptList(scripts: await scriptRefs);
600+
return ScriptList(scripts: await getScriptRefs());
587601
}
588602

589603
/// Calls the Chrome Runtime.getProperties API for the object with [objectId].
@@ -714,19 +728,50 @@ class AppInspector implements AppInspectorInterface {
714728
///
715729
/// This will get repopulated on restarts and reloads.
716730
///
731+
/// If [invalidatedModuleReport] is provided, only invalidates and
732+
/// recalculates caches for the invalidated libraries.
733+
///
717734
/// Returns the list of scripts refs cached.
718-
Future<List<ScriptRef>> _populateScriptCaches() {
735+
Future<List<ScriptRef>> _populateScriptCaches([
736+
InvalidatedModuleReport? invalidatedModuleReport,
737+
]) {
719738
return _scriptCacheMemoizer.runOnce(() async {
720739
final scripts =
721740
await globalToolConfiguration.loadStrategy
722741
.metadataProviderFor(appConnection.request.entrypointPath)
723742
.scripts;
743+
final invalidatedLibraries = invalidatedModuleReport?.deletedLibraries
744+
.union(invalidatedModuleReport.reloadedLibraries);
745+
if (invalidatedLibraries != null) {
746+
for (final libraryUri in invalidatedLibraries) {
747+
final libraryRef = await _libraryHelper.libraryRefFor(libraryUri);
748+
final libraryId = libraryRef?.id;
749+
if (libraryId == null) continue;
750+
final scriptRefs = _libraryIdToScriptRefs.remove(libraryId);
751+
if (scriptRefs == null) continue;
752+
for (final scriptRef in scriptRefs) {
753+
final scriptId = scriptRef.id;
754+
final scriptUri = scriptRef.uri;
755+
if (scriptId != null && scriptUri != null) {
756+
_scriptRefsById.remove(scriptId);
757+
_scriptIdToLibraryId.remove(scriptId);
758+
_serverPathToScriptRef.remove(
759+
DartUri(scriptUri, _root).serverPath,
760+
);
761+
}
762+
}
763+
}
764+
}
724765
// For all the non-dart: libraries, find their parts and create scriptRefs
725766
// for them.
726767
final userLibraries = _userLibraryUris(
727768
isolate.libraries ?? <LibraryRef>[],
728769
);
729770
for (final uri in userLibraries) {
771+
if (invalidatedLibraries != null &&
772+
!invalidatedLibraries.contains(uri)) {
773+
continue;
774+
}
730775
final parts = scripts[uri];
731776
final scriptRefs = [
732777
ScriptRef(uri: uri, id: createId()),

‎dwds/lib/src/debugging/libraries.dart‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import 'package:collection/collection.dart';
66
import 'package:dwds/src/config/tool_configuration.dart';
77
import 'package:dwds/src/debugging/metadata/class.dart';
8+
import 'package:dwds/src/debugging/metadata/provider.dart';
89
import 'package:dwds/src/services/chrome_debug_exception.dart';
910
import 'package:dwds/src/utilities/domain.dart';
1011
import 'package:logging/logging.dart';
@@ -51,21 +52,35 @@ class LibraryHelper extends Domain {
5152
return _rootLib!;
5253
}
5354

55+
void invalidate(InvalidatedModuleReport invalidatedModuleReport) {
56+
final invalidatedLibraries = invalidatedModuleReport.deletedLibraries.union(
57+
invalidatedModuleReport.reloadedLibraries,
58+
);
59+
for (final library in invalidatedLibraries) {
60+
// These will later be initialized by `libraryFor` if needed.
61+
_librariesById.remove(library);
62+
_libraryRefsById.remove(library);
63+
}
64+
for (final library in invalidatedModuleReport.reloadedLibraries) {
65+
_libraryRefsById[library] = _createLibraryRef(library);
66+
}
67+
}
68+
69+
LibraryRef _createLibraryRef(String library) =>
70+
LibraryRef(id: library, name: library, uri: library);
71+
5472
/// Returns all libraryRefs in the app.
5573
///
56-
/// Note this can return a cached result.
74+
/// Note this can return a cached result that can be selectively reinitialized
75+
/// using [invalidate].
5776
Future<List<LibraryRef>> get libraryRefs async {
5877
if (_libraryRefsById.isNotEmpty) return _libraryRefsById.values.toList();
5978
final libraries =
6079
await globalToolConfiguration.loadStrategy
6180
.metadataProviderFor(inspector.appConnection.request.entrypointPath)
6281
.libraries;
6382
for (final library in libraries) {
64-
_libraryRefsById[library] = LibraryRef(
65-
id: library,
66-
name: library,
67-
uri: library,
68-
);
83+
_libraryRefsById[library] = _createLibraryRef(library);
6984
}
7085
return _libraryRefsById.values.toList();
7186
}

‎dwds/lib/src/debugging/modules.dart‎

Lines changed: 58 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,25 @@ class Modules {
3030

3131
Modules(this._root);
3232

33-
/// Initializes the mapping from source to module.
33+
/// Initializes mappings after invalidating modified libraries/modules.
3434
///
3535
/// Intended to be called multiple times throughout the development workflow,
3636
/// e.g. after a hot-reload.
37-
void initialize(String entrypoint) {
38-
// TODO(srujzs): Can we do better and only invalidate the sources/modules
39-
// that were deleted/reloaded? This would require removing the
40-
// deleted/reloaded libraries/sources/modules from the following maps and
41-
// then only processing that set in `_initializeMapping`. It's doable, but
42-
// these calculations are also not that expensive.
43-
_sourceToModule.clear();
44-
_moduleToSources.clear();
45-
_sourceToLibrary.clear();
46-
_libraryToModule.clear();
47-
_moduleMemoizer = AsyncMemoizer();
48-
_entrypoint = entrypoint;
37+
Future<void> initialize(
38+
String entrypoint, [
39+
InvalidatedModuleReport? invalidatedModuleReport,
40+
]) async {
41+
if (invalidatedModuleReport != null) {
42+
assert(_entrypoint == entrypoint);
43+
await _initializeMapping(invalidatedModuleReport);
44+
} else {
45+
_sourceToModule.clear();
46+
_moduleToSources.clear();
47+
_sourceToLibrary.clear();
48+
_libraryToModule.clear();
49+
_moduleMemoizer = AsyncMemoizer();
50+
_entrypoint = entrypoint;
51+
}
4952
}
5053

5154
/// Returns the containing module for the provided Dart server path.
@@ -88,7 +91,37 @@ class Modules {
8891
return chromePathToRuntimeScriptId[serverPath];
8992
}
9093

94+
String _getLibraryServerPath(String library) =>
95+
library.startsWith('dart:')
96+
? library
97+
: DartUri(library, _root).serverPath;
98+
99+
Set<String> _invalidateLibraries(
100+
InvalidatedModuleReport invalidatedModuleReport,
101+
) {
102+
Set<String> invalidatedLibraries;
103+
invalidatedLibraries = invalidatedModuleReport.deletedLibraries.union(
104+
invalidatedModuleReport.reloadedLibraries,
105+
);
106+
final invalidatedModules = invalidatedModuleReport.deletedModules.union(
107+
invalidatedModuleReport.reloadedModules,
108+
);
109+
for (final library in invalidatedLibraries) {
110+
final libraryServerPath = _getLibraryServerPath(library);
111+
_sourceToLibrary.remove(libraryServerPath);
112+
_sourceToModule.remove(libraryServerPath);
113+
_libraryToModule.remove(library);
114+
}
115+
for (final module in invalidatedModules) {
116+
_moduleToSources.remove(module);
117+
}
118+
return invalidatedLibraries;
119+
}
120+
91121
/// Initializes [_sourceToModule], [_moduleToSources], and [_sourceToLibrary].
122+
///
123+
/// If [invalidatedModuleReport] is not null, only updates the maps for the
124+
/// invalidated libraries in the report.
92125
Future<void> _initializeMapping([
93126
InvalidatedModuleReport? invalidatedModuleReport,
94127
]) async {
@@ -99,12 +132,19 @@ class Modules {
99132
final libraryToScripts = await provider.scripts;
100133
final scriptToModule = await provider.scriptToModule;
101134

135+
final invalidatedLibraries =
136+
invalidatedModuleReport != null
137+
? _invalidateLibraries(invalidatedModuleReport)
138+
: null;
139+
102140
for (final library in libraryToScripts.keys) {
141+
if (invalidatedLibraries != null) {
142+
// Note that every module will have at least one library associated with
143+
// it, so it's okay to only process the invalidated libraries.
144+
if (!invalidatedLibraries.contains(library)) continue;
145+
}
103146
final scripts = libraryToScripts[library]!;
104-
final libraryServerPath =
105-
library.startsWith('dart:')
106-
? library
107-
: DartUri(library, _root).serverPath;
147+
final libraryServerPath = _getLibraryServerPath(library);
108148

109149
if (scriptToModule.containsKey(library)) {
110150
final module = scriptToModule[library]!;
@@ -115,10 +155,7 @@ class Modules {
115155
_libraryToModule[library] = module;
116156

117157
for (final script in scripts) {
118-
final scriptServerPath =
119-
script.startsWith('dart:')
120-
? script
121-
: DartUri(script, _root).serverPath;
158+
final scriptServerPath = _getLibraryServerPath(script);
122159

123160
_sourceToModule[scriptServerPath] = module;
124161
_moduleToSources[module]!.add(scriptServerPath);

‎dwds/lib/src/services/chrome_proxy_service.dart‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,15 +249,18 @@ class ChromeProxyService implements VmServiceInterface {
249249
final invalidatedModuleReport = await globalToolConfiguration.loadStrategy
250250
.reinitializeEntrypointAfterReload(entrypoint, reloadedModules);
251251
await _initializeEntrypoint(entrypoint, invalidatedModuleReport);
252-
await inspector.initialize();
252+
await inspector.initialize(invalidatedModuleReport);
253253
}
254254

255255
/// Initializes metadata in [Locations], [Modules], and [ExpressionCompiler].
256+
///
257+
/// If [invalidatedModuleReport] is not null, only removes and reinitializes
258+
/// invalidated metadata.
256259
Future<void> _initializeEntrypoint(
257260
String entrypoint, [
258261
InvalidatedModuleReport? invalidatedModuleReport,
259262
]) async {
260-
_modules.initialize(entrypoint);
263+
await _modules.initialize(entrypoint, invalidatedModuleReport);
261264
await _locations.initialize(entrypoint, invalidatedModuleReport);
262265
await _skipLists.initialize(entrypoint, invalidatedModuleReport);
263266
// We do not need to wait for compiler dependencies to be updated as the
@@ -353,7 +356,7 @@ class ChromeProxyService implements VmServiceInterface {
353356
if (!newConnection) {
354357
await globalToolConfiguration.loadStrategy.trackEntrypoint(entrypoint);
355358
}
356-
_initializeEntrypoint(entrypoint);
359+
await _initializeEntrypoint(entrypoint);
357360

358361
debugger.notifyPausedAtStart();
359362
_inspector = await AppInspector.create(

‎dwds/test/fixtures/fakes.dart‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ class FakeInspector implements AppInspector {
6868
}
6969

7070
@override
71-
Future<void> initialize() async => {};
71+
Future<void> initialize([
72+
InvalidatedModuleReport? invalidatedModuleReport,
73+
]) async => {};
7274

7375
@override
7476
Future<InstanceRef?> instanceRefFor(Object value) async =>
@@ -157,10 +159,10 @@ class FakeModules implements Modules {
157159
_path = path;
158160

159161
@override
160-
void initialize(
162+
Future<void> initialize(
161163
String entrypoint, [
162164
InvalidatedModuleReport? invalidatedModuleReport,
163-
]) {}
165+
]) async {}
164166

165167
@override
166168
Future<Uri> libraryForSource(String serverPath) async => Uri(path: _library);

0 commit comments

Comments
 (0)