Skip to content

Commit

Permalink
Optimize js_util getProperty calls to lowered call to native JS.
Browse files Browse the repository at this point in the history
No change in the generated JavaScript for dart2js based on both a small
foo.dart sample file and tests/lib/js/js_util/properties_test.dart

All changes in generated JavaScript for DDC are smaller and clearer:
  - foo.dart: https://paste.googleplex.com/4738045023617024
  - properties_test.dart: https://paste.googleplex.com/5107699705446400

Bug: #44533
Change-Id: I670f0226fbef90d05a95c51d918831bea58d6aa3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/187840
Commit-Queue: Riley Porter <rileyporter@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
  • Loading branch information
rileyporter authored and commit-bot@chromium.org committed Mar 6, 2021
1 parent 7aaf620 commit bfaf674
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:kernel/ast.dart';
import 'package:kernel/core_types.dart';
import 'package:kernel/kernel.dart';

/// Replaces js_util methods with inline calls to foreign_helper JS which
/// emits the code as a JavaScript code fragment.
class JsUtilOptimizer extends Transformer {
final Procedure _jsTarget;
final Procedure _getPropertyTarget;

JsUtilOptimizer(CoreTypes coreTypes)
: _jsTarget =
coreTypes.index.getTopLevelMember('dart:_foreign_helper', 'JS'),
_getPropertyTarget =
coreTypes.index.getTopLevelMember('dart:js_util', 'getProperty') {}

/// Replaces js_util method calls with lowering straight to JS fragment call.
///
/// Lowers the following types of js_util calls:
/// - `getProperty` for any argument types
@override
visitStaticInvocation(StaticInvocation node) {
if (node.target == _getPropertyTarget) {
node = _lowerGetProperty(node);
}
node.transformChildren(this);
return node;
}

/// Lowers the given js_util `getProperty` call to the foreign_helper JS call
/// for any argument type. Lowers `getProperty(o, name)` to
/// `JS('Object|Null', '#.#', o, name)`.
StaticInvocation _lowerGetProperty(StaticInvocation node) {
Arguments args = node.arguments;
assert(args.positional.length == 2);
return StaticInvocation(
_jsTarget,
Arguments(
[
StringLiteral("Object|Null"),
StringLiteral("#.#"),
args.positional.first,
args.positional.last
],
// TODO(rileyporter): Copy type from getProperty when it's generic.
types: [DynamicType()],
))
..fileOffset = node.fileOffset;
}
}
7 changes: 7 additions & 0 deletions pkg/compiler/lib/src/kernel/dart2js_target.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ library compiler.src.kernel.dart2js_target;
import 'package:_fe_analyzer_shared/src/messages/codes.dart'
show Message, LocatedMessage;
import 'package:_js_interop_checks/js_interop_checks.dart';
import 'package:_js_interop_checks/src/transformations/js_util_optimizer.dart';
import 'package:kernel/ast.dart' as ir;
import 'package:kernel/class_hierarchy.dart';
import 'package:kernel/core_types.dart';
Expand Down Expand Up @@ -92,8 +93,10 @@ class Dart2jsTarget extends Target {

@override
List<String> get extraIndexedLibraries => const [
'dart:_foreign_helper',
'dart:_interceptors',
'dart:_js_helper',
'dart:js_util'
];

@override
Expand Down Expand Up @@ -127,7 +130,11 @@ class Dart2jsTarget extends Target {
{void logger(String msg),
ChangedStructureNotifier changedStructureNotifier}) {
_nativeClasses ??= JsInteropChecks.getNativeClasses(component);
var jsUtilOptimizer = JsUtilOptimizer(coreTypes);
for (var library in libraries) {
// TODO (rileyporter): Merge js_util optimizations with other lowerings
// in the single pass in `transformations/lowering.dart`.
jsUtilOptimizer.visitLibrary(library);
JsInteropChecks(
coreTypes,
diagnosticReporter as DiagnosticReporter<Message, LocatedMessage>,
Expand Down
5 changes: 5 additions & 0 deletions pkg/dev_compiler/lib/src/kernel/target.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import 'package:kernel/target/changed_structure_notifier.dart';
import 'package:kernel/target/targets.dart';
import 'package:kernel/transformations/track_widget_constructor_locations.dart';
import 'package:_js_interop_checks/js_interop_checks.dart';
import 'package:_js_interop_checks/src/transformations/js_util_optimizer.dart';

import 'constants.dart' show DevCompilerConstantsBackend;
import 'kernel_helpers.dart';
Expand Down Expand Up @@ -92,11 +93,13 @@ class DevCompilerTarget extends Target {
'dart:collection',
'dart:html',
'dart:indexed_db',
'dart:js_util',
'dart:math',
'dart:svg',
'dart:web_audio',
'dart:web_gl',
'dart:web_sql',
'dart:_foreign_helper',
'dart:_interceptors',
'dart:_js_helper',
'dart:_native_typed_data',
Expand Down Expand Up @@ -152,8 +155,10 @@ class DevCompilerTarget extends Target {
{void Function(String msg) logger,
ChangedStructureNotifier changedStructureNotifier}) {
_nativeClasses ??= JsInteropChecks.getNativeClasses(component);
var jsUtilOptimizer = JsUtilOptimizer(coreTypes);
for (var library in libraries) {
_CovarianceTransformer(library).transform();
jsUtilOptimizer.visitLibrary(library);
JsInteropChecks(
coreTypes,
diagnosticReporter as DiagnosticReporter<Message, LocatedMessage>,
Expand Down
22 changes: 22 additions & 0 deletions tests/lib/js/js_util/properties_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class Foo {
external num get a;
external num bar();
external Object get objectProperty;
external List get list;
}

@JS('Foo')
Expand Down Expand Up @@ -213,6 +214,11 @@ main() {
js_util.callMethod(
js_util.getProperty(f, 'fnList')[0], 'apply', [f, []]),
equals(42));
expect(js_util.getProperty(f.list, "0"), equals(2));
var index = 0;
expect(js_util.getProperty(f.list, index++), equals(2));
expect(index, equals(1));
expect(js_util.getProperty(f.list, index), equals(4));

// Accessing nested object properites.
var objectProperty = js_util.getProperty(f, 'objectProperty');
Expand All @@ -222,6 +228,13 @@ main() {
expect(
js_util.getProperty(objectProperty, 'functionProperty') is Function,
isTrue);
// Using nested getProperty calls.
expect(
js_util.getProperty(
js_util.getProperty(
js_util.getProperty(f, 'objectProperty'), 'list'),
1),
equals(20));

// Using a variable for the property name.
String propertyName = 'a';
Expand Down Expand Up @@ -289,6 +302,10 @@ main() {
expect(js_util.getProperty(f.objectProperty, 'c'), equals('new val'));
js_util.setProperty(f.objectProperty, 'list', [1, 2, 3]);
expect(js_util.getProperty(f.objectProperty, 'list')[1], equals(2));
// Using a nested getProperty call.
js_util.setProperty(
js_util.getProperty(f, 'objectProperty'), 'c', 'nested val');
expect(js_util.getProperty(f.objectProperty, 'c'), equals('nested val'));

// Using a variable for the property name.
String propertyName = 'bar';
Expand Down Expand Up @@ -317,6 +334,11 @@ main() {
// Call method on a nested function property.
expect(js_util.callMethod(f.objectProperty, 'functionProperty', []),
equals('Function Property'));
// Using a nested getProperty call.
expect(
js_util.callMethod(
js_util.getProperty(f, 'objectProperty'), 'functionProperty', []),
equals('Function Property'));

// Call method with different args.
expect(
Expand Down
22 changes: 22 additions & 0 deletions tests/lib_2/js/js_util/properties_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class Foo {
external num get a;
external num bar();
external Object get objectProperty;
external List get list;
}

@JS('Foo')
Expand Down Expand Up @@ -213,6 +214,11 @@ main() {
js_util.callMethod(
js_util.getProperty(f, 'fnList')[0], 'apply', [f, []]),
equals(42));
expect(js_util.getProperty(f.list, "0"), equals(2));
var index = 0;
expect(js_util.getProperty(f.list, index++), equals(2));
expect(index, equals(1));
expect(js_util.getProperty(f.list, index), equals(4));

// Accessing nested object properites.
var objectProperty = js_util.getProperty(f, 'objectProperty');
Expand All @@ -222,6 +228,13 @@ main() {
expect(
js_util.getProperty(objectProperty, 'functionProperty') is Function,
isTrue);
// Using nested getProperty calls.
expect(
js_util.getProperty(
js_util.getProperty(
js_util.getProperty(f, 'objectProperty'), 'list'),
1),
equals(20));

// Using a variable for the property name.
String propertyName = 'a';
Expand Down Expand Up @@ -289,6 +302,10 @@ main() {
expect(js_util.getProperty(f.objectProperty, 'c'), equals('new val'));
js_util.setProperty(f.objectProperty, 'list', [1, 2, 3]);
expect(js_util.getProperty(f.objectProperty, 'list')[1], equals(2));
// Using a nested getProperty call.
js_util.setProperty(
js_util.getProperty(f, 'objectProperty'), 'c', 'nested val');
expect(js_util.getProperty(f.objectProperty, 'c'), equals('nested val'));

// Using a variable for the property name.
String propertyName = 'bar';
Expand Down Expand Up @@ -317,6 +334,11 @@ main() {
// Call method on a nested function property.
expect(js_util.callMethod(f.objectProperty, 'functionProperty', []),
equals('Function Property'));
// Using a nested getProperty call.
expect(
js_util.callMethod(
js_util.getProperty(f, 'objectProperty'), 'functionProperty', []),
equals('Function Property'));

// Call method with different args.
expect(
Expand Down

0 comments on commit bfaf674

Please sign in to comment.