Skip to content

Commit a6c8e56

Browse files
osa1osa1
authored
Improve enum decoding -- alternative (#981)
When decoding an enum value, we call a callback in the enum field's `FieldInfo`. The callback then indexes a map mapping enum numbers to Dart values. When these conditions hold: - The known enum numbers are all positive. (so that we can use a list and index it with the number) - The known enum numbers are more than 70% of the large known enum number. (so that the list won't have a lot of `null` entries, wasting heap space) We now generate a list instead of a map to map enum numbers to Dart values. Similar to the map, the list is runtime allocated. No new code generated per message or enum type. AOT benchmarks: - Before: `protobuf_PackedEnumDecoding(RunTimeRaw): 47585.14 us.` - After: `protobuf_PackedEnumDecoding(RunTimeRaw): 38974.566666666666 us.` - Diff: -18% Wasm benchmarks: - Before: `protobuf_PackedEnumDecoding(RunTimeRaw): 52225.0 us.` - After: `protobuf_PackedEnumDecoding(RunTimeRaw): 34283.33333333333 us.` - Diff: -34% **Alternatives considered:** - #980 uses a map always, but eliminates the `valueOf` closure. - #985 uses a list always, and does binary search in the list when the list is "shallow". - #987 is the same as #985, but instead of calling the `valueOf` closure it stores an extra field in `FieldInfo`s for whether to binary search or directly index. These are all slower than the current PR.
1 parent 376c9ce commit a6c8e56

File tree

12 files changed

+171
-42
lines changed

12 files changed

+171
-42
lines changed

‎protobuf/lib/src/protobuf/protobuf_enum.dart

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
// ignore_for_file: non_constant_identifier_names
6+
57
part of '../../protobuf.dart';
68

79
/// A base class for all proto enum types.
@@ -37,12 +39,28 @@ class ProtobufEnum {
3739
/// Creates a new constant [ProtobufEnum] using [value] and [name].
3840
const ProtobufEnum(this.value, this.name);
3941

40-
/// Creates a Map for all of the [ProtobufEnum]s in [byIndex], mapping each
42+
/// This function is for generated code.
43+
///
44+
/// Creates a Map for all of the [ProtobufEnum]s in [enumValues], mapping each
4145
/// [ProtobufEnum]'s [value] to the [ProtobufEnum].
42-
static Map<int, T> initByValue<T extends ProtobufEnum>(List<T> byIndex) {
46+
///
47+
/// @nodoc
48+
static Map<int, T> initByValue<T extends ProtobufEnum>(List<T> enumValues) {
4349
final byValue = <int, T>{};
44-
for (final v in byIndex) {
45-
byValue[v.value] = v;
50+
for (final enumValue in enumValues) {
51+
byValue[enumValue.value] = enumValue;
52+
}
53+
return byValue;
54+
}
55+
56+
/// This function is for generated code.
57+
///
58+
/// @nodoc
59+
static List<T?> $_initByValueList<T extends ProtobufEnum>(
60+
List<T> enumValues, int maxEnumValue) {
61+
final byValue = List<T?>.filled(maxEnumValue + 1, null);
62+
for (final enumValue in enumValues) {
63+
byValue[enumValue.value] = enumValue;
4664
}
4765
return byValue;
4866
}

‎protoc_plugin/Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ TEST_PROTO_LIST = \
3030
entity \
3131
enum_extension \
3232
enum_name \
33+
enums \
3334
extend_unittest \
3435
ExtensionEnumNameConflict \
3536
ExtensionNameConflict \

‎protoc_plugin/lib/src/enum_generator.dart

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,37 @@ class EnumGenerator extends ProtobufContainer {
175175
out.println('];');
176176
out.println();
177177

178-
out.println(
179-
'static final $coreImportPrefix.Map<$coreImportPrefix.int, $classname> _byValue ='
180-
' $protobufImportPrefix.ProtobufEnum.initByValue(values);');
181-
out.println('static $classname? valueOf($coreImportPrefix.int value) =>'
182-
' _byValue[value];');
178+
var maxEnumValue = -1;
179+
for (final valueDescriptor in _canonicalValues) {
180+
if (valueDescriptor.number.isNegative) {
181+
maxEnumValue = -1; // don't use list
182+
break;
183+
}
184+
if (valueDescriptor.number > maxEnumValue) {
185+
maxEnumValue = valueDescriptor.number;
186+
}
187+
}
188+
189+
final useList = _canonicalValues.isEmpty ||
190+
(maxEnumValue >= 0 &&
191+
_canonicalValues.length / (maxEnumValue + 1) >= 0.7);
192+
193+
if (useList) {
194+
out.println(
195+
'static final $coreImportPrefix.List<$classname?> _byValue ='
196+
' $protobufImportPrefix.ProtobufEnum.\$_initByValueList(values, $maxEnumValue);');
197+
198+
out.println('static $classname? valueOf($coreImportPrefix.int value) =>'
199+
' value < 0 || value >= _byValue.length ? null : _byValue[value];');
200+
} else {
201+
out.println(
202+
'static final $coreImportPrefix.Map<$coreImportPrefix.int, $classname> _byValue ='
203+
' $protobufImportPrefix.ProtobufEnum.initByValue(values);');
204+
205+
out.println('static $classname? valueOf($coreImportPrefix.int value) =>'
206+
' _byValue[value];');
207+
}
208+
183209
out.println();
184210

185211
out.println('const $classname._(super.v, super.n);');

‎protoc_plugin/lib/src/generated/descriptor.pbenum.dart

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,10 @@ class FieldDescriptorProto_Type extends $pb.ProtobufEnum {
8888
TYPE_SINT64,
8989
];
9090

91-
static final $core.Map<$core.int, FieldDescriptorProto_Type> _byValue =
92-
$pb.ProtobufEnum.initByValue(values);
93-
static FieldDescriptorProto_Type? valueOf($core.int value) => _byValue[value];
91+
static final $core.List<FieldDescriptorProto_Type?> _byValue =
92+
$pb.ProtobufEnum.$_initByValueList(values, 18);
93+
static FieldDescriptorProto_Type? valueOf($core.int value) =>
94+
value < 0 || value >= _byValue.length ? null : _byValue[value];
9495

9596
const FieldDescriptorProto_Type._(super.v, super.n);
9697
}
@@ -111,10 +112,10 @@ class FieldDescriptorProto_Label extends $pb.ProtobufEnum {
111112
LABEL_REPEATED,
112113
];
113114

114-
static final $core.Map<$core.int, FieldDescriptorProto_Label> _byValue =
115-
$pb.ProtobufEnum.initByValue(values);
115+
static final $core.List<FieldDescriptorProto_Label?> _byValue =
116+
$pb.ProtobufEnum.$_initByValueList(values, 3);
116117
static FieldDescriptorProto_Label? valueOf($core.int value) =>
117-
_byValue[value];
118+
value < 0 || value >= _byValue.length ? null : _byValue[value];
118119

119120
const FieldDescriptorProto_Label._(super.v, super.n);
120121
}
@@ -137,9 +138,10 @@ class FileOptions_OptimizeMode extends $pb.ProtobufEnum {
137138
LITE_RUNTIME,
138139
];
139140

140-
static final $core.Map<$core.int, FileOptions_OptimizeMode> _byValue =
141-
$pb.ProtobufEnum.initByValue(values);
142-
static FileOptions_OptimizeMode? valueOf($core.int value) => _byValue[value];
141+
static final $core.List<FileOptions_OptimizeMode?> _byValue =
142+
$pb.ProtobufEnum.$_initByValueList(values, 3);
143+
static FileOptions_OptimizeMode? valueOf($core.int value) =>
144+
value < 0 || value >= _byValue.length ? null : _byValue[value];
143145

144146
const FileOptions_OptimizeMode._(super.v, super.n);
145147
}
@@ -159,9 +161,10 @@ class FieldOptions_CType extends $pb.ProtobufEnum {
159161
STRING_PIECE,
160162
];
161163

162-
static final $core.Map<$core.int, FieldOptions_CType> _byValue =
163-
$pb.ProtobufEnum.initByValue(values);
164-
static FieldOptions_CType? valueOf($core.int value) => _byValue[value];
164+
static final $core.List<FieldOptions_CType?> _byValue =
165+
$pb.ProtobufEnum.$_initByValueList(values, 2);
166+
static FieldOptions_CType? valueOf($core.int value) =>
167+
value < 0 || value >= _byValue.length ? null : _byValue[value];
165168

166169
const FieldOptions_CType._(super.v, super.n);
167170
}
@@ -185,9 +188,10 @@ class FieldOptions_JSType extends $pb.ProtobufEnum {
185188
JS_NUMBER,
186189
];
187190

188-
static final $core.Map<$core.int, FieldOptions_JSType> _byValue =
189-
$pb.ProtobufEnum.initByValue(values);
190-
static FieldOptions_JSType? valueOf($core.int value) => _byValue[value];
191+
static final $core.List<FieldOptions_JSType?> _byValue =
192+
$pb.ProtobufEnum.$_initByValueList(values, 2);
193+
static FieldOptions_JSType? valueOf($core.int value) =>
194+
value < 0 || value >= _byValue.length ? null : _byValue[value];
191195

192196
const FieldOptions_JSType._(super.v, super.n);
193197
}
@@ -212,10 +216,10 @@ class MethodOptions_IdempotencyLevel extends $pb.ProtobufEnum {
212216
IDEMPOTENT,
213217
];
214218

215-
static final $core.Map<$core.int, MethodOptions_IdempotencyLevel> _byValue =
216-
$pb.ProtobufEnum.initByValue(values);
219+
static final $core.List<MethodOptions_IdempotencyLevel?> _byValue =
220+
$pb.ProtobufEnum.$_initByValueList(values, 2);
217221
static MethodOptions_IdempotencyLevel? valueOf($core.int value) =>
218-
_byValue[value];
222+
value < 0 || value >= _byValue.length ? null : _byValue[value];
219223

220224
const MethodOptions_IdempotencyLevel._(super.v, super.n);
221225
}

‎protoc_plugin/lib/src/generated/plugin.pbenum.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ class CodeGeneratorResponse_Feature extends $pb.ProtobufEnum {
2727
FEATURE_PROTO3_OPTIONAL,
2828
];
2929

30-
static final $core.Map<$core.int, CodeGeneratorResponse_Feature> _byValue =
31-
$pb.ProtobufEnum.initByValue(values);
30+
static final $core.List<CodeGeneratorResponse_Feature?> _byValue =
31+
$pb.ProtobufEnum.$_initByValueList(values, 1);
3232
static CodeGeneratorResponse_Feature? valueOf($core.int value) =>
33-
_byValue[value];
33+
value < 0 || value >= _byValue.length ? null : _byValue[value];
3434

3535
const CodeGeneratorResponse_Feature._(super.v, super.n);
3636
}

‎protoc_plugin/test/generated_message_test.dart

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import '../out/protos/constructor_args/google/protobuf/unittest.pb.dart'
1010
import '../out/protos/constructor_args/google/protobuf/unittest_import.pb.dart'
1111
as constructor_args_unittest_import;
1212
import '../out/protos/duplicate_names_import.pb.dart';
13+
import '../out/protos/enums.pb.dart';
1314
import '../out/protos/google/protobuf/unittest.pb.dart';
1415
import '../out/protos/google/protobuf/unittest_import.pb.dart';
1516
import '../out/protos/google/protobuf/unittest_optimize_for.pb.dart';
@@ -890,4 +891,27 @@ void main() {
890891
// constructor arguments, to be able to reuse `assertAllFieldsSet`.
891892
assertAllFieldsSet(TestAllTypes.fromBuffer(value.writeToBuffer()));
892893
});
894+
895+
test('Handling enums defined out of order', () {
896+
final message = MessageWithEnums();
897+
for (final enum_ in DenseEnum.values) {
898+
message.denseEnums.add(enum_);
899+
}
900+
for (final enum_ in DenseEnumOutOfOrder.values) {
901+
message.denseOutOfOrderEnums.add(enum_);
902+
}
903+
for (final enum_ in SparseEnum.values) {
904+
message.sparseEnums.add(enum_);
905+
}
906+
for (final enum_ in SparseEnumOutOfOrder.values) {
907+
message.sparseOutOfOrderEnums.add(enum_);
908+
}
909+
910+
final encoded = message.writeToBuffer();
911+
final decoded = MessageWithEnums.fromBuffer(encoded);
912+
expect(decoded.denseEnums, DenseEnum.values);
913+
expect(decoded.denseOutOfOrderEnums, DenseEnumOutOfOrder.values);
914+
expect(decoded.sparseEnums, SparseEnum.values);
915+
expect(decoded.sparseOutOfOrderEnums, SparseEnumOutOfOrder.values);
916+
});
893917
}

‎protoc_plugin/test/goldens/deprecations.pbenum

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@ class A extends $pb.ProtobufEnum {
2424
A2,
2525
];
2626

27-
static final $core.Map<$core.int, A> _byValue =
28-
$pb.ProtobufEnum.initByValue(values);
29-
static A? valueOf($core.int value) => _byValue[value];
27+
static final $core.List<A?> _byValue =
28+
$pb.ProtobufEnum.$_initByValueList(values, 1);
29+
static A? valueOf($core.int value) =>
30+
value < 0 || value >= _byValue.length ? null : _byValue[value];
3031

3132
const A._(super.v, super.n);
3233
}

‎protoc_plugin/test/goldens/doc_comments.pbenum

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@ class A extends $pb.ProtobufEnum {
2626
A2,
2727
];
2828

29-
static final $core.Map<$core.int, A> _byValue =
30-
$pb.ProtobufEnum.initByValue(values);
31-
static A? valueOf($core.int value) => _byValue[value];
29+
static final $core.List<A?> _byValue =
30+
$pb.ProtobufEnum.$_initByValueList(values, 1);
31+
static A? valueOf($core.int value) =>
32+
value < 0 || value >= _byValue.length ? null : _byValue[value];
3233

3334
const A._(super.v, super.n);
3435
}

‎protoc_plugin/test/goldens/enum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ class PhoneType extends $pb.ProtobufEnum {
1111
WORK,
1212
];
1313

14-
static final $core.Map<$core.int, PhoneType> _byValue = $pb.ProtobufEnum.initByValue(values);
15-
static PhoneType? valueOf($core.int value) => _byValue[value];
14+
static final $core.List<PhoneType?> _byValue = $pb.ProtobufEnum.$_initByValueList(values, 2);
15+
static PhoneType? valueOf($core.int value) => value < 0 || value >= _byValue.length ? null : _byValue[value];
1616

1717
const PhoneType._(super.v, super.n);
1818
}

‎protoc_plugin/test/goldens/messageGeneratorEnums

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ class PhoneNumber_PhoneType extends $pb.ProtobufEnum {
1111
WORK,
1212
];
1313

14-
static final $core.Map<$core.int, PhoneNumber_PhoneType> _byValue = $pb.ProtobufEnum.initByValue(values);
15-
static PhoneNumber_PhoneType? valueOf($core.int value) => _byValue[value];
14+
static final $core.List<PhoneNumber_PhoneType?> _byValue = $pb.ProtobufEnum.$_initByValueList(values, 2);
15+
static PhoneNumber_PhoneType? valueOf($core.int value) => value < 0 || value >= _byValue.length ? null : _byValue[value];
1616

1717
const PhoneNumber_PhoneType._(super.v, super.n);
1818
}

‎protoc_plugin/test/goldens/topLevelEnum.pbenum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ class PhoneType extends $pb.ProtobufEnum {
2626
WORK,
2727
];
2828

29-
static final $core.Map<$core.int, PhoneType> _byValue = $pb.ProtobufEnum.initByValue(values);
30-
static PhoneType? valueOf($core.int value) => _byValue[value];
29+
static final $core.List<PhoneType?> _byValue = $pb.ProtobufEnum.$_initByValueList(values, 2);
30+
static PhoneType? valueOf($core.int value) => value < 0 || value >= _byValue.length ? null : _byValue[value];
3131

3232
const PhoneType._(super.v, super.n);
3333
}

‎protoc_plugin/test/protos/enums.proto

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
enum DenseEnum {
2+
DENSE_ENUM_0 = 0;
3+
DENSE_ENUM_1 = 1;
4+
DENSE_ENUM_2 = 2;
5+
DENSE_ENUM_3 = 3;
6+
DENSE_ENUM_4 = 4;
7+
}
8+
9+
enum DenseEnumOutOfOrder {
10+
DENSE_ENUM_OOO_0 = 0;
11+
DENSE_ENUM_OOO_2 = 2;
12+
DENSE_ENUM_OOO_4 = 4;
13+
DENSE_ENUM_OOO_3 = 3;
14+
DENSE_ENUM_OOO_1 = 1;
15+
}
16+
17+
enum SparseEnum {
18+
SPARSE_ENUM_ZERO = 0;
19+
SPARSE_ENUM_MIN_INT = -2147483648;
20+
SPARSE_ENUM_1 = -1000000000;
21+
SPARSE_ENUM_2 = -100000000;
22+
SPARSE_ENUM_3 = -10000000;
23+
SPARSE_ENUM_4 = -1000000;
24+
SPARSE_ENUM_5 = -100000;
25+
SPARSE_ENUM_6 = 100000;
26+
SPARSE_ENUM_7 = 1000000;
27+
SPARSE_ENUM_8 = 10000000;
28+
SPARSE_ENUM_9 = 100000000;
29+
SPARSE_ENUM_10 = 1000000000;
30+
SPARSE_ENUM_MAX_INT = 2147483647;
31+
}
32+
33+
enum SparseEnumOutOfOrder {
34+
SPARSE_ENUM_OOO_ZERO = 0;
35+
SPARSE_ENUM_OOO_1 = -1000000000;
36+
SPARSE_ENUM_OOO_MAX_INT = 2147483647;
37+
SPARSE_ENUM_OOO_4 = -1000000;
38+
SPARSE_ENUM_OOO_6 = 100000;
39+
SPARSE_ENUM_OOO_MIN_INT = -2147483648;
40+
SPARSE_ENUM_OOO_7 = 1000000;
41+
SPARSE_ENUM_OOO_5 = -100000;
42+
SPARSE_ENUM_OOO_8 = 10000000;
43+
SPARSE_ENUM_OOO_3 = -10000000;
44+
SPARSE_ENUM_OOO_10 = 1000000000;
45+
SPARSE_ENUM_OOO_2 = -100000000;
46+
SPARSE_ENUM_OOO_9 = 100000000;
47+
}
48+
49+
message MessageWithEnums {
50+
repeated DenseEnum denseEnums = 1;
51+
repeated DenseEnumOutOfOrder denseOutOfOrderEnums = 2;
52+
repeated SparseEnum sparseEnums = 3;
53+
repeated SparseEnumOutOfOrder sparseOutOfOrderEnums = 4;
54+
}

0 commit comments

Comments
 (0)