Skip to content

Commit 1f59259

Browse files
scheglovCommit Queue
authored andcommitted
Fine. Deduplicate prefix-scope requirement recording
Record import-prefix lookups only once per identifier during a single requirements run. This avoids repeatedly writing the same `exportedTopLevels` entries for every imported library when the same name is looked up multiple times. Key changes: - Introduce `PrefixScopeRequirementState` and pass it to `RequirementsManifest.record_importPrefixScope_lookup`. The method now records requirements for a given `id` only on the first lookup seen by that state. - Add `InstanceElementRequirementState` and `LibraryElementRequirementState` to cache per-owner lookups inside `_getInstanceItem`, `_getInterfaceItem`, and `_getLibraryRequirements`, reducing map churn. - Add `RequirementsManifest.stopRecording()` and call it after analysis/linking to dispose per-run state and reclaim memory. - Update call sites: - `PrefixScope.lookup()` now supplies a persistent state. - `record_library_exportScope_get()` uses a one-shot state for direct library export-scope queries. Impact: - Lowers the cost of requirement recording from ~6% to ~1.74% of total analysis time in observed profiles. - Preserves correctness: all needed exported IDs are still recorded; we simply suppress duplicates. - Trade-off: a small, transient per-run memory footprint for the dedup sets and caches, cleared via `stopRecording()`. Rationale: Repeated name lookups were causing redundant `Map` writes (often writing the same `null`/ID values many times). By binding prefix-scope recording to a per-run state and caching element/library requirement slots, we remove hot-path duplication without sacrificing precision. Change-Id: Ibe07162aa0503348956b9c41beea493d469c0193 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/450366 Commit-Queue: Konstantin Shcheglov <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent 3286cb4 commit 1f59259

File tree

5 files changed

+126
-4
lines changed

5 files changed

+126
-4
lines changed

pkg/analyzer/lib/src/dart/analysis/driver.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,6 +1395,7 @@ class AnalysisDriver {
13951395

13961396
if (withFineDependencies) {
13971397
assert(requirements!.assertSerialization());
1398+
globalResultRequirements?.stopRecording();
13981399
globalResultRequirements = null;
13991400
}
14001401

pkg/analyzer/lib/src/dart/analysis/library_context.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ class LibraryContext {
332332
elementFactory: elementFactory,
333333
libraryUriSet: cycle.libraryUris,
334334
);
335+
globalResultRequirements?.stopRecording();
335336
globalResultRequirements = null;
336337
requirements.removeReqForLibs(cycle.libraryUris);
337338
assert(requirements.assertSerialization());

pkg/analyzer/lib/src/dart/element/element.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4221,6 +4221,10 @@ abstract class InstanceElementImpl extends ElementImpl
42214221
List<SetterElementImpl> _setters = [];
42224222
List<MethodElementImpl> _methods = [];
42234223

4224+
@trackedInternal
4225+
InstanceElementRequirementState requirementState =
4226+
InstanceElementRequirementState();
4227+
42244228
@override
42254229
@trackedIncludedInId
42264230
InstanceElement get baseElement => this;
@@ -5999,6 +6003,10 @@ class LibraryElementImpl extends ElementImpl
59996003
@trackedInternal
60006004
LibraryManifest? manifest;
60016005

6006+
@trackedInternal
6007+
LibraryElementRequirementState requirementState =
6008+
LibraryElementRequirementState();
6009+
60026010
@trackedInternal
60036011
late final LibraryElementImplInternal internal = LibraryElementImplInternal(
60046012
this,

pkg/analyzer/lib/src/dart/element/scope.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,9 @@ class PrefixScope implements Scope {
518518

519519
ImportsTrackingOfPrefix? _importsTracking;
520520

521+
final PrefixScopeRequirementState _requirementState =
522+
PrefixScopeRequirementState();
523+
521524
PrefixScope({
522525
required this.libraryFragment,
523526
required this.parent,
@@ -579,6 +582,7 @@ class PrefixScope implements Scope {
579582

580583
if (globalResultRequirements case var resultRequirements?) {
581584
resultRequirements.record_importPrefixScope_lookup(
585+
state: _requirementState,
582586
importedLibraries: _importedLibraries,
583587
id: id,
584588
);

pkg/analyzer/lib/src/fine/requirements.dart

Lines changed: 112 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'dart:collection';
56
import 'dart:typed_data';
67

78
import 'package:analyzer/dart/element/element.dart';
@@ -191,6 +192,27 @@ final class ExportRequirementShowCombinator
191192
}
192193
}
193194

195+
class InstanceElementRequirementState {
196+
RequirementsManifest? _owner;
197+
_InstanceItemWithRequirements? _instanceResult;
198+
_InterfaceItemWithRequirements? _interfaceResult;
199+
200+
void _dispose() {
201+
_owner = null;
202+
_instanceResult = null;
203+
_interfaceResult = null;
204+
}
205+
206+
void _ensureFor(RequirementsManifest owner) {
207+
if (!identical(_owner, owner)) {
208+
owner._instanceElementStates.add(this);
209+
_owner = owner;
210+
_instanceResult = null;
211+
_interfaceResult = null;
212+
}
213+
}
214+
}
215+
194216
/// Requirements for [InstanceElementImpl].
195217
///
196218
/// If [InterfaceElementImpl], there are additional requirements in form
@@ -330,6 +352,24 @@ class InterfaceItemRequirements {
330352
}
331353
}
332354

355+
class LibraryElementRequirementState {
356+
RequirementsManifest? _owner;
357+
LibraryRequirements? _result;
358+
359+
void _dispose() {
360+
_owner = null;
361+
_result = null;
362+
}
363+
364+
void _ensureFor(RequirementsManifest owner) {
365+
if (!identical(_owner, owner)) {
366+
owner._libraryElementStates.add(this);
367+
_owner = owner;
368+
_result = null;
369+
}
370+
}
371+
}
372+
333373
/// Requirements for all `export`s of a library.
334374
@visibleForTesting
335375
class LibraryExportRequirements {
@@ -701,6 +741,29 @@ class OpaqueApiUse {
701741
}
702742
}
703743

744+
/// Mutable state [RequirementsManifest] uses whenever a prefix-scope lookup
745+
/// happens. It binds to the current [RequirementsManifest] and suppresses
746+
/// duplicate recordings for the same identifier during that run.
747+
class PrefixScopeRequirementState {
748+
RequirementsManifest? _owner;
749+
750+
/// The set of names for which we already recorded requirements.
751+
HashSet<String> _idSet = HashSet();
752+
753+
void _dispose() {
754+
_owner = null;
755+
_idSet = HashSet();
756+
}
757+
758+
void _ensureFor(RequirementsManifest owner) {
759+
if (!identical(_owner, owner)) {
760+
owner._prefixScopeStates.add(this);
761+
_owner = owner;
762+
_idSet = HashSet();
763+
}
764+
}
765+
}
766+
704767
class RequirementsManifest {
705768
/// LibraryUri => LibraryRequirements
706769
final Map<Uri, LibraryRequirements> libraries = {};
@@ -714,6 +777,10 @@ class RequirementsManifest {
714777

715778
int _recordingLockLevel = 0;
716779

780+
final List<InstanceElementRequirementState> _instanceElementStates = [];
781+
final List<LibraryElementRequirementState> _libraryElementStates = [];
782+
final List<PrefixScopeRequirementState> _prefixScopeStates = [];
783+
717784
RequirementsManifest();
718785

719786
factory RequirementsManifest.read(SummaryDataReader reader) {
@@ -1486,6 +1553,7 @@ class RequirementsManifest {
14861553
/// Record that [id] was looked up in the import prefix scope that
14871554
/// imports [importedLibraries].
14881555
void record_importPrefixScope_lookup({
1556+
required PrefixScopeRequirementState state,
14891557
required List<LibraryElementImpl> importedLibraries,
14901558
required String id,
14911559
}) {
@@ -1495,6 +1563,11 @@ class RequirementsManifest {
14951563
return;
14961564
}
14971565

1566+
state._ensureFor(this);
1567+
if (!state._idSet.add(id)) {
1568+
return;
1569+
}
1570+
14981571
var getterLookupName = id.asLookupName;
14991572
var setterLookupName = '$id='.asLookupName;
15001573

@@ -1990,6 +2063,7 @@ class RequirementsManifest {
19902063
required String name,
19912064
}) {
19922065
record_importPrefixScope_lookup(
2066+
state: PrefixScopeRequirementState(),
19932067
importedLibraries: [element],
19942068
id: name.removeSuffix('=') ?? name,
19952069
);
@@ -2305,6 +2379,23 @@ class RequirementsManifest {
23052379
}
23062380
}
23072381

2382+
void stopRecording() {
2383+
for (var state in _instanceElementStates) {
2384+
state._dispose();
2385+
}
2386+
_instanceElementStates.clear();
2387+
2388+
for (var state in _libraryElementStates) {
2389+
state._dispose();
2390+
}
2391+
_libraryElementStates.clear();
2392+
2393+
for (var state in _prefixScopeStates) {
2394+
state._dispose();
2395+
}
2396+
_prefixScopeStates.clear();
2397+
}
2398+
23082399
void write(BufferedSink sink) {
23092400
sink.writeMap(
23102401
libraries,
@@ -2321,6 +2412,11 @@ class RequirementsManifest {
23212412
}
23222413

23232414
_InstanceItemWithRequirements? _getInstanceItem(InstanceElementImpl element) {
2415+
var state = element.requirementState.._ensureFor(this);
2416+
if (state._instanceResult case var result?) {
2417+
return result;
2418+
}
2419+
23242420
var libraryElement = element.library;
23252421
var manifest = libraryElement.manifest;
23262422

@@ -2349,16 +2445,21 @@ class RequirementsManifest {
23492445

23502446
var requirements = instancesMap[instanceName] ??=
23512447
InstanceItemRequirements.empty();
2352-
2353-
return _InstanceItemWithRequirements(
2448+
var result = _InstanceItemWithRequirements(
23542449
item: instanceItem,
23552450
requirements: requirements,
23562451
);
2452+
return state._instanceResult = result;
23572453
}
23582454

23592455
_InterfaceItemWithRequirements? _getInterfaceItem(
23602456
InterfaceElementImpl element,
23612457
) {
2458+
var state = element.requirementState.._ensureFor(this);
2459+
if (state._interfaceResult case var result?) {
2460+
return result;
2461+
}
2462+
23622463
var libraryElement = element.library;
23632464
var manifest = libraryElement.manifest;
23642465

@@ -2386,14 +2487,21 @@ class RequirementsManifest {
23862487

23872488
var requirements = interfacesMap[interfaceName] ??=
23882489
InterfaceItemRequirements.empty();
2389-
return _InterfaceItemWithRequirements(
2490+
var result = _InterfaceItemWithRequirements(
23902491
item: interfaceItem,
23912492
requirements: requirements,
23922493
);
2494+
return state._interfaceResult = result;
23932495
}
23942496

23952497
LibraryRequirements _getLibraryRequirements(LibraryElementImpl element) {
2396-
return libraries[element.uri] ??= LibraryRequirements.empty();
2498+
var state = element.requirementState.._ensureFor(this);
2499+
if (state._result case var result?) {
2500+
return result;
2501+
}
2502+
2503+
var result = libraries[element.uri] ??= LibraryRequirements.empty();
2504+
return state._result = result;
23972505
}
23982506

23992507
String _qualifiedMethodName(

0 commit comments

Comments
 (0)