From 65a4cb9340cc6de941d07efefe2fc7f88523c2b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Tue, 20 Aug 2024 13:57:18 +0000 Subject: [PATCH] [dart2wasm] Use Wasm arrays directly for `_JsonListener` state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improves bytes/second in an internal JSON decoding benchmark from 227,791,937 to 246,298,043 (+8%). The container stack (`_JsonListener.stack`) could hold the pair `(array, array length)`, however since records are also heap allocated, and size of the record would be the same as `GrowableList`, I've reused the `GrowableList` type for the stack elements. To avoid duplicating the code for growing the Wasm arrays in class fields, a new intrinsic `pushWasmArray` implemented as a macro. It's implemented as a kernel transform and it copies the arguments as expressions in the transformed code to grow the Wasm array when it's full. To change as little is possible in one CL (to be able to measure impact), growth strategy for the arrays are kept the same. Change-Id: I2e6ba7704a83e6e6d8ada5d9015e208069737170 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380705 Commit-Queue: Ömer Ağacan Reviewed-by: Martin Kustermann --- .../lib/list_factory_specializer.dart | 12 +- pkg/dart2wasm/lib/transformers.dart | 273 +++++++++++++++++- sdk/lib/_internal/wasm/lib/convert_patch.dart | 88 ++++-- .../_internal/wasm/lib/internal_patch.dart | 18 ++ sdk/lib/_internal/wasm/lib/list.dart | 9 + 5 files changed, 369 insertions(+), 31 deletions(-) diff --git a/pkg/dart2wasm/lib/list_factory_specializer.dart b/pkg/dart2wasm/lib/list_factory_specializer.dart index f66d81e4ab5..b64b096decc 100644 --- a/pkg/dart2wasm/lib/list_factory_specializer.dart +++ b/pkg/dart2wasm/lib/list_factory_specializer.dart @@ -20,8 +20,8 @@ import 'package:kernel/core_types.dart' show CoreTypes; /// List.generate(n, y, growable: false) => _List.generate(n, y) /// ``` class ListFactorySpecializer { - final Map _transformers = - {}; + final Map + _transformers = {}; final Procedure _fixedListEmptyFactory; final Procedure _fixedListFactory; @@ -63,7 +63,7 @@ class ListFactorySpecializer { _transformers[_listGenerateFactory] = _transformListGenerateFactory; } - TreeNode transformStaticInvocation(StaticInvocation invocation) { + StaticInvocation transformStaticInvocation(StaticInvocation invocation) { final target = invocation.target; final transformer = _transformers[target]; if (transformer != null) { @@ -76,7 +76,7 @@ class ListFactorySpecializer { // List.filled(n, x, growable: true) => _GrowableList.filled(n, x) // List.filled(n, null) => _List(n) // List.filled(n, x) => _List.filled(n, x) - TreeNode _transformListFilledFactory(StaticInvocation node) { + StaticInvocation _transformListFilledFactory(StaticInvocation node) { final args = node.arguments; assert(args.positional.length == 2); final length = args.positional[0]; @@ -122,7 +122,7 @@ class ListFactorySpecializer { // List.empty() => _List.empty() // List.empty(growable: false) => _List.empty() // List.empty(growable: true) => _GrowableList.empty() - TreeNode _transformListEmptyFactory(StaticInvocation node) { + StaticInvocation _transformListEmptyFactory(StaticInvocation node) { final args = node.arguments; assert(args.positional.isEmpty); final bool? growable = @@ -146,7 +146,7 @@ class ListFactorySpecializer { // List.generate(n, y) => _GrowableList.generate(n, y) // List.generate(n, y, growable: false) => _List.generate(n, y) - TreeNode _transformListGenerateFactory(StaticInvocation node) { + StaticInvocation _transformListGenerateFactory(StaticInvocation node) { final args = node.arguments; assert(args.positional.length == 2); final length = args.positional[0]; diff --git a/pkg/dart2wasm/lib/transformers.dart b/pkg/dart2wasm/lib/transformers.dart index 0c54b2c2f01..cb4efe41932 100644 --- a/pkg/dart2wasm/lib/transformers.dart +++ b/pkg/dart2wasm/lib/transformers.dart @@ -4,6 +4,7 @@ import 'package:kernel/ast.dart'; import 'package:kernel/class_hierarchy.dart'; +import 'package:kernel/clone.dart'; import 'package:kernel/core_types.dart'; import 'package:kernel/type_algebra.dart'; import 'package:kernel/type_environment.dart'; @@ -59,6 +60,8 @@ class _WasmTransformer extends Transformer { final ListFactorySpecializer _listFactorySpecializer; + final PushPopWasmArrayTransformer _pushPopWasmArrayTransformer; + StaticTypeContext get typeContext => _cachedTypeContext ??= StaticTypeContext(_currentMember!, env); @@ -103,7 +106,8 @@ class _WasmTransformer extends Transformer { .getProcedure('dart:async', 'StreamController', 'set:onListen'), _streamControllerSetOnResume = coreTypes.index .getProcedure('dart:async', 'StreamController', 'set:onResume'), - _listFactorySpecializer = ListFactorySpecializer(coreTypes); + _listFactorySpecializer = ListFactorySpecializer(coreTypes), + _pushPopWasmArrayTransformer = PushPopWasmArrayTransformer(coreTypes); @override defaultMember(Member node) { @@ -714,11 +718,12 @@ class _WasmTransformer extends Transformer { @override TreeNode visitStaticInvocation(StaticInvocation node) { node.transformChildren(this); - return _listFactorySpecializer.transformStaticInvocation(node); + return _pushPopWasmArrayTransformer.transformStaticInvocation( + _listFactorySpecializer.transformStaticInvocation(node)); } @override - visitFunctionTearOff(FunctionTearOff node) { + TreeNode visitFunctionTearOff(FunctionTearOff node) { node.transformChildren(this); return node.receiver; } @@ -737,3 +742,265 @@ class _AsyncStarFrame { _AsyncStarFrame(this.controllerVar, this.pausedVar, this.emittedValueType); } + +/// Converts `pushWasmArray(array, length, elem, nextCapacity)` to: +/// +/// if (array.length == length) { +/// final newArray = WasmArray(nextCapacity); +/// newArray.copy(0, array, 0, length); +/// array = newArray; +/// } +/// array[length] = elem; +/// length += 1; +/// +/// and `popWasmArray(array, length)` to block expression: +/// +/// { +/// length -= 1; +/// final T _value = array[length]; +/// array[length] = null; +/// } => _value +/// +/// This allows unboxing growable list in class fields. +/// +/// `array` and `length` arguments need to be either `VariableGet` or +/// `InstanceGet`. +class PushPopWasmArrayTransformer { + final CoreTypes _coreTypes; + final Procedure _intAdd; + final Procedure _intSubtract; + final InterfaceType _intType; + final Procedure _popWasmArray; + final Procedure _pushWasmArray; + final Class _wasmArrayClass; + final Procedure _wasmArrayCopy; + final Procedure _wasmArrayElementGet; + final Procedure _wasmArrayElementSet; + final Procedure _wasmArrayFactory; + final Member _wasmArrayLength; + + PushPopWasmArrayTransformer(this._coreTypes) + : _intAdd = _coreTypes.index.getProcedure('dart:core', 'num', '+'), + _intSubtract = _coreTypes.index.getProcedure('dart:core', 'num', '-'), + _intType = _coreTypes.intNonNullableRawType, + _popWasmArray = _coreTypes.index + .getTopLevelProcedure('dart:_internal', 'popWasmArray'), + _pushWasmArray = _coreTypes.index + .getTopLevelProcedure('dart:_internal', 'pushWasmArray'), + _wasmArrayClass = _coreTypes.index.getClass('dart:_wasm', 'WasmArray'), + _wasmArrayCopy = + _coreTypes.index.getProcedure('dart:_wasm', 'WasmArrayExt', 'copy'), + _wasmArrayElementGet = + _coreTypes.index.getProcedure('dart:_wasm', 'WasmArrayExt', '[]'), + _wasmArrayElementSet = + _coreTypes.index.getProcedure('dart:_wasm', 'WasmArrayExt', '[]='), + _wasmArrayFactory = + _coreTypes.index.getProcedure('dart:_wasm', 'WasmArray', ''), + _wasmArrayLength = _coreTypes.index + .getProcedure('dart:_wasm', 'WasmArrayRef', 'get:length'); + + Expression transformStaticInvocation(StaticInvocation invocation) { + if (invocation.target == _pushWasmArray) { + return _transformPushWasmArray(invocation); + } else if (invocation.target == _popWasmArray) { + return _transformPopWasmArray(invocation); + } else { + return invocation; + } + } + + Expression _transformPushWasmArray(StaticInvocation invocation) { + final elementType = invocation.arguments.types[0]; + + final positionalArguments = invocation.arguments.positional; + assert(positionalArguments.length == 4); + + final array = positionalArguments[0]; + final length = positionalArguments[1]; + final elem = positionalArguments[2]; + final nextCapacity = positionalArguments[3]; + + assert(array is InstanceGet || array is VariableGet); + assert(length is InstanceGet || length is VariableGet); + + // Collect variables referenced in `VariableGet`s. These will be passed to + // the cloner as "already cloned" to avoid cloning them. + final variableCollector = _VariableCollector(); + array.accept(variableCollector); + length.accept(variableCollector); + elem.accept(variableCollector); + nextCapacity.accept(variableCollector); + + final variables = variableCollector.variables; + + // Clone an expression. + Expression clone(Expression node) { + final cloner = CloneVisitorNotMembers(); + for (final variable in variables) { + cloner.setVariableClone(variable, variable); + } + return cloner.clone(node); + } + + // array.length == length + final objectEqualsType = _procedureType(_coreTypes.objectEquals); + final lengthCheck = EqualsCall( + InstanceGet(InstanceAccessKind.Instance, array, Name('length'), + interfaceTarget: _wasmArrayLength, resultType: _intType), + length, + functionType: objectEqualsType, + interfaceTarget: _coreTypes.objectEquals); + + // WasmArray(nextCapacity) + final arrayAllocation = StaticInvocation( + _wasmArrayFactory, Arguments([nextCapacity], types: [elementType])); + + // var newArray = WasmArray(nextCapacity) + final newArrayVariable = VariableDeclaration('newArray', + initializer: arrayAllocation, + type: InterfaceType( + _wasmArrayClass, Nullability.nonNullable, [elementType])); + + // newArray.copy(...) + final newArrayCopy = StaticInvocation( + _wasmArrayCopy, + Arguments([ + VariableGet(newArrayVariable), + IntLiteral(0), + clone(array), + IntLiteral(0), + clone(length), + ], types: [ + elementType + ])); + + // array = newArray + final Statement arrayFieldUpdate; + if (array is InstanceGet) { + arrayFieldUpdate = ExpressionStatement(InstanceSet(array.kind, + clone(array.receiver), array.name, VariableGet(newArrayVariable), + interfaceTarget: array.interfaceTarget)); + } else { + final arrayVariableGet = array as VariableGet; + arrayFieldUpdate = ExpressionStatement(VariableSet( + arrayVariableGet.variable, VariableGet(newArrayVariable))); + } + + final List arrayGrowStatements = [ + newArrayVariable, + ExpressionStatement(newArrayCopy), + arrayFieldUpdate + ]; + + // array[length] = elem + final arrayPush = ExpressionStatement(StaticInvocation(_wasmArrayElementSet, + Arguments([clone(array), clone(length), elem], types: [elementType]))); + + // length + 1 + final intAddType = _procedureType(_intAdd); + final lengthPlusOne = InstanceInvocation(InstanceAccessKind.Instance, + clone(length), Name('+'), Arguments([IntLiteral(1)]), + interfaceTarget: _intAdd, functionType: intAddType); + + // length = length + 1 + final Statement arrayLengthUpdate; + if (length is InstanceGet) { + arrayLengthUpdate = ExpressionStatement(InstanceSet( + length.kind, clone(length.receiver), length.name, lengthPlusOne, + interfaceTarget: length.interfaceTarget)); + } else { + final lengthVariableGet = length as VariableGet; + arrayLengthUpdate = ExpressionStatement( + VariableSet(lengthVariableGet.variable, lengthPlusOne)); + } + + return BlockExpression( + Block([ + IfStatement(lengthCheck, Block(arrayGrowStatements), null), + arrayPush, + arrayLengthUpdate + ]), + NullLiteral()); + } + + Expression _transformPopWasmArray(StaticInvocation invocation) { + final elementType = invocation.arguments.types[0] as InterfaceType; + final elementTypeNullable = + elementType.withDeclaredNullability(Nullability.nullable); + + final positionalArguments = invocation.arguments.positional; + assert(positionalArguments.length == 4); + + final array = positionalArguments[0]; + final length = positionalArguments[1]; + + assert(array is InstanceGet || array is VariableGet); + assert(length is InstanceGet || length is VariableGet); + + // Collect variables referenced in `VariableGet`s. These will be passed to + // the cloner as "already cloned" to avoid cloning them. + final variableCollector = _VariableCollector(); + array.accept(variableCollector); + length.accept(variableCollector); + + final variables = variableCollector.variables; + + // Clone an expression. + Expression clone(Expression node) { + final cloner = CloneVisitorNotMembers(); + for (final variable in variables) { + cloner.setVariableClone(variable, variable); + } + return cloner.clone(node); + } + + // length - 1 + final intSubtractType = _procedureType(_intSubtract); + final lengthMinusOne = InstanceInvocation(InstanceAccessKind.Instance, + clone(length), Name('-'), Arguments([IntLiteral(1)]), + interfaceTarget: _intSubtract, functionType: intSubtractType); + + // length -= 1 + final Statement arrayLengthUpdate; + if (length is InstanceGet) { + arrayLengthUpdate = ExpressionStatement(InstanceSet( + length.kind, clone(length.receiver), length.name, lengthMinusOne, + interfaceTarget: length.interfaceTarget)); + } else { + final lengthVariableGet = length as VariableGet; + arrayLengthUpdate = ExpressionStatement( + VariableSet(lengthVariableGet.variable, lengthMinusOne)); + } + + // array[length] + final arrayGet = StaticInvocation(_wasmArrayElementGet, + Arguments([clone(array), clone(length)], types: [elementTypeNullable])); + + // final temp = array[length] + final arrayGetVariable = VariableDeclaration.forValue(arrayGet, + isFinal: true, type: elementTypeNullable); + + // array[length] = null + final arrayClearElement = ExpressionStatement(StaticInvocation( + _wasmArrayElementSet, + Arguments([clone(array), clone(length), NullLiteral()], + types: [elementTypeNullable]))); + + return BlockExpression( + Block([arrayLengthUpdate, arrayGetVariable, arrayClearElement]), + VariableGet(arrayGetVariable)); + } + + static FunctionType _procedureType(Procedure procedure) => + procedure.signatureType ?? + procedure.function.computeFunctionType(Nullability.nonNullable); +} + +class _VariableCollector extends RecursiveVisitor { + Set variables = {}; + + @override + void visitVariableGet(VariableGet node) { + variables.add(node.variable); + } +} diff --git a/sdk/lib/_internal/wasm/lib/convert_patch.dart b/sdk/lib/_internal/wasm/lib/convert_patch.dart index 536af4629bc..e88c0bf0161 100644 --- a/sdk/lib/_internal/wasm/lib/convert_patch.dart +++ b/sdk/lib/_internal/wasm/lib/convert_patch.dart @@ -3,11 +3,12 @@ // BSD-style license that can be found in the LICENSE file. import "dart:_compact_hash" show createMapFromKeyValueListUnsafe; -import "dart:_internal" show patch, POWERS_OF_TEN, unsafeCast; +import "dart:_internal" + show patch, POWERS_OF_TEN, unsafeCast, pushWasmArray, popWasmArray; import "dart:_js_string_convert"; import "dart:_js_types"; import "dart:_js_helper" show jsStringToDartString; -import "dart:_list" show GrowableList; +import "dart:_list" show GrowableList, GrowableListUnsafeExtensions; import "dart:_string"; import "dart:_typed_data"; import "dart:_wasm"; @@ -80,31 +81,74 @@ class _JsonListener { /** * Stack used to handle nested containers. * - * The current container is pushed on the stack when a new one is - * started. + * The current container is pushed on the stack when a new one is started. */ - final List stack = []; + WasmArray stack = WasmArray(0); + int stackLength = 0; + + void stackPush(WasmArray? value, int valueLength) { + final GrowableList? valueAsList = value == null + ? null + : GrowableList.withDataAndLength(value, valueLength); + + // `GrowableList._nextCapacity` is copied here as the next capacity. We + // can't use `GrowableList._nextCapacity` as tear-off as it's difficult to + // inline tear-offs manually in the `pushWasmArray` compiler. + pushWasmArray?>( + this.stack, this.stackLength, valueAsList, (stackLength * 2) | 3); + } + + GrowableList? stackPop() { + assert(stackLength != 0); + return popWasmArray>(stack, stackLength); + } /** Contents of the current container being built, or null if not building a - * container. - * - * When building [Map] this will contain array of key-value pairs. - */ - GrowableList? currentContainer; + * container. + * + * When building a [Map] this will contain array of key-value pairs. + */ + WasmArray? currentContainer = null; + int currentContainerLength = 0; + + void currentContainerPush(Object? value) { + WasmArray currentContainerNonNull = + unsafeCast>(this.currentContainer); + // Same as above, this copies `GrowableList._nextCapacity` as the next + // capacity. + pushWasmArray(currentContainerNonNull, this.currentContainerLength, + value, (currentContainerLength * 2) | 3); + currentContainer = currentContainerNonNull; + } /** The most recently read value. */ Object? value; /** Pushes the currently active container. */ void beginContainer() { - stack.add(currentContainer); - currentContainer = GrowableList.empty(); + stackPush(currentContainer, currentContainerLength); + currentContainer = const WasmArray.literal([]); + currentContainerLength = 0; } /** Pops the top container from the [stack]. */ void popContainer() { - value = currentContainer; - currentContainer = unsafeCast(stack.removeLast()); + final currentContainerLocal = currentContainer; + if (currentContainerLocal == null) { + value = null; + } else { + value = GrowableList.withDataAndLength( + currentContainerLocal, currentContainerLength); + } + + final GrowableList? currentContainerList = stackPop(); + if (currentContainerList == null) { + currentContainer = null; + currentContainerLength = 0; + } else { + currentContainer = currentContainerList.data; + currentContainerLength = currentContainerList.length; + } } void handleString(String value) { @@ -128,17 +172,18 @@ class _JsonListener { } void propertyName() { - unsafeCast(currentContainer).add(value); + currentContainerPush(value); value = null; } void propertyValue() { - final keyValuePairs = unsafeCast(currentContainer); if (reviver case final reviver?) { - final key = keyValuePairs.last; - keyValuePairs.add(reviver(key, value)); + final keyValuePairs = + unsafeCast>(currentContainer); // null deref + final key = keyValuePairs[currentContainerLength - 1]; + currentContainerPush(reviver(key, value)); } else { - keyValuePairs.add(value); + currentContainerPush(value); } value = null; } @@ -154,12 +199,11 @@ class _JsonListener { } void arrayElement() { - var list = unsafeCast(currentContainer); var reviver = this.reviver; if (reviver != null) { - value = reviver(list.length, value); + value = reviver(currentContainerLength, value); } - list.add(value); + currentContainerPush(value); value = null; } diff --git a/sdk/lib/_internal/wasm/lib/internal_patch.dart b/sdk/lib/_internal/wasm/lib/internal_patch.dart index 624cca4d864..b567999c446 100644 --- a/sdk/lib/_internal/wasm/lib/internal_patch.dart +++ b/sdk/lib/_internal/wasm/lib/internal_patch.dart @@ -188,3 +188,21 @@ void indexCheckWithName(int index, int length, String name) { @patch Future loadDynamicModule({Uri? uri, Uint8List? bytes}) => throw 'Unsupported operation'; + +/// Compiler intrinsic to push an element to a Wasm array in a class field or +/// variable. +/// +/// The `array` and `length` arguments need to be `InstanceGet`s (e.g. `this.x`) +/// or `VariableGet`s (e.g. `x`). This function will update the class field +/// (when the argument is `InstanceGet`) or the variable (when the argument is +/// `InstanceGet`). +/// +/// `elem` is the element to be pushed onto the array and can have any shape. +/// +/// `nextCapacity` is the capacity to be used when growing the array. It can +/// have any shape, and it will be evaluated only when the array is full. +external void pushWasmArray( + WasmArray array, int length, T elem, int nextCapacity); + +/// Similar to `pushWasmArray`, but for popping. +external T? popWasmArray(WasmArray array, int length); diff --git a/sdk/lib/_internal/wasm/lib/list.dart b/sdk/lib/_internal/wasm/lib/list.dart index 53ffaf91b6e..acb7744fd8f 100644 --- a/sdk/lib/_internal/wasm/lib/list.dart +++ b/sdk/lib/_internal/wasm/lib/list.dart @@ -240,6 +240,10 @@ class GrowableList extends _ModifiableList { GrowableList._withData(WasmArray data) : super._withData(data.length, data); + @pragma("wasm:prefer-inline") + GrowableList.withDataAndLength(WasmArray data, int length) + : super._withData(length, data); + @pragma("wasm:prefer-inline") factory GrowableList(int length) => GrowableList._(length, length); @@ -545,3 +549,8 @@ class _GrowableListIterator implements Iterator { return true; } } + +extension GrowableListUnsafeExtensions on GrowableList { + @pragma('wasm:prefer-inline') + WasmArray get data => _data; +}