Specialize closures for 0 and 2 arguments
Change-Id: Ia21249cb9671960eb3e0d96afef5e5efb8ba7ee2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105200
Commit-Queue: Stephen Adams <sra@google.com>
Reviewed-by: Joshua Litt <joshualitt@google.com>
diff --git a/pkg/compiler/lib/src/common_elements.dart b/pkg/compiler/lib/src/common_elements.dart
index a57232b..91d8067 100644
--- a/pkg/compiler/lib/src/common_elements.dart
+++ b/pkg/compiler/lib/src/common_elements.dart
@@ -335,6 +335,8 @@
FunctionEntity findHelperFunction(String name);
ClassEntity get closureClass;
+ ClassEntity get closureClass0Args;
+ ClassEntity get closureClass2Args;
ClassEntity get boundClosureClass;
@@ -1536,6 +1538,16 @@
@override
ClassEntity get closureClass => _closureClass ??= _findHelperClass('Closure');
+ ClassEntity _closureClass0Args;
+ @override
+ ClassEntity get closureClass0Args =>
+ _closureClass0Args ??= _findHelperClass('Closure0Args');
+
+ ClassEntity _closureClass2Args;
+ @override
+ ClassEntity get closureClass2Args =>
+ _closureClass2Args ??= _findHelperClass('Closure2Args');
+
ClassEntity _boundClosureClass;
@override
ClassEntity get boundClosureClass =>
diff --git a/pkg/compiler/lib/src/elements/entities.dart b/pkg/compiler/lib/src/elements/entities.dart
index f136327..b0f12cf 100644
--- a/pkg/compiler/lib/src/elements/entities.dart
+++ b/pkg/compiler/lib/src/elements/entities.dart
@@ -291,6 +291,12 @@
static const ParameterStructure zeroArguments =
ParameterStructure._(0, 0, [], {}, 0);
+ static const ParameterStructure oneArgument =
+ ParameterStructure._(1, 1, [], {}, 0);
+
+ static const ParameterStructure twoArguments =
+ ParameterStructure._(2, 2, [], {}, 0);
+
static const List<ParameterStructure> _simple = [
ParameterStructure._(0, 0, [], {}, 0),
ParameterStructure._(1, 1, [], {}, 0),
diff --git a/pkg/compiler/lib/src/js_backend/backend_impact.dart b/pkg/compiler/lib/src/js_backend/backend_impact.dart
index cb72411..e1de963 100644
--- a/pkg/compiler/lib/src/js_backend/backend_impact.dart
+++ b/pkg/compiler/lib/src/js_backend/backend_impact.dart
@@ -574,8 +574,11 @@
BackendImpact _functionClass;
BackendImpact get functionClass {
- return _functionClass ??=
- new BackendImpact(globalClasses: [_commonElements.closureClass]);
+ return _functionClass ??= new BackendImpact(globalClasses: [
+ _commonElements.closureClass,
+ _commonElements.closureClass0Args,
+ _commonElements.closureClass2Args,
+ ]);
}
BackendImpact _mapClass;
diff --git a/pkg/compiler/lib/src/js_emitter/model.dart b/pkg/compiler/lib/src/js_emitter/model.dart
index fd4296e..156ccb1 100644
--- a/pkg/compiler/lib/src/js_emitter/model.dart
+++ b/pkg/compiler/lib/src/js_emitter/model.dart
@@ -241,7 +241,14 @@
final bool onlyForConstructor;
final bool isDirectlyInstantiated;
final bool isNative;
- final bool isClosureBaseClass; // Common base class for closures.
+
+ /// `true` if this is the one class that is the root of all 'Closure' classes.
+ final bool isClosureBaseClass;
+
+ /// If non-null, this class is used as a base class for closures with a fixed
+ /// small number of arguments in order to inherit `Function.apply`
+ /// metadata. The value is the fixed number of arguments.
+ final int sharedClosureApplyMetadata;
final bool isMixinApplicationWithMembers;
@@ -279,6 +286,7 @@
this.isDirectlyInstantiated,
this.isNative,
this.isClosureBaseClass,
+ this.sharedClosureApplyMetadata,
this.isMixinApplicationWithMembers}) {
assert(onlyForRti != null);
assert(onlyForConstructor != null);
@@ -492,6 +500,8 @@
/// functions that can be torn off.
final bool isClosureCallMethod;
+ final bool inheritsApplyMetadata;
+
/// True if the interceptor calling convention is used for this method.
final bool isIntercepted;
@@ -512,6 +522,7 @@
int requiredParameterCount,
/* List | Map */ optionalParameterDefaultValues,
this.isClosureCallMethod,
+ this.inheritsApplyMetadata,
this.isIntercepted,
js.Expression functionType,
int applyIndex,
diff --git a/pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart b/pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart
index 6c581a5..2a8ae0a 100644
--- a/pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart
+++ b/pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart
@@ -557,7 +557,18 @@
bool onlyForRti = _nativeData.isJsInteropClass(cls);
bool hasRtiField = _rtiNeed.classNeedsTypeArguments(cls);
bool onlyForConstructorOrRti = onlyForConstructor || onlyForRti;
+
+ // Recognize the specialized base classes for closures.
bool isClosureBaseClass = cls == _commonElements.closureClass;
+ int sharedClosureApplyMetadata;
+ if (cls == _commonElements.closureClass) {
+ // The root base class has metadata for single-argument closures.
+ sharedClosureApplyMetadata = 1;
+ } else if (cls == _commonElements.closureClass0Args) {
+ sharedClosureApplyMetadata = 0;
+ } else if (cls == _commonElements.closureClass2Args) {
+ sharedClosureApplyMetadata = 2;
+ }
List<Method> methods = [];
List<StubMethod> callStubs = [];
@@ -712,6 +723,7 @@
assert(!_nativeData.isNativeClass(cls));
assert(methods.isEmpty);
assert(!isClosureBaseClass);
+ assert(sharedClosureApplyMetadata == null);
result = MixinApplication(cls, typeData, name, instanceFields, callStubs,
checkedSetters, gettersSetters, isChecks, typeTests.functionTypeIndex,
@@ -738,6 +750,7 @@
onlyForConstructor: onlyForConstructor,
isNative: _nativeData.isNativeClass(cls),
isClosureBaseClass: isClosureBaseClass,
+ sharedClosureApplyMetadata: sharedClosureApplyMetadata,
isMixinApplicationWithMembers: isMixinApplicationWithMembers);
}
_classes[cls] = result;
@@ -814,6 +827,7 @@
bool tearOffNeedsDirectAccess = false;
js.Name tearOffName;
bool isClosureCallMethod = false;
+ bool inheritsApplyMetadata = false;
bool isNotApplyTarget =
!element.isFunction || element.isGetter || element.isSetter;
@@ -829,6 +843,17 @@
if (element.enclosingClass.isClosure) {
canTearOff = false;
isClosureCallMethod = true;
+ ClassEntity superclass =
+ _elementEnvironment.getSuperClass(element.enclosingClass);
+ if (superclass == _commonElements.closureClass &&
+ element.parameterStructure == ParameterStructure.oneArgument ||
+ superclass == _commonElements.closureClass0Args &&
+ element.parameterStructure ==
+ ParameterStructure.zeroArguments ||
+ superclass == _commonElements.closureClass2Args &&
+ element.parameterStructure == ParameterStructure.twoArguments) {
+ inheritsApplyMetadata = true;
+ }
} else {
// Careful with operators.
bool needsSuperGetter = _codegenWorld.methodsNeedsSuperGetter(element);
@@ -881,6 +906,7 @@
tearOffName: tearOffName,
tearOffNeedsDirectAccess: tearOffNeedsDirectAccess,
isClosureCallMethod: isClosureCallMethod,
+ inheritsApplyMetadata: inheritsApplyMetadata,
isIntercepted: isIntercepted,
aliasName: aliasName,
canBeApplied: canBeApplied,
diff --git a/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart b/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart
index 9f40195..bf57340 100644
--- a/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart
+++ b/pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart
@@ -592,6 +592,9 @@
_call1Name ??= _namer.getNameForJsGetName(null, JsGetName.CALL_PREFIX1);
js.Name get call2Name =>
_call2Name ??= _namer.getNameForJsGetName(null, JsGetName.CALL_PREFIX2);
+ List<js.Name> _callNamesByArity;
+ List<js.Name> get callNamesByArity =>
+ _callNamesByArity ??= [call0Name, call1Name, call2Name];
FragmentEmitter(
this._options,
@@ -1068,26 +1071,31 @@
allMethods.forEach((Method method) {
emitInstanceMethod(method)
.forEach((js.Expression name, js.Expression code) {
- var prop = js.Property(name, code);
- registerEntityAst(method.element, prop);
- properties.add(prop);
+ final property = js.Property(name, code);
+ registerEntityAst(method.element, property);
+ properties.add(property);
});
});
- if (cls.isClosureBaseClass) {
- // Closures extend a common base class, so we can put properties on the
- // prototype for common values.
+ // Closures have metadata that is often the same. We avoid repeated metadata
+ // by putting it on a shared superclass. It is overridden in the subclass if
+ // necessary.
- // Closures taking exactly one argument are common.
+ int arity = cls.sharedClosureApplyMetadata;
+ if (arity != null) {
+ // This is a closure base class that has the specialized `Function.apply`
+ // metadata for functions taking exactly [arity] arguments.
properties.add(js.Property(js.string(_namer.fixedNames.callCatchAllName),
- js.quoteName(call1Name)));
+ js.quoteName(callNamesByArity[arity])));
properties.add(js.Property(
- js.string(_namer.fixedNames.requiredParameterField), js.number(1)));
+ js.string(_namer.fixedNames.requiredParameterField),
+ js.number(arity)));
+ }
+ if (cls.isClosureBaseClass) {
// Most closures have no optional arguments.
properties.add(js.Property(
- js.string(_namer.fixedNames.defaultValuesField),
- new js.LiteralNull()));
+ js.string(_namer.fixedNames.defaultValuesField), js.LiteralNull()));
}
return new js.ObjectInitializer(properties);
@@ -1112,35 +1120,27 @@
}
if (method.isClosureCallMethod && method.canBeApplied) {
- // TODO(sra): We should also add these properties for the user-defined
- // `call` method on classes. Function.apply is currently broken for
- // complex cases. [forceAdd] might be true when this is fixed.
- bool forceAdd = !method.isClosureCallMethod;
+ // The `call` method might flow to `Function.apply`, so the metadata for
+ // `Function.apply` is needed.
- // Common case of "call*": "call$1" is stored on the Closure class.
- if (method.applyIndex != 0 ||
- method.parameterStubs.isNotEmpty ||
- method.requiredParameterCount != 1 ||
- forceAdd) {
+ // Avoid adding the metadata if a superclass has the same metadata.
+ if (!method.inheritsApplyMetadata) {
js.Name applyName = method.applyIndex == 0
? method.name
: method.parameterStubs[method.applyIndex - 1].name;
properties[js.string(_namer.fixedNames.callCatchAllName)] =
js.quoteName(applyName);
- }
- // Common case of '1' is stored on the Closure class.
- if (method.requiredParameterCount != 1 || forceAdd) {
properties[js.string(_namer.fixedNames.requiredParameterField)] =
js.number(method.requiredParameterCount);
- }
- js.Expression defaultValues =
- _encodeOptionalParameterDefaultValues(method);
- // Default values property of `null` is stored on the common JS
- // superclass.
- if (defaultValues is! js.LiteralNull || forceAdd) {
- properties[js.string(_namer.fixedNames.defaultValuesField)] =
- defaultValues;
+ js.Expression defaultValues =
+ _encodeOptionalParameterDefaultValues(method);
+ // Default values property of `null` is stored on the common JS
+ // superclass.
+ if (defaultValues is! js.LiteralNull) {
+ properties[js.string(_namer.fixedNames.defaultValuesField)] =
+ defaultValues;
+ }
}
}
}
diff --git a/pkg/compiler/lib/src/js_model/js_world_builder.dart b/pkg/compiler/lib/src/js_model/js_world_builder.dart
index c07ccea..8a4928d 100644
--- a/pkg/compiler/lib/src/js_model/js_world_builder.dart
+++ b/pkg/compiler/lib/src/js_model/js_world_builder.dart
@@ -417,7 +417,8 @@
Map<ir.VariableDeclaration, JRecordField> boxedVariables,
KernelScopeInfo info,
{bool createSignatureMethod}) {
- ClassEntity superclass = _commonElements.closureClass;
+ ClassEntity superclass =
+ _chooseClosureSuperclass(originalClosureFunctionNode);
JsClosureClassInfo closureClassInfo = _elementMap.constructClosureClass(
member,
@@ -440,6 +441,21 @@
return closureClassInfo;
}
+ ClassEntity _chooseClosureSuperclass(ir.FunctionNode node) {
+ // Choose a superclass so that similar closures can share the metadata used
+ // by `Function.apply`.
+ int requiredParameterCount = node.requiredParameterCount;
+ if (node.typeParameters.isEmpty &&
+ node.namedParameters.isEmpty &&
+ requiredParameterCount == node.positionalParameters.length) {
+ if (requiredParameterCount == 0) return _commonElements.closureClass0Args;
+ if (requiredParameterCount == 2) return _commonElements.closureClass2Args;
+ }
+ // Note that the base closure class has specialized metadata for the common
+ // case of single-argument functions.
+ return _commonElements.closureClass;
+ }
+
OutputUnitData _convertOutputUnitData(JsToFrontendMapImpl map,
OutputUnitData data, ClosureData closureDataLookup) {
// Convert front-end maps containing K-class and K-local function keys to a
diff --git a/sdk/lib/_internal/js_runtime/lib/js_helper.dart b/sdk/lib/_internal/js_runtime/lib/js_helper.dart
index 4ef1735..f06c29a 100644
--- a/sdk/lib/_internal/js_runtime/lib/js_helper.dart
+++ b/sdk/lib/_internal/js_runtime/lib/js_helper.dart
@@ -2342,6 +2342,12 @@
return Closure.fromTearOff(parameters);
}
+/// Base class for closures with no arguments.
+abstract class Closure0Args extends Closure {}
+
+/// Base class for closures with two positional arguments.
+abstract class Closure2Args extends Closure {}
+
/// Represents an implicit closure of a function.
abstract class TearOffClosure extends Closure {}
diff --git a/tests/web/internal/lax_runtime_type_closure_to_string1_test.dart b/tests/web/internal/lax_runtime_type_closure_to_string1_test.dart
index aed91ee..220d3c6 100644
--- a/tests/web/internal/lax_runtime_type_closure_to_string1_test.dart
+++ b/tests/web/internal/lax_runtime_type_closure_to_string1_test.dart
@@ -18,7 +18,7 @@
var toString = '${local1.runtimeType}';
if ('$Object' == 'Object') {
// `true` if non-minified.
- Expect.equals("Closure", toString);
+ Expect.equals("Closure0Args", toString);
}
print(toString);
local2(0);
diff --git a/tests/web_2/internal/lax_runtime_type_closure_to_string1_test.dart b/tests/web_2/internal/lax_runtime_type_closure_to_string1_test.dart
index 51d90d5..210d320 100644
--- a/tests/web_2/internal/lax_runtime_type_closure_to_string1_test.dart
+++ b/tests/web_2/internal/lax_runtime_type_closure_to_string1_test.dart
@@ -20,7 +20,7 @@
var toString = '${local1.runtimeType}';
if ('$Object' == 'Object') {
// `true` if non-minified.
- Expect.equals("Closure", toString);
+ Expect.equals("Closure0Args", toString);
}
print(toString);
local2(0);