From cf9f45f1d4bd4ba2d4475efd33e39e96b56be706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20A=C4=9Facan?= Date: Wed, 19 Mar 2025 08:10:16 -0700 Subject: [PATCH] [dart2wasm] Copy VM's map and set factory transformers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Transform factory calls to default map and set classes to the constructor calls to the classes to improve kernel. Also remove some redundant null checks in VM's transformer. `source_map_simple_optimized_test.dart` is updated: with improved kernel wasm-opt now eliminates the `testMain` function, so the stack trace doesn't mention it. Fixes https://github.com/dart-lang/sdk/issues/60343. Tested: minor refactoring in VM doesn't need testing. Wasm tested with existing tests. Change-Id: Ie448d1374ff0e1b278859f22bc250899e0e4cfd0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/416640 Reviewed-by: Martin Kustermann Commit-Queue: Ömer Ağacan --- pkg/dart2wasm/lib/factory_specializer.dart | 41 +++++++++++++++++++ .../lib/list_factory_specializer.dart | 10 ++--- .../lib/map_factory_specializer.dart | 37 +++++++++++++++++ .../lib/set_factory_specializer.dart | 38 +++++++++++++++++ pkg/dart2wasm/lib/transformers.dart | 16 +++++--- .../specializer/map_factory_specializer.dart | 18 ++++---- .../specializer/set_factory_specializer.dart | 18 ++++---- .../source_map_simple_optimized_test.dart | 1 - 8 files changed, 150 insertions(+), 29 deletions(-) create mode 100644 pkg/dart2wasm/lib/factory_specializer.dart create mode 100644 pkg/dart2wasm/lib/map_factory_specializer.dart create mode 100644 pkg/dart2wasm/lib/set_factory_specializer.dart diff --git a/pkg/dart2wasm/lib/factory_specializer.dart b/pkg/dart2wasm/lib/factory_specializer.dart new file mode 100644 index 00000000000..b77a4e11502 --- /dev/null +++ b/pkg/dart2wasm/lib/factory_specializer.dart @@ -0,0 +1,41 @@ +// Copyright (c) 2025, 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/core_types.dart'; +import 'package:kernel/kernel.dart'; + +import 'list_factory_specializer.dart'; +import 'map_factory_specializer.dart'; +import 'set_factory_specializer.dart'; + +typedef SpecializerTransformer = TreeNode Function(StaticInvocation node); + +abstract class BaseSpecializer { + // Populated in constructors of subclasses. + final Map transformers = {}; +} + +class FactorySpecializer extends BaseSpecializer { + final ListFactorySpecializer _listFactorySpecializer; + final SetFactorySpecializer _setFactorySpecializer; + final MapFactorySpecializer _mapFactorySpecializer; + + FactorySpecializer(CoreTypes coreTypes) + : _listFactorySpecializer = ListFactorySpecializer(coreTypes), + _setFactorySpecializer = SetFactorySpecializer(coreTypes), + _mapFactorySpecializer = MapFactorySpecializer(coreTypes) { + transformers.addAll(_listFactorySpecializer.transformers); + transformers.addAll(_setFactorySpecializer.transformers); + transformers.addAll(_mapFactorySpecializer.transformers); + } + + TreeNode transformStaticInvocation(StaticInvocation invocation) { + final target = invocation.target; + final transformer = transformers[target]; + if (transformer != null) { + return transformer(invocation); + } + return invocation; + } +} diff --git a/pkg/dart2wasm/lib/list_factory_specializer.dart b/pkg/dart2wasm/lib/list_factory_specializer.dart index b64b096decc..45c181b3602 100644 --- a/pkg/dart2wasm/lib/list_factory_specializer.dart +++ b/pkg/dart2wasm/lib/list_factory_specializer.dart @@ -21,7 +21,7 @@ import 'package:kernel/core_types.dart' show CoreTypes; /// ``` class ListFactorySpecializer { final Map - _transformers = {}; + transformers = {}; final Procedure _fixedListEmptyFactory; final Procedure _fixedListFactory; @@ -58,14 +58,14 @@ class ListFactorySpecializer { .getProcedure('dart:_list', 'ModifiableFixedLengthList', 'filled'), _fixedListGenerateFactory = coreTypes.index.getProcedure( 'dart:_list', 'ModifiableFixedLengthList', 'generate') { - _transformers[_listFilledFactory] = _transformListFilledFactory; - _transformers[_listEmptyFactory] = _transformListEmptyFactory; - _transformers[_listGenerateFactory] = _transformListGenerateFactory; + transformers[_listFilledFactory] = _transformListFilledFactory; + transformers[_listEmptyFactory] = _transformListEmptyFactory; + transformers[_listGenerateFactory] = _transformListGenerateFactory; } StaticInvocation transformStaticInvocation(StaticInvocation invocation) { final target = invocation.target; - final transformer = _transformers[target]; + final transformer = transformers[target]; if (transformer != null) { return transformer(invocation); } diff --git a/pkg/dart2wasm/lib/map_factory_specializer.dart b/pkg/dart2wasm/lib/map_factory_specializer.dart new file mode 100644 index 00000000000..565d0328746 --- /dev/null +++ b/pkg/dart2wasm/lib/map_factory_specializer.dart @@ -0,0 +1,37 @@ +// Copyright (c) 2025, 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 'factory_specializer.dart'; + +/// Replaces invocation of Map factory constructors with factories of +/// Wasm-specific classes. +/// +/// new LinkedHashMap() => new DefaultMap() +class MapFactorySpecializer extends BaseSpecializer { + final Procedure _linkedHashMapDefaultFactory; + final Constructor _internalLinkedHashMapConstructor; + + MapFactorySpecializer(CoreTypes coreTypes) + : _linkedHashMapDefaultFactory = coreTypes.index + .getProcedure('dart:collection', 'LinkedHashMap', ''), + _internalLinkedHashMapConstructor = coreTypes.index + .getConstructor('dart:_compact_hash', 'DefaultMap', '') { + transformers.addAll({_linkedHashMapDefaultFactory: transformLinkedHashMap}); + } + + TreeNode transformLinkedHashMap(StaticInvocation node) { + final args = node.arguments; + if (args.named.isEmpty) { + return ConstructorInvocation( + _internalLinkedHashMapConstructor, + Arguments([], types: args.types), + )..fileOffset = node.fileOffset; + } + + return node; + } +} diff --git a/pkg/dart2wasm/lib/set_factory_specializer.dart b/pkg/dart2wasm/lib/set_factory_specializer.dart new file mode 100644 index 00000000000..caba2626906 --- /dev/null +++ b/pkg/dart2wasm/lib/set_factory_specializer.dart @@ -0,0 +1,38 @@ +// Copyright (c) 2025, 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' show CoreTypes; +import 'package:kernel/core_types.dart'; + +import 'factory_specializer.dart'; + +/// Replaces invocation of Set factory constructors with factories of +/// Wasm-specific classes. +/// +/// new LinkedHashSet() => new DefaultSet() +class SetFactorySpecializer extends BaseSpecializer { + final Procedure _linkedHashSetDefaultFactory; + final Constructor _internalLinkedHashSetConstructor; + + SetFactorySpecializer(CoreTypes coreTypes) + : _linkedHashSetDefaultFactory = coreTypes.index + .getProcedure('dart:collection', 'LinkedHashSet', ''), + _internalLinkedHashSetConstructor = coreTypes.index + .getConstructor('dart:_compact_hash', 'DefaultSet', '') { + transformers.addAll({_linkedHashSetDefaultFactory: transformLinkedHashSet}); + } + + TreeNode transformLinkedHashSet(StaticInvocation node) { + final args = node.arguments; + assert(args.positional.isEmpty); + if (args.named.isEmpty) { + return ConstructorInvocation( + _internalLinkedHashSetConstructor, + Arguments([], types: args.types), + ); + } + return node; + } +} diff --git a/pkg/dart2wasm/lib/transformers.dart b/pkg/dart2wasm/lib/transformers.dart index b2a01c74702..60c3285a80c 100644 --- a/pkg/dart2wasm/lib/transformers.dart +++ b/pkg/dart2wasm/lib/transformers.dart @@ -9,7 +9,7 @@ import 'package:kernel/core_types.dart'; import 'package:kernel/type_algebra.dart'; import 'package:kernel/type_environment.dart'; -import 'list_factory_specializer.dart'; +import 'factory_specializer.dart'; import 'util.dart'; void transformLibraries( @@ -63,7 +63,7 @@ class _WasmTransformer extends Transformer { final List<_AsyncStarFrame> _asyncStarFrames = []; bool _enclosingIsAsyncStar = false; - final ListFactorySpecializer _listFactorySpecializer; + final FactorySpecializer _factorySpecializer; final PushPopWasmArrayTransformer _pushPopWasmArrayTransformer; @@ -119,7 +119,7 @@ class _WasmTransformer extends Transformer { .getTopLevelProcedure("dart:_internal", "loadLibrary"), _checkLibraryIsLoaded = coreTypes.index .getTopLevelProcedure("dart:_internal", "checkLibraryIsLoaded"), - _listFactorySpecializer = ListFactorySpecializer(coreTypes), + _factorySpecializer = FactorySpecializer(coreTypes), _pushPopWasmArrayTransformer = PushPopWasmArrayTransformer(coreTypes); @override @@ -738,8 +738,14 @@ class _WasmTransformer extends Transformer { node.target = _trySetStackTrace; } - return _pushPopWasmArrayTransformer.transformStaticInvocation( - _listFactorySpecializer.transformStaticInvocation(node)); + TreeNode transformed = + _pushPopWasmArrayTransformer.transformStaticInvocation(node); + + if (transformed is StaticInvocation) { + transformed = _factorySpecializer.transformStaticInvocation(transformed); + } + + return transformed; } @override diff --git a/pkg/vm/lib/modular/specializer/map_factory_specializer.dart b/pkg/vm/lib/modular/specializer/map_factory_specializer.dart index 3cb5b499599..73ba9a7026a 100644 --- a/pkg/vm/lib/modular/specializer/map_factory_specializer.dart +++ b/pkg/vm/lib/modular/specializer/map_factory_specializer.dart @@ -16,20 +16,20 @@ class MapFactorySpecializer extends BaseSpecializer { final Constructor _internalLinkedHashMapConstructor; MapFactorySpecializer(CoreTypes coreTypes) - : _linkedHashMapDefaultFactory = assertNotNull( - coreTypes.index.getProcedure('dart:collection', 'LinkedHashMap', ''), + : _linkedHashMapDefaultFactory = coreTypes.index.getProcedure( + 'dart:collection', + 'LinkedHashMap', + '', ), - _internalLinkedHashMapConstructor = assertNotNull( - coreTypes.index.getConstructor('dart:_compact_hash', '_Map', ''), + + _internalLinkedHashMapConstructor = coreTypes.index.getConstructor( + 'dart:_compact_hash', + '_Map', + '', ) { transformers.addAll({_linkedHashMapDefaultFactory: transformLinkedHashMap}); } - static T assertNotNull(T t) { - assert(t != null); - return t; - } - TreeNode transformLinkedHashMap(StaticInvocation node) { final args = node.arguments; if (args.named.isEmpty) { diff --git a/pkg/vm/lib/modular/specializer/set_factory_specializer.dart b/pkg/vm/lib/modular/specializer/set_factory_specializer.dart index c9882f811d8..82d4a2d4f13 100644 --- a/pkg/vm/lib/modular/specializer/set_factory_specializer.dart +++ b/pkg/vm/lib/modular/specializer/set_factory_specializer.dart @@ -17,20 +17,20 @@ class SetFactorySpecializer extends BaseSpecializer { final Constructor _internalLinkedHashSetConstructor; SetFactorySpecializer(CoreTypes coreTypes) - : _linkedHashSetDefaultFactory = assertNotNull( - coreTypes.index.getProcedure('dart:collection', 'LinkedHashSet', ''), + : _linkedHashSetDefaultFactory = coreTypes.index.getProcedure( + 'dart:collection', + 'LinkedHashSet', + '', ), - _internalLinkedHashSetConstructor = assertNotNull( - coreTypes.index.getConstructor('dart:_compact_hash', '_Set', ''), + + _internalLinkedHashSetConstructor = coreTypes.index.getConstructor( + 'dart:_compact_hash', + '_Set', + '', ) { transformers.addAll({_linkedHashSetDefaultFactory: transformLinkedHashSet}); } - static T assertNotNull(T t) { - assert(t != null); - return t; - } - TreeNode transformLinkedHashSet(StaticInvocation node) { final args = node.arguments; assert(args.positional.isEmpty); diff --git a/tests/web/wasm/source_map_simple_optimized_test.dart b/tests/web/wasm/source_map_simple_optimized_test.dart index bda94e3b651..f854a2a5b37 100644 --- a/tests/web/wasm/source_map_simple_optimized_test.dart +++ b/tests/web/wasm/source_map_simple_optimized_test.dart @@ -14,7 +14,6 @@ const List<(String?, int?, int?, String?)?> frameDetails = [ ('errors_patch.dart', null, null, '_throwWithCurrentStackTrace'), ('source_map_simple_lib.dart', 16, 3, 'g'), ('source_map_simple_lib.dart', 12, 3, 'f'), - ('source_map_simple_lib.dart', 39, 5, 'testMain'), ]; /*