languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-14 17:59:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

[ddc] Apply select inlining for dart:_rti

Browse source 
Adds simple method inlining for select patterns only in the dart:_rti
library as an optimization. This helps avoid chains of costly
accesses method calls that in the end simply perform a single
operation and return the result.

For example and snipet from the compiled SDK before:

```
if (_rti._isString(object)) {...}
```
and after:

```
if(typeof object == "string") {...}
```

Issue: https://github.com/dart-lang/sdk/issues/48585
Change-Id: I90596294d35a8fd75d74014c6a12f6e8c726cfcc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/324571
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
This commit is contained in:
Nicholas Shahan 2023-09-30 00:20:58 +00:00 committed by Commit Queue
parent 3fae23ae69
commit 53ba601220
4 changed files with 269 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

55
pkg/dev_compiler/lib/src/kernel/compiler.dart
View file

@ -16,6 +16,7 @@ import 'package:collection/collection.dart'
import 'package:front_end/src/api_unstable/ddc.dart';
import 'package:js_shared/synced/embedded_names.dart' show JsGetName, JsBuiltin;
import 'package:kernel/class_hierarchy.dart';
import 'package:kernel/clone.dart';
import 'package:kernel/core_types.dart';
import 'package:kernel/kernel.dart';
import 'package:kernel/library_index.dart';
@ -144,6 +145,10 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
final TypeRecipeGenerator _typeRecipeGenerator;
/// Visitor used for testing static invocations in the dart:_rti library to
/// determine if they are suitable for inlining at call sites.
final BasicInlineTester _inlineTester;
/// The current element being loaded.
/// We can use this to determine if we're loading top-level code or not:
///
@ -377,7 +382,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
_extensionTypeEraser = ExtensionTypeEraser(),
_typeRecipeGenerator = TypeRecipeGenerator(_coreTypes, _hierarchy),
_extensionIndex =
ExtensionIndex(_coreTypes, _staticTypeContext.typeEnvironment);
ExtensionIndex(_coreTypes, _staticTypeContext.typeEnvironment),
_inlineTester = BasicInlineTester(_constants);
@override
Library? get currentLibrary => _currentLibrary;
@ -6316,6 +6322,53 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
if (target.isFactory) return _emitFactoryInvocation(node);
var enclosingLibrary = target.enclosingLibrary;
if (isDartLibrary(enclosingLibrary, '_rti') &&
_inlineTester.canInline(target.function)) {
// Transform code that would otherwise appear as a static invocation:
// ```
// if (_rti._isString(object)) {...}
// ```
//
// to be avoid cost of extra function calls:
//
// ```
// if (typeof object == "string") {...}
// ```
var body = node.target.function.body;
Expression? bodyToInline;
// Extract the body.
if (body is ReturnStatement) {
// Ex: foo() => <body>;
bodyToInline = body.expression;
} else if (body is Block) {
// Ex: foo() { <body> }
var singleStatement = body.statements.single;
if (singleStatement is ReturnStatement) {
bodyToInline = singleStatement.expression;
}
}
if (bodyToInline != null) {
// Clone the function parameters and create the mappings from the clone
// to the argument passed.
var cloner = CloneVisitorNotMembers();
var originalParameters = target.function.positionalParameters;
var replacementArguments = node.arguments.positional;
var replacements = {
for (var i = 0; i < originalParameters.length; i++)
originalParameters[i].accept(cloner) as VariableDeclaration:
replacementArguments[i],
};
// Clone the body using the same cloner to ensure the cloned parameters
// are correctly linked to their accesses.
var cloneToInline = bodyToInline.accept(cloner);
// Substitute the use of the parameters with the values passed.
var replacer = VariableGetReplacer(replacements);
var replaced = cloneToInline.accept(replacer) as Expression;
// Compile the result normally and wrap in parenthesis.
return js.call('(#)', [replaced.accept(this)]);
}
}
if (_isDartInternal(enclosingLibrary)) {
var args = node.arguments;
if (args.positional.length == 1 &&

202
pkg/dev_compiler/lib/src/kernel/kernel_helpers.dart
View file

@ -3,11 +3,14 @@
// BSD-style license that can be found in the LICENSE file.
import 'dart:collection';
import 'package:collection/collection.dart';
import 'package:kernel/core_types.dart';
import 'package:kernel/kernel.dart' hide Pattern;
import 'package:kernel/src/replacement_visitor.dart';
import 'constants.dart';
Never throwUnsupportedInvalidType(InvalidType type) => throw UnsupportedError(
'Unsupported invalid type $type (${type.runtimeType}).');
@ -387,3 +390,202 @@ class ExtensionTypeEraser extends ReplacementVisitor {
DartType? visitExtensionType(ExtensionType node, int variance) =>
node.typeErasure.accept1(this, Variance.unrelated) ?? node.typeErasure;
}
/// Replaces [VariableGet] nodes with a different expression defined by a
/// replacement map.
class VariableGetReplacer extends Transformer {
final Map<VariableDeclaration, Expression> _replacements;
VariableGetReplacer(this._replacements);
@override
TreeNode visitVariableGet(VariableGet node) {
var replacement = _replacements[node.variable];
return replacement ?? node;
}
}
/// Tests a [StaticInvocation] node to determine if it would be safe to inline.
///
/// Each static invocation should be inspected individually as this class only
/// inspects the body of a given invocation target, and does not recurse
/// transitively into the bodies of other invocations it finds.
///
/// The determination of what is safe to inline is specifically targeting simple
/// methods in the dart:_rti library and has not been validated to use as an
/// inliner for static methods from any other libraries. Instead of inlining by
/// introducing let variables that preserve the evaluation order of the call,
/// only code where it is safe to replace the argument of the call at the
/// place where it used is considered suitable.
///
/// The body of the target method may only contain simple expressions known to
/// have no side-effects like null, bool or string literals, constants, enum
/// values, and variable accesses. Additionally some static invocations from the
/// runtime libraries are permitted in the body when they have been manually
/// inspected for side effects and annotated as safe with
/// `@pragma('ddc:trust-inline`). After the last method argument has been used,
/// this restriction is weakened to allow any static invocations to appear.
class BasicInlineTester extends TreeVisitorDefault<bool> {
/// The constants used to access constant evaluation for annotations.
final DevCompilerConstants _constants;
/// The order that the arguments are expected to used in the body of the
/// function for it to be considered safe for inlining.
///
/// This is essential to preserve the execution order of the expressions
/// passed as arguments.
List<VariableDeclaration>? _expectedArgumentOrder;
/// The position in [_expectedArgumentOrder] to begin looking for the next
/// used argument.
int _nextArgIndex = 0;
/// Returns `true` if uses of all the expected arguments have already been
/// seen by the visitor.
bool get _allArgumentsUsed => _nextArgIndex >= _expectedArgumentOrder!.length;
BasicInlineTester(this._constants);
/// Returns `true` when [possibleInline] is considered simple enough to
/// be safe to inline.
///
/// The considerations for inlining are designed specifically to target select
/// invocations in the dart:_rti library.
bool canInline(FunctionNode possibleInline) {
if (possibleInline.namedParameters.isNotEmpty ||
possibleInline.typeParameters.isNotEmpty ||
possibleInline.positionalParameters.length !=
possibleInline.requiredParameterCount) {
// Only consider functions with required positional arguments.
return false;
}
final body = possibleInline.body;
// No code available here to inline.
if (body == null) return false;
_expectedArgumentOrder = possibleInline.positionalParameters;
_nextArgIndex = 0;
// Important to note that the static invocation being considered for inline
// is not visited. Instead the body of the target function being invoked is
// visited to determine if inlining is feasible. Any other static
// invocations appearing within that body *will* be visited.
final result = body.accept(this);
// Avoid retaining any of the arguments after returning.
_expectedArgumentOrder = null;
return result;
}
/// Returns `true` when [node] is annotated with
/// `@pragma('ddc:trust-inline`)`.
bool _hasTrustInlinePragma(NamedNode node) {
var annotation = findAnnotation(node, (e) {
if (!isBuiltinAnnotation(e, 'core', 'pragma')) return false;
var name = _constants.getFieldValueFromAnnotation(e, 'name') as String?;
return name == 'ddc:trust-inline';
});
return annotation != null;
}
@override
bool defaultTreeNode(Node _) => false;
@override
bool visitReturnStatement(ReturnStatement node) {
var returnExpression = node.expression;
return returnExpression == null ? false : returnExpression.accept(this);
}
@override
bool visitBlock(Block node) {
if (node.statements.length != 1) return false;
return node.statements.single.accept(this);
}
@override
bool visitStaticInvocation(StaticInvocation node) {
// Reaching this visitor means that a static invocation appears in the body
// of the function being evaluated for inlining.
var positionalArgs = node.arguments.positional;
if (isInlineJS(node.target)) {
// Ensure JS expressions do not span multiple statements. Since these are
// inlined elsewhere we don't want to accidentally combine multiple
// Javascript statements into a position where they don't belong. Instead
// of inspecting if it is valid, just reject any source templates that
// contain ';' characters.
var codeLiteral = positionalArgs[1];
String? code;
if (codeLiteral is StringLiteral) {
code = codeLiteral.value;
} else if (codeLiteral is ConstantExpression) {
var constant = codeLiteral.constant;
if (constant is StringConstant) {
code = constant.value;
}
}
if (code == null || code.contains(';')) return false;
}
// Inspect the arguments.
for (var argument in positionalArgs) {
if (!argument.accept(this)) return false;
}
// While all of the uses of the arguments are still being discovered,
// only allow additional static invocations explicitly annotated as
// trusted. This prevents inlining a static invocation that could
// change the evaluation order of the expressions passed as arguments.
// Example:
// ```
// echo(s) {
// print(s);
// return s;
// }
//
// dangerous(arg1, arg2) {
// return fn(arg1, echo('third'), arg2);
// }
// ```
// Calls to `dangerous()` should not be inlined without let variables
// because the expression evaluation order would be disrupted:
// ```
// dangerous(echo('first'), echo('second'));
// | |
// V V
// fn(echo('first'), echo('third'), echo('second')) // INVALID!
return _allArgumentsUsed || _hasTrustInlinePragma(node.target);
}
@override
bool visitBoolLiteral(BoolLiteral _) => true;
@override
bool visitNullLiteral(NullLiteral _) => true;
@override
bool visitStringLiteral(StringLiteral _) => true;
@override
bool visitConstantExpression(ConstantExpression _) => true;
@override
bool visitVariableGet(VariableGet node) {
// The variable is an argument of the function to be inlined. Verify it
// appears in an order consistent with the order the arguments appeared
// in the call.
final location =
_expectedArgumentOrder!.indexOf(node.variable, _nextArgIndex);
// Either the variable isn't one of the expected arguments at all, or it is
// appearing out of the expected order.
if (location == -1) return false;
// Advance the start of the expected range past the variable just found to
// ensure the variable can only be used once.
_nextArgIndex = location + 1;
return true;
}
@override
bool visitField(Field node) => node.isEnumElement;
@override
bool visitStaticGet(StaticGet node) {
return node.target.accept(this);
}
}

15
sdk/lib/_internal/js_dev_runtime/private/foreign_helper.dart
View file

@ -112,6 +112,7 @@ import 'dart:_rti' show Rti;
*/
// Add additional optional arguments if needed. The method is treated internally
// as a variable argument method.
@pragma('ddc:trust-inline')
external T JS<T extends Object?>(String typeDescription, String codeTemplate,
[arg0,
arg1,
@ -227,6 +228,7 @@ external String JS_FUNCTION_TYPE_OPTIONAL_PARAMETERS_TAG();
external String JS_FUNCTION_TYPE_NAMED_PARAMETERS_TAG();
/// Returns the JS name for [name] from the Namer.
@pragma('ddc:trust-inline')
external String JS_GET_NAME(JsGetName name);
/// Returns the state of a flag that is determined by the state of the compiler
@ -286,6 +288,7 @@ dynamic spread(args) {
///
/// The [name] should be a constant defined in the `_js_shared_embedded_names`
/// library.
@pragma('ddc:trust-inline')
external JS_EMBEDDED_GLOBAL(String typeDescription, String name);
/// Instructs the compiler to execute the [builtinName] action at the call-site.
@ -318,18 +321,20 @@ Object getJSArrayInteropRti() => TYPE_REF<JSArray>();
/// A valid example of where this can be used is as the second argument
/// to V8's Error.captureStackTrace. See
/// https://code.google.com/p/v8/wiki/JavaScriptStackTraceApi.
// TODO(nshahan) Replace calls at compile time?
external RAW_DART_FUNCTION_REF(Function function);
/// Returns a TypeReference to [T].
// TODO(nshahan) Replace calls at compile time?
/// Returns a reference to the internal value that represents [T].
///
/// Static calls to this function are inserted directly by the compiler.
external Rti TYPE_REF<T>();
/// Returns a TypeReference to [T]*.
// TODO(nshahan) Replace calls at compile time?
/// Returns a reference to the internal value that represents [T]*.
///
/// Static calls to this function are inserted directly by the compiler.
external Rti LEGACY_TYPE_REF<T>();
/// JavaScript string concatenation. Inputs must be Strings.
@pragma('ddc:trust-inline')
external String JS_STRING_CONCAT(String a, String b);
/// Identifier used to access the JavaScript class definition for [type].

3
sdk/lib/_internal/js_shared/lib/rti.dart
View file

@ -511,6 +511,7 @@ class _FunctionParameters {
}
}
@pragma('ddc:trust-inline')
Object? _theUniverse() => JS_EMBEDDED_GLOBAL('', RTI_UNIVERSE);
Rti _rtiEval(Rti environment, String recipe) {
@ -1975,9 +1976,11 @@ class _Universe {
// Field accessors.
@pragma('ddc:trust-inline')
static Object evalCache(Object? universe) =>
JS('', '#.#', universe, RtiUniverseFieldNames.evalCache);
@pragma('ddc:trust-inline')
static Object typeRules(Object? universe) =>
JS('', '#.#', universe, RtiUniverseFieldNames.typeRules);