From b3a93f44eb6615507a47151138a2b8a0446b3821 Mon Sep 17 00:00:00 2001 From: Stephen Adams Date: Fri, 27 Oct 2017 21:21:13 +0000 Subject: [PATCH] Add dart2js `noInline` and `tryInline` annotations to package:meta There will be a follow-up change to remove NoInline from package:expect. Change-Id: I6dc33241ea6d8b6a8994da6a8ec5295dab30f4da Reviewed-on: https://dart-review.googlesource.com/17043 Reviewed-by: Sigmund Cherem --- pkg/compiler/lib/src/common_elements.dart | 33 ++++++++ .../lib/src/js_backend/annotations.dart | 14 ++++ pkg/compiler/lib/src/js_backend/backend.dart | 18 ++++- pkg/meta/CHANGELOG.md | 4 + pkg/meta/lib/dart2js.dart | 42 ++++++++++ pkg/meta/pubspec.yaml | 2 +- .../dart2js/meta_annotations2_test.dart | 66 +++++++++++++++ .../dart2js/meta_annotations3_test.dart | 81 +++++++++++++++++++ .../dart2js/meta_annotations_test.dart | 69 ++++++++++++++++ 9 files changed, 326 insertions(+), 3 deletions(-) create mode 100644 pkg/meta/lib/dart2js.dart create mode 100644 tests/compiler/dart2js/meta_annotations2_test.dart create mode 100644 tests/compiler/dart2js/meta_annotations3_test.dart create mode 100644 tests/compiler/dart2js/meta_annotations_test.dart diff --git a/pkg/compiler/lib/src/common_elements.dart b/pkg/compiler/lib/src/common_elements.dart index 03763ac600a..2b30ddb712b 100644 --- a/pkg/compiler/lib/src/common_elements.dart +++ b/pkg/compiler/lib/src/common_elements.dart @@ -1172,6 +1172,39 @@ class CommonElements { return _expectAssumeDynamicClass; } + static final Uri PACKAGE_META_DART2JS = + new Uri(scheme: 'package', path: 'meta/dart2js.dart'); + + bool _metaAnnotationChecked = false; + ClassEntity _metaNoInlineClass; + ClassEntity _metaTryInlineClass; + + void _ensureMetaAnnotations() { + if (!_metaAnnotationChecked) { + _metaAnnotationChecked = true; + LibraryEntity library = _env.lookupLibrary(PACKAGE_META_DART2JS); + if (library != null) { + _metaNoInlineClass = _env.lookupClass(library, '_NoInline'); + _metaTryInlineClass = _env.lookupClass(library, '_TryInline'); + if (_metaNoInlineClass == null || _metaTryInlineClass == null) { + // This is not the package you're looking for. + _metaNoInlineClass = null; + _metaTryInlineClass = null; + } + } + } + } + + ClassEntity get metaNoInlineClass { + _ensureMetaAnnotations(); + return _metaNoInlineClass; + } + + ClassEntity get metaTryInlineClass { + _ensureMetaAnnotations(); + return _metaTryInlineClass; + } + bool isForeign(MemberEntity element) => element.library == foreignLibrary; /// Returns `true` if the implementation of the 'operator ==' [function] is diff --git a/pkg/compiler/lib/src/js_backend/annotations.dart b/pkg/compiler/lib/src/js_backend/annotations.dart index 6d5470ff5f2..906f4015b66 100644 --- a/pkg/compiler/lib/src/js_backend/annotations.dart +++ b/pkg/compiler/lib/src/js_backend/annotations.dart @@ -11,6 +11,10 @@ import '../elements/entities.dart'; /// Returns `true` if inlining is disabled for [element]. bool noInline(ElementEnvironment elementEnvironment, CommonElements commonElements, MemberEntity element) { + if (_hasAnnotation( + elementEnvironment, element, commonElements.metaNoInlineClass)) { + return true; + } if (_hasAnnotation( elementEnvironment, element, commonElements.expectNoInlineClass)) { // TODO(floitsch): restrict to elements from the test directory. @@ -20,6 +24,16 @@ bool noInline(ElementEnvironment elementEnvironment, elementEnvironment, element, commonElements.noInlineClass); } +/// Returns `true` if inlining is requested for [element]. +bool tryInline(ElementEnvironment elementEnvironment, + CommonElements commonElements, MemberEntity element) { + if (_hasAnnotation( + elementEnvironment, element, commonElements.metaTryInlineClass)) { + return true; + } + return false; +} + /// Returns `true` if parameter and returns types should be trusted for /// [element]. bool trustTypeAnnotations(ElementEnvironment elementEnvironment, diff --git a/pkg/compiler/lib/src/js_backend/backend.dart b/pkg/compiler/lib/src/js_backend/backend.dart index d542568116d..6b5e062d841 100644 --- a/pkg/compiler/lib/src/js_backend/backend.dart +++ b/pkg/compiler/lib/src/js_backend/backend.dart @@ -1019,12 +1019,27 @@ class JavaScriptBackend { return; } + bool hasNoInline = false; + bool hasForceInline = false; + if (element.isFunction || element.isConstructor) { if (optimizerHints.noInline( elementEnvironment, commonElements, element)) { + hasNoInline = true; inlineCache.markAsNonInlinable(element); } + if (optimizerHints.tryInline( + elementEnvironment, commonElements, element)) { + hasForceInline = true; + if (hasNoInline) { + reporter.reportErrorMessage(element, MessageKind.GENERIC, + {'text': '@tryInline must not be used with @noInline.'}); + } else { + inlineCache.markAsMustInline(element); + } + } } + if (element.isField) return; FunctionEntity method = element; @@ -1033,8 +1048,7 @@ class JavaScriptBackend { !canLibraryUseNative(library)) { return; } - bool hasNoInline = false; - bool hasForceInline = false; + bool hasNoThrows = false; bool hasNoSideEffects = false; for (ConstantValue constantValue diff --git a/pkg/meta/CHANGELOG.md b/pkg/meta/CHANGELOG.md index 3f893d47d72..05da93a161c 100644 --- a/pkg/meta/CHANGELOG.md +++ b/pkg/meta/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.1.4 + +* Add `@dart2js.noInline` and `@dart2js.tryInline` annotations. + ## 1.1.2 * Rollback SDK constraint update for 2.0.0. No longer needed. diff --git a/pkg/meta/lib/dart2js.dart b/pkg/meta/lib/dart2js.dart new file mode 100644 index 00000000000..9420b317cc3 --- /dev/null +++ b/pkg/meta/lib/dart2js.dart @@ -0,0 +1,42 @@ +// Copyright (c) 2017, 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. + +/// Constants for use in metadata annotations to provide hints to dart2js. + +library meta_dart2js; + +/// An annotation for methods to request that dart2js does not inline the +/// method. +/// +/// import 'package:meta/dart2js.dart' as dart2js; +/// +/// @dart2js.noInline +/// String text() => 'A String of unusual size'; +/// +const _NoInline noInline = const _NoInline(); + +/// An annotation for methods method to request that dart2js always inlines the +/// method. +/// +/// dart2js will attempt to inline the method regardless of its size. Even with +/// this annotation, there are conditions that can prevent dart2js from inlining +/// a method, including complex control flow. +/// +/// import 'package:meta/dart2js.dart' as dart2js; +/// +/// @dart2js.tryInline +/// String bigMethod() { +/// for (int i in "Hello".runes) print(i); +/// } +/// +/// It is an error to use both `@noInline` and `@tryInline`. +const _TryInline tryInline = const _TryInline(); + +class _NoInline { + const _NoInline(); +} + +class _TryInline { + const _TryInline(); +} diff --git a/pkg/meta/pubspec.yaml b/pkg/meta/pubspec.yaml index 387c252724e..ae9a1237711 100644 --- a/pkg/meta/pubspec.yaml +++ b/pkg/meta/pubspec.yaml @@ -1,5 +1,5 @@ name: meta -version: 1.1.3 +version: 1.1.4 author: Dart Team homepage: https://github.com/dart-lang/sdk/tree/master/pkg/meta description: > diff --git a/tests/compiler/dart2js/meta_annotations2_test.dart b/tests/compiler/dart2js/meta_annotations2_test.dart new file mode 100644 index 00000000000..d615372d6d2 --- /dev/null +++ b/tests/compiler/dart2js/meta_annotations2_test.dart @@ -0,0 +1,66 @@ +// Copyright (c) 2017, 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. + +// Functional test of 'noInline' annotation from package:meta/dart2js.dart. + +import "package:expect/expect.dart"; +import "package:async_helper/async_helper.dart"; +import 'package:compiler/compiler_new.dart'; +import 'memory_compiler.dart'; + +const MEMORY_SOURCE_FILES = const { + 'main.dart': r''' + import 'package:meta/dart2js.dart'; + + @noInline + foo(y) => 49912344 + y; + + class A { + var field; + + @noInline + A([this.field = 4711]); + + @noInline + static bar(x) => x + 123455; + + @noInline + gee(x, y) => x + y + 81234512; + } + + main() { + print(foo(23412)); + print(A.bar(87654)); + print(new A().gee(1337, 919182)); + print(new A().field + 1); + }''' +}; + +void main() { + asyncTest(() async { + OutputCollector collector = new OutputCollector(); + await runCompiler( + memorySourceFiles: MEMORY_SOURCE_FILES, outputProvider: collector); + // Simply check that the constants of the small functions are still in the + // output, and that we don't see the result of constant folding. + String jsOutput = collector.getOutput('', OutputType.js); + + void has(String text) { + Expect.isTrue(jsOutput.contains(text), "output should contain '$text'"); + } + + void hasNot(String text) { + Expect.isFalse( + jsOutput.contains(text), "output must not contain '$text'"); + } + + has('49912344'); + has('123455'); + has('81234512'); + hasNot('49935756'); + hasNot('211109'); + hasNot('82155031'); + hasNot('4712'); + }); +} diff --git a/tests/compiler/dart2js/meta_annotations3_test.dart b/tests/compiler/dart2js/meta_annotations3_test.dart new file mode 100644 index 00000000000..005cbf18c49 --- /dev/null +++ b/tests/compiler/dart2js/meta_annotations3_test.dart @@ -0,0 +1,81 @@ +// Copyright (c) 2017, 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. + +// Functional test of 'tryInline' annotation from package:meta/dart2js.dart. + +import "package:expect/expect.dart"; +import "package:async_helper/async_helper.dart"; +import 'package:compiler/compiler_new.dart'; +import 'memory_compiler.dart'; + +const MEMORY_SOURCE_FILES = const { + 'main.dart': r''' + import 'package:meta/dart2js.dart'; + + monster1(u) { + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + } + + @tryInline + monster2(u) { + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + print(u + 1000); + } + + main() { + // large function used twice should not normally be inlined. + monster1(38000992); + monster1(38000993); + monster2(48000992); + monster2(48000993); + }''' +}; + +void main() { + asyncTest(() async { + OutputCollector collector = new OutputCollector(); + await runCompiler( + memorySourceFiles: MEMORY_SOURCE_FILES, outputProvider: collector); + String jsOutput = collector.getOutput('', OutputType.js); + + void has(String text) { + Expect.isTrue(jsOutput.contains(text), "output should contain '$text'"); + } + + void hasNot(String text) { + print(jsOutput); + Expect.isFalse( + jsOutput.contains(text), "output must not contain '$text'"); + } + + // Check that (u + 1000) from monster1 is not inlined and constant folded. + has('38000992'); + has('38000993'); + hasNot('38001992'); + hasNot('38001993'); + + // Check that (u + 1000) from monster2 is inlined and constant folded. + hasNot('48000992'); + hasNot('48000993'); + has('48001992'); + has('48001993'); + }); +} diff --git a/tests/compiler/dart2js/meta_annotations_test.dart b/tests/compiler/dart2js/meta_annotations_test.dart new file mode 100644 index 00000000000..f3ad7fb7b32 --- /dev/null +++ b/tests/compiler/dart2js/meta_annotations_test.dart @@ -0,0 +1,69 @@ +// Copyright (c) 2017, 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:expect/expect.dart'; +import 'package:async_helper/async_helper.dart'; +import 'package:compiler/src/compiler.dart'; +import 'package:compiler/src/elements/elements.dart'; +import 'package:compiler/src/js_backend/annotations.dart' as optimizerHints; +import 'package:compiler/src/world.dart' show ClosedWorld; +import 'memory_compiler.dart'; + +const Map MEMORY_SOURCE_FILES = const { + 'main.dart': r""" +import 'package:meta/dart2js.dart'; + +int method(String arg) => arg.length; + +@noInline +int methodNoInline(String arg) => arg.length; + +@tryInline +int methodTryInline(String arg) => arg.length; + + +void main(List args) { + print(method(args[0])); + print(methodNoInline('bar')); + print(methodTryInline('bar')); +} +""" +}; + +main() { + asyncTest(() async { + CompilationResult result = + await runCompiler(memorySourceFiles: MEMORY_SOURCE_FILES); + Compiler compiler = result.compiler; + ClosedWorld closedWorld = + compiler.resolutionWorldBuilder.closedWorldForTesting; + Expect.isFalse(compiler.compilationFailed, 'Unsuccessful compilation'); + Expect.isNotNull(closedWorld.commonElements.metaNoInlineClass, + 'NoInlineClass is unresolved.'); + Expect.isNotNull(closedWorld.commonElements.metaTryInlineClass, + 'TryInlineClass is unresolved.'); + + void test(String name, + {bool expectNoInline: false, bool expectTryInline: false}) { + LibraryElement mainApp = + compiler.frontendStrategy.elementEnvironment.mainLibrary; + MethodElement method = mainApp.find(name); + Expect.isNotNull(method); + Expect.equals( + expectNoInline, + optimizerHints.noInline(closedWorld.elementEnvironment, + closedWorld.commonElements, method), + "Unexpected annotation of @noInline on '$method'."); + Expect.equals( + expectTryInline, + optimizerHints.tryInline(closedWorld.elementEnvironment, + closedWorld.commonElements, method), + "Unexpected annotation of @tryInline on '$method'."); + } + + test('method'); + test('methodNoInline', expectNoInline: true); + test('methodTryInline', expectTryInline: true); + }); +}