Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 271f724

Browse files
natebiggsCommit Queue
natebiggs
authored and
Commit Queue
committed
[dart2js] Add live abstract members into closed world.
Effects of this change are minimal on a large application: - ~9100 new members being tracked. - +500KB closed world serialized output (.3% increase) - No discernible difference in closed world runtime - Heap space in global inference after closed world deserialization increase is <1MB. - No change in JS output These abstract members will be used by the new hierarchy-based dynamic call type graph linearization. Change-Id: I4ce56d12e039effe0a855fd23f2a5e69d0847025 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/264900 Reviewed-by: Mayank Patke <fishythefish@google.com> Commit-Queue: Nate Biggs <natebiggs@google.com>
1 parent 99b3dff commit 271f724

15 files changed

+168
-21
lines changed

‎pkg/compiler/lib/src/js_model/element_map_impl.dart

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
128128
this._environment,
129129
KernelToElementMap _elementMap,
130130
Map<MemberEntity, MemberUsage> liveMemberUsage,
131+
Set<MemberEntity> liveAbstractMembers,
131132
AnnotationsData annotations)
132133
: this.options = _elementMap.options {
133134
_elementEnvironment = JsElementEnvironment(this);
@@ -165,7 +166,7 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
165166
IndexedClass newClass = JClass(newLibrary as JLibrary, oldClass.name,
166167
isAbstract: oldClass.isAbstract);
167168
JClassEnv newEnv = env.convert(_elementMap, liveMemberUsage,
168-
(ir.Library library) => libraryMap[library]!);
169+
liveAbstractMembers, (ir.Library library) => libraryMap[library]!);
169170
classMap[env.cls] = classes.register(newClass, data.convert(), newEnv);
170171
assert(newClass.classIndex == oldClass.classIndex);
171172
libraries.getEnv(newLibrary).registerClass(newClass.name, newEnv);
@@ -176,7 +177,7 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
176177
memberIndex++) {
177178
IndexedMember oldMember = _elementMap.members.getEntity(memberIndex)!;
178179
MemberUsage? memberUsage = liveMemberUsage[oldMember];
179-
if (memberUsage == null) {
180+
if (memberUsage == null && !liveAbstractMembers.contains(oldMember)) {
180181
members.skipIndex(oldMember.memberIndex);
181182
continue;
182183
}
@@ -199,7 +200,7 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
199200
} else if (oldMember is ConstructorEntity) {
200201
final constructor = oldMember as IndexedConstructor;
201202
ParameterStructure parameterStructure =
202-
annotations.hasNoElision(constructor)
203+
annotations.hasNoElision(constructor) || memberUsage == null
203204
? constructor.parameterStructure
204205
: memberUsage.invokedParameters!;
205206
if (constructor.isFactoryConstructor) {
@@ -231,7 +232,9 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
231232
} else {
232233
final function = oldMember as IndexedFunction;
233234
ParameterStructure parameterStructure =
234-
annotations.hasNoElision(function)
235+
annotations.hasNoElision(function) ||
236+
memberUsage == null ||
237+
function.isAbstract
235238
? function.parameterStructure
236239
: memberUsage.invokedParameters!;
237240
newMember = JMethod(newLibrary, newClass, memberName,

‎pkg/compiler/lib/src/js_model/js_strategy.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ class JsBackendStrategy implements interfaces.JsBackendStrategy {
184184
_compiler.environment,
185185
strategy.elementMap,
186186
closedWorld.liveMemberUsage,
187+
closedWorld.liveAbstractInstanceMembers,
187188
closedWorld.annotationsData);
188189
ClosureDataBuilder closureDataBuilder = ClosureDataBuilder(
189190
_compiler.reporter, _elementMap, closedWorld.annotationsData);

‎pkg/compiler/lib/src/js_model/js_world.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ class JClosedWorld implements World {
8888

8989
final Set<MemberEntity> liveInstanceMembers;
9090

91+
final Set<MemberEntity> liveAbstractInstanceMembers;
92+
9193
/// Members that are written either directly or through a setter selector.
9294
final Set<MemberEntity> assignedInstanceMembers;
9395

@@ -125,6 +127,7 @@ class JClosedWorld implements World {
125127
this.implementedClasses,
126128
this.liveNativeClasses,
127129
this.liveInstanceMembers,
130+
this.liveAbstractInstanceMembers,
128131
this.assignedInstanceMembers,
129132
this.processedMembers,
130133
this.extractTypeArgumentsInterfacesNewRti,
@@ -171,6 +174,8 @@ class JClosedWorld implements World {
171174
Set<ClassEntity> extractTypeArgumentsInterfacesNewRti =
172175
source.readClasses().toSet();
173176
Set<MemberEntity> liveInstanceMembers = source.readMembers().toSet();
177+
Set<MemberEntity> liveAbstractInstanceMembers =
178+
source.readMembers().toSet();
174179
Set<MemberEntity> assignedInstanceMembers = source.readMembers().toSet();
175180
Set<MemberEntity> processedMembers = source.readMembers().toSet();
176181
Map<ClassEntity, Set<ClassEntity>> mixinUses =
@@ -204,6 +209,7 @@ class JClosedWorld implements World {
204209
implementedClasses,
205210
liveNativeClasses,
206211
liveInstanceMembers,
212+
liveAbstractInstanceMembers,
207213
assignedInstanceMembers,
208214
processedMembers,
209215
extractTypeArgumentsInterfacesNewRti,
@@ -232,6 +238,7 @@ class JClosedWorld implements World {
232238
sink.writeClasses(liveNativeClasses);
233239
sink.writeClasses(extractTypeArgumentsInterfacesNewRti);
234240
sink.writeMembers(liveInstanceMembers);
241+
sink.writeMembers(liveAbstractInstanceMembers);
235242
sink.writeMembers(assignedInstanceMembers);
236243
sink.writeMembers(processedMembers);
237244
sink.writeClassMap(

‎pkg/compiler/lib/src/js_model/js_world_builder.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ class JClosedWorldBuilder {
111111

112112
Set<MemberEntity> liveInstanceMembers =
113113
map.toBackendMemberSet(closedWorld.liveInstanceMembers);
114+
Set<MemberEntity> liveAbstractInstanceMembers =
115+
map.toBackendMemberSet(closedWorld.liveAbstractInstanceMembers);
114116

115117
Map<ClassEntity, Set<ClassEntity>> mixinUses =
116118
map.toBackendClassMap(closedWorld.mixinUses, map.toBackendClassSet);
@@ -226,6 +228,7 @@ class JClosedWorldBuilder {
226228
// represent the synthesized call methods for static and instance method
227229
// closurizations.
228230
liveInstanceMembers /*..addAll(callMethods)*/,
231+
liveAbstractInstanceMembers,
229232
assignedInstanceMembers,
230233
processedMembers,
231234
extractTypeArgumentsInterfacesNewRti,

‎pkg/compiler/lib/src/kernel/env.dart

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ abstract class KClassEnv {
246246
JClassEnv convert(
247247
IrToElementMap kElementMap,
248248
Map<MemberEntity, MemberUsage> liveMemberUsage,
249+
Set<MemberEntity> liveAbstractMembers,
249250
LibraryEntity Function(ir.Library library) getJLibrary);
250251

251252
/// Returns `true` if [node] is a known member of this class.
@@ -435,6 +436,7 @@ class KClassEnvImpl implements KClassEnv {
435436
JClassEnv convert(
436437
IrToElementMap kElementMap,
437438
Map<MemberEntity, MemberUsage> liveMemberUsage,
439+
Set<MemberEntity> liveAbstractMembers,
438440
LibraryEntity Function(ir.Library library) getJLibrary) {
439441
Map<String, ir.Member> constructorMap;
440442
Map<Name, ir.Member> memberMap;
@@ -445,7 +447,8 @@ class KClassEnvImpl implements KClassEnv {
445447
constructorMap = <String, ir.Member>{};
446448
_constructorMap!.forEach((String name, ir.Member node) {
447449
MemberEntity member = kElementMap.getMember(node);
448-
if (liveMemberUsage.containsKey(member)) {
450+
if (liveMemberUsage.containsKey(member) ||
451+
liveAbstractMembers.contains(member)) {
449452
constructorMap[name] = node;
450453
}
451454
});
@@ -456,7 +459,8 @@ class KClassEnvImpl implements KClassEnv {
456459
memberMap = <Name, ir.Member>{};
457460
_memberMap!.forEach((Name name, ir.Member node) {
458461
MemberEntity member = kElementMap.getMember(node);
459-
if (liveMemberUsage.containsKey(member)) {
462+
if (liveMemberUsage.containsKey(member) ||
463+
liveAbstractMembers.contains(member)) {
460464
memberMap[name] = node;
461465
}
462466
});
@@ -467,7 +471,8 @@ class KClassEnvImpl implements KClassEnv {
467471
members = <ir.Member>[];
468472
_members!.forEach((ir.Member node) {
469473
MemberEntity member = kElementMap.getMember(node);
470-
if (liveMemberUsage.containsKey(member)) {
474+
if (liveMemberUsage.containsKey(member) ||
475+
liveAbstractMembers.contains(member)) {
471476
members.add(node);
472477
}
473478
});
@@ -682,7 +687,12 @@ class KFunctionDataImpl extends KMemberDataImpl
682687
@override
683688
FunctionData convert() {
684689
return FunctionDataImpl(
685-
node, functionNode, RegularMemberDefinition(node), staticTypes!);
690+
node,
691+
functionNode,
692+
RegularMemberDefinition(node),
693+
// Abstract members without bodies will not have expressions so we use
694+
// an empty cache.
695+
staticTypes ?? const StaticTypeCache());
686696
}
687697

688698
@override
@@ -757,7 +767,12 @@ class KFieldDataImpl extends KMemberDataImpl implements KFieldData {
757767

758768
@override
759769
JFieldData convert() {
760-
return JFieldDataImpl(node, RegularMemberDefinition(node), staticTypes!);
770+
return JFieldDataImpl(
771+
node,
772+
RegularMemberDefinition(node),
773+
// Late fields in abstract classes won't have initializers so we use an
774+
// empty cache.
775+
staticTypes ?? const StaticTypeCache());
761776
}
762777
}
763778

‎pkg/compiler/lib/src/kernel/kernel_world.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class KClosedWorld implements BuiltWorld {
4040
// TODO(johnniwinther): Can this be derived from [ClassSet]s?
4141
final Set<ClassEntity> _implementedClasses;
4242
final Iterable<MemberEntity> liveInstanceMembers;
43+
final Iterable<MemberEntity> liveAbstractInstanceMembers;
4344

4445
/// Members that are written either directly or through a setter selector.
4546
final Iterable<MemberEntity> assignedInstanceMembers;
@@ -102,6 +103,7 @@ class KClosedWorld implements BuiltWorld {
102103
required Set<ClassEntity> implementedClasses,
103104
required this.liveNativeClasses,
104105
required this.liveInstanceMembers,
106+
required this.liveAbstractInstanceMembers,
105107
required this.assignedInstanceMembers,
106108
required this.liveMemberUsage,
107109
required this.mixinUses,

‎pkg/compiler/lib/src/resolution/enqueuer.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,9 @@ class ResolutionEnqueuer extends Enqueuer {
122122
}
123123
if (useSet.contains(ClassUse.IMPLEMENTED)) {
124124
applyImpact(listener.registerImplementedClass(cls));
125+
if (cls.isAbstract) {
126+
worldBuilder.processAbstractClassMembers(cls, _applyMemberUse);
127+
}
125128
}
126129
}
127130

‎pkg/compiler/lib/src/universe/resolution_world_builder.dart

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ class ResolutionWorldBuilder extends WorldBuilder
268268
bool _closed = false;
269269
KClosedWorld? _closedWorldCache;
270270
final Set<MemberEntity> _liveInstanceMembers = {};
271+
final Set<MemberEntity> _liveAbstractInstanceMembers = {};
271272

272273
final Set<ConstantValue> _constantValues = {};
273274

@@ -451,9 +452,10 @@ class ResolutionWorldBuilder extends WorldBuilder
451452
EnumSet<MemberUse> action(MemberUsage usage),
452453
bool shouldBeRemoved(MemberUsage usage)) {
453454
_processSet(memberMap, methodName, (MemberUsage usage) {
454-
if (selector.appliesUnnamed(usage.entity) &&
455-
_selectorConstraintsStrategy.appliedUnnamed(
456-
dynamicUse, usage.entity, this)) {
455+
if (usage.entity.isAbstract ||
456+
selector.appliesUnnamed(usage.entity) &&
457+
(_selectorConstraintsStrategy.appliedUnnamed(
458+
dynamicUse, usage.entity, this))) {
457459
memberUsed(usage.entity, action(usage));
458460
return shouldBeRemoved(usage);
459461
}
@@ -680,11 +682,23 @@ class ResolutionWorldBuilder extends WorldBuilder
680682
{bool checkEnqueuerConsistency = false}) {
681683
_elementEnvironment.forEachClassMember(cls,
682684
(ClassEntity cls, MemberEntity member) {
683-
_processInstantiatedClassMember(cls, member, memberUsed,
685+
_processMemberInUsedClass(cls, member, memberUsed,
684686
checkEnqueuerConsistency: checkEnqueuerConsistency);
685687
});
686688
}
687689

690+
@override
691+
void processAbstractClassMembers(
692+
ClassEntity cls, MemberUsedCallback memberUsed) {
693+
_elementEnvironment.forEachLocalClassMember(cls, (MemberEntity member) {
694+
if (member.isAbstract) {
695+
// Check for potential usages of abstract members (i.e. of their
696+
// overrides) and save them if they are used.
697+
_processMemberInUsedClass(cls, member, memberUsed);
698+
}
699+
});
700+
}
701+
688702
/// Call [updateUsage] on all [MemberUsage]s in the set in [map] for
689703
/// [memberName]. If [updateUsage] returns `true` the usage is removed from
690704
/// the set.
@@ -785,7 +799,13 @@ class ResolutionWorldBuilder extends WorldBuilder
785799
return usage;
786800
}
787801

788-
void _processInstantiatedClassMember(
802+
/// Determines whether [member] is potentially used and calls the [memberUsed]
803+
/// callback on it if it is.
804+
///
805+
/// [member] can be concrete or abstract and in either case potential usage
806+
/// is determined by comparing the signature of [member] to the selector
807+
/// structure (including arguments) at call sites.
808+
void _processMemberInUsedClass(
789809
ClassEntity cls, MemberEntity member, MemberUsedCallback memberUsed,
790810
{bool checkEnqueuerConsistency = false}) {
791811
if (!member.isInstanceMember) return;
@@ -851,8 +871,12 @@ class ResolutionWorldBuilder extends WorldBuilder
851871

852872
@override
853873
void registerUsedElement(MemberEntity element) {
854-
if (element.isInstanceMember && !element.isAbstract) {
855-
_liveInstanceMembers.add(element);
874+
if (element.isInstanceMember) {
875+
if (element.isAbstract) {
876+
_liveAbstractInstanceMembers.add(element);
877+
} else {
878+
_liveInstanceMembers.add(element);
879+
}
856880
}
857881
}
858882

@@ -997,6 +1021,7 @@ class ResolutionWorldBuilder extends WorldBuilder
9971021
implementedClasses: _implementedClasses,
9981022
liveNativeClasses: _nativeResolutionEnqueuer.liveNativeClasses,
9991023
liveInstanceMembers: _liveInstanceMembers,
1024+
liveAbstractInstanceMembers: _liveAbstractInstanceMembers,
10001025
assignedInstanceMembers: computeAssignedInstanceMembers(),
10011026
liveMemberUsage: liveMemberUsage,
10021027
mixinUses: _classHierarchyBuilder.mixinUses,

‎pkg/compiler/lib/src/universe/resolution_world_builder_interfaces.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ abstract class ResolutionWorldBuilder implements World {
2121
void processClassMembers(ClassEntity cls, MemberUsedCallback memberUsed,
2222
{bool checkEnqueuerConsistency = false});
2323

24+
void processAbstractClassMembers(
25+
ClassEntity cls, MemberUsedCallback memberUsed);
26+
2427
bool isInheritedIn(
2528
MemberEntity member, ClassEntity type, ClassRelation relation);
2629

0 commit comments

Comments
 (0)