From d14e62c48ea7ebe12abb66c5c2628ef97464ebbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20A=C4=9Facan?= Date: Wed, 7 May 2025 10:28:42 +0100 Subject: [PATCH 1/3] Fix packed enum decoding benchmark While investigating why decoding packed enums is so much slower in AOT compared to decoding packed int32 (both are varints on the wire) I noticed that the enum decoding benchmark should actually be slower, because currently TFA is able to specialize the enum int value to Dart enum value mapping to a direct call, in this function: ``` ProtobufEnum? _decodeEnum( int tagNumber, ExtensionRegistry? registry, int rawValue) { final f = valueOfFunc(tagNumber); if (f != null) { return f(rawValue); // <------------------- HERE } ... } ``` Wasm code for this function, before this PR: ``` (func $BuilderInfo._decodeEnum (;641;) (param $var0 (ref $BuilderInfo_214)) (param $var1 i64) (param $var2 i64) (result (ref null $Enum)) (local $var3 (ref null $FieldInfo_223)) local.get $var0 struct.get $BuilderInfo_214 $field4 i32.const 71 local.get $var1 struct.new $BoxedInt call $_DefaultMap&_HashFieldBase&MapMixin&_HashBase&_OperatorEqualsAndHashCode&_LinkedHashMapMixin.[] ref.cast null $FieldInfo_223 local.tee $var3 ref.is_null if (result (ref null $#Closure-0-1_815)) ref.null none else local.get $var3 struct.get $FieldInfo_223 $field9 end ref.is_null i32.eqz if local.get $var2 call $Enum.valueOf return end ref.null none ) ``` Note that this calls `$Enum.valueOf` even though this function is generic on the enum type. With this PR we add another enum to the proto file and decode it in setup, so that TFA is unable to specialize `_deocdeEnum` to one specific enum type. New code: ``` (func $BuilderInfo._decodeEnum (;643;) (param $var0 (ref $BuilderInfo_214)) (param $var1 i64) (param $var2 i64) (result (ref null $ProtobufEnum)) (local $var3 (ref null $FieldInfo_229)) (local $var4 (ref null $#Closure-0-1)) (local $var5 (ref $#Closure-0-1)) local.get $var0 struct.get $BuilderInfo_214 $field4 i32.const 71 local.get $var1 struct.new $BoxedInt call $_DefaultMap&_HashFieldBase&MapMixin&_HashBase&_OperatorEqualsAndHashCode&_LinkedHashMapMixin.[] ref.cast null $FieldInfo_229 local.tee $var3 ref.is_null if (result (ref null $#Closure-0-1)) ref.null none else local.get $var3 struct.get $FieldInfo_229 $field9 end local.tee $var4 ref.is_null i32.eqz if local.get $var4 ref.as_non_null local.tee $var5 struct.get $#Closure-0-1 $field2 i32.const 71 local.get $var2 struct.new $BoxedInt local.get $var5 struct.get $#Closure-0-1 $field3 struct.get $#Vtable-0-1 $field1 call_ref $type39 ref.cast null $ProtobufEnum return end ref.null none ) ``` Wasm benchmark results: ``` // Before protobuf_PackedEnumDecoding(RunTimeRaw): 41120.0 us. // After protobuf_PackedEnumDecoding(RunTimeRaw): 52750.0 us. ``` VM benchmark results: ``` // Before protobuf_PackedEnumDecoding(RunTimeRaw): 45051.520000000004 us. // After protobuf_PackedEnumDecoding(RunTimeRaw): 54661.125 us. ``` --- benchmarks/bin/binary_decode_packed.dart | 26 ++++++++++++++++++++++-- benchmarks/protos/packed_fields.proto | 23 ++++++++++++++------- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/benchmarks/bin/binary_decode_packed.dart b/benchmarks/bin/binary_decode_packed.dart index 5e4d3e35..7cd48b13 100644 --- a/benchmarks/bin/binary_decode_packed.dart +++ b/benchmarks/bin/binary_decode_packed.dart @@ -146,13 +146,35 @@ class PackedEnumDecodingBenchmark extends BenchmarkBase { PackedEnumDecodingBenchmark() : super('PackedEnumDecoding') { final rand = Random(123); final message = PackedFields(); - final numEnums = Enum.values.length; + final numEnums = Enum1.values.length; for (var i = 0; i < 1000000; i += 1) { - message.packedEnum.add(Enum.values[rand.nextInt(numEnums)]); + message.packedEnum1.add(Enum1.values[rand.nextInt(numEnums)]); } encoded = message.writeToBuffer(); } + @override + void setup() { + // Decode different enums to prevent TFA from specializing enum decoding + // code to one type. + final rand = Random(123); + final message = PackedFields(); + for (var i = 0; i < 100; i += 1) { + message.packedEnum1.add(Enum1.values[rand.nextInt(Enum1.values.length)]); + } + for (var i = 0; i < 100; i += 1) { + message.packedEnum2.add(Enum2.values[rand.nextInt(Enum2.values.length)]); + } + final encoded = message.writeToBuffer(); + final decoded = PackedFields()..mergeFromBuffer(encoded); + if (decoded.packedEnum1.length != 100) { + throw "BUG"; + } + if (decoded.packedEnum2.length != 100) { + throw "BUG"; + } + } + @override void run() { sink = PackedFields()..mergeFromBuffer(encoded); diff --git a/benchmarks/protos/packed_fields.proto b/benchmarks/protos/packed_fields.proto index 2e4f9451..09d32d3b 100644 --- a/benchmarks/protos/packed_fields.proto +++ b/benchmarks/protos/packed_fields.proto @@ -8,13 +8,22 @@ message PackedFields { repeated sint32 packedSint32 = 5 [packed = true]; repeated sint64 packedSint64 = 6 [packed = true]; repeated bool packedBool = 7 [packed = true]; - repeated Enum packedEnum = 8 [packed = true]; + repeated Enum1 packedEnum1 = 8 [packed = true]; + repeated Enum2 packedEnum2 = 9 [packed = true]; } -enum Enum { - ENUM_1 = 0; - ENUM_2 = 1; - ENUM_3 = 2; - ENUM_4 = 4; - ENUM_5 = 5; +enum Enum1 { + ENUM_1_1 = 0; + ENUM_1_2 = 1; + ENUM_1_3 = 2; + ENUM_1_4 = 4; + ENUM_1_5 = 5; +} + +enum Enum2 { + ENUM_2_1 = 0; + ENUM_2_2 = 1; + ENUM_2_3 = 2; + ENUM_2_4 = 4; + ENUM_2_5 = 5; } From e52ed5dadcf2f5075e1241595291c04b7b785ed4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20A=C4=9Facan?= Date: Wed, 7 May 2025 10:36:59 +0100 Subject: [PATCH 2/3] Fix warn --- benchmarks/bin/binary_decode_packed.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/benchmarks/bin/binary_decode_packed.dart b/benchmarks/bin/binary_decode_packed.dart index 7cd48b13..f1f3ead6 100644 --- a/benchmarks/bin/binary_decode_packed.dart +++ b/benchmarks/bin/binary_decode_packed.dart @@ -168,10 +168,10 @@ class PackedEnumDecodingBenchmark extends BenchmarkBase { final encoded = message.writeToBuffer(); final decoded = PackedFields()..mergeFromBuffer(encoded); if (decoded.packedEnum1.length != 100) { - throw "BUG"; + throw AssertionError("BUG"); } if (decoded.packedEnum2.length != 100) { - throw "BUG"; + throw AssertionError("BUG"); } } From f7cb25d15d7f58ce033e943eff2011081399bec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20A=C4=9Facan?= Date: Wed, 7 May 2025 10:38:06 +0100 Subject: [PATCH 3/3] Fix warn --- benchmarks/bin/binary_decode_packed.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/benchmarks/bin/binary_decode_packed.dart b/benchmarks/bin/binary_decode_packed.dart index f1f3ead6..f640cca4 100644 --- a/benchmarks/bin/binary_decode_packed.dart +++ b/benchmarks/bin/binary_decode_packed.dart @@ -168,10 +168,10 @@ class PackedEnumDecodingBenchmark extends BenchmarkBase { final encoded = message.writeToBuffer(); final decoded = PackedFields()..mergeFromBuffer(encoded); if (decoded.packedEnum1.length != 100) { - throw AssertionError("BUG"); + throw AssertionError('BUG'); } if (decoded.packedEnum2.length != 100) { - throw AssertionError("BUG"); + throw AssertionError('BUG'); } }