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.
diff --git a/protobuf/lib/src/protobuf/protobuf_enum.dart b/protobuf/lib/src/protobuf/protobuf_enum.dart
index b96c119..46c173e 100644
--- a/protobuf/lib/src/protobuf/protobuf_enum.dart
+++ b/protobuf/lib/src/protobuf/protobuf_enum.dart
@@ -2,6 +2,8 @@
// 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.
+// ignore_for_file: non_constant_identifier_names
+
part of '../../protobuf.dart';
/// A base class for all proto enum types.
@@ -37,12 +39,28 @@
/// Creates a new constant [ProtobufEnum] using [value] and [name].
const ProtobufEnum(this.value, this.name);
- /// Creates a Map for all of the [ProtobufEnum]s in [byIndex], mapping each
+ /// This function is for generated code.
+ ///
+ /// Creates a Map for all of the [ProtobufEnum]s in [enumValues], mapping each
/// [ProtobufEnum]'s [value] to the [ProtobufEnum].
- static Map<int, T> initByValue<T extends ProtobufEnum>(List<T> byIndex) {
+ ///
+ /// @nodoc
+ static Map<int, T> initByValue<T extends ProtobufEnum>(List<T> enumValues) {
final byValue = <int, T>{};
- for (final v in byIndex) {
- byValue[v.value] = v;
+ for (final enumValue in enumValues) {
+ byValue[enumValue.value] = enumValue;
+ }
+ return byValue;
+ }
+
+ /// This function is for generated code.
+ ///
+ /// @nodoc
+ static List<T?> $_initByValueList<T extends ProtobufEnum>(
+ List<T> enumValues, int maxEnumValue) {
+ final byValue = List<T?>.filled(maxEnumValue + 1, null);
+ for (final enumValue in enumValues) {
+ byValue[enumValue.value] = enumValue;
}
return byValue;
}
diff --git a/protoc_plugin/Makefile b/protoc_plugin/Makefile
index bab82e3..6c04fa5 100644
--- a/protoc_plugin/Makefile
+++ b/protoc_plugin/Makefile
@@ -30,6 +30,7 @@
entity \
enum_extension \
enum_name \
+ enums \
extend_unittest \
ExtensionEnumNameConflict \
ExtensionNameConflict \
diff --git a/protoc_plugin/lib/src/enum_generator.dart b/protoc_plugin/lib/src/enum_generator.dart
index a1cd8b5..287a4fe 100644
--- a/protoc_plugin/lib/src/enum_generator.dart
+++ b/protoc_plugin/lib/src/enum_generator.dart
@@ -175,11 +175,37 @@
out.println('];');
out.println();
- out.println(
- 'static final $coreImportPrefix.Map<$coreImportPrefix.int, $classname> _byValue ='
- ' $protobufImportPrefix.ProtobufEnum.initByValue(values);');
- out.println('static $classname? valueOf($coreImportPrefix.int value) =>'
- ' _byValue[value];');
+ var maxEnumValue = -1;
+ for (final valueDescriptor in _canonicalValues) {
+ if (valueDescriptor.number.isNegative) {
+ maxEnumValue = -1; // don't use list
+ break;
+ }
+ if (valueDescriptor.number > maxEnumValue) {
+ maxEnumValue = valueDescriptor.number;
+ }
+ }
+
+ final useList = _canonicalValues.isEmpty ||
+ (maxEnumValue >= 0 &&
+ _canonicalValues.length / (maxEnumValue + 1) >= 0.7);
+
+ if (useList) {
+ out.println(
+ 'static final $coreImportPrefix.List<$classname?> _byValue ='
+ ' $protobufImportPrefix.ProtobufEnum.\$_initByValueList(values, $maxEnumValue);');
+
+ out.println('static $classname? valueOf($coreImportPrefix.int value) =>'
+ ' value < 0 || value >= _byValue.length ? null : _byValue[value];');
+ } else {
+ out.println(
+ 'static final $coreImportPrefix.Map<$coreImportPrefix.int, $classname> _byValue ='
+ ' $protobufImportPrefix.ProtobufEnum.initByValue(values);');
+
+ out.println('static $classname? valueOf($coreImportPrefix.int value) =>'
+ ' _byValue[value];');
+ }
+
out.println();
out.println('const $classname._(super.v, super.n);');
diff --git a/protoc_plugin/lib/src/generated/descriptor.pbenum.dart b/protoc_plugin/lib/src/generated/descriptor.pbenum.dart
index dc6f349..15909ed 100644
--- a/protoc_plugin/lib/src/generated/descriptor.pbenum.dart
+++ b/protoc_plugin/lib/src/generated/descriptor.pbenum.dart
@@ -88,9 +88,10 @@
TYPE_SINT64,
];
- static final $core.Map<$core.int, FieldDescriptorProto_Type> _byValue =
- $pb.ProtobufEnum.initByValue(values);
- static FieldDescriptorProto_Type? valueOf($core.int value) => _byValue[value];
+ static final $core.List<FieldDescriptorProto_Type?> _byValue =
+ $pb.ProtobufEnum.$_initByValueList(values, 18);
+ static FieldDescriptorProto_Type? valueOf($core.int value) =>
+ value < 0 || value >= _byValue.length ? null : _byValue[value];
const FieldDescriptorProto_Type._(super.v, super.n);
}
@@ -111,10 +112,10 @@
LABEL_REPEATED,
];
- static final $core.Map<$core.int, FieldDescriptorProto_Label> _byValue =
- $pb.ProtobufEnum.initByValue(values);
+ static final $core.List<FieldDescriptorProto_Label?> _byValue =
+ $pb.ProtobufEnum.$_initByValueList(values, 3);
static FieldDescriptorProto_Label? valueOf($core.int value) =>
- _byValue[value];
+ value < 0 || value >= _byValue.length ? null : _byValue[value];
const FieldDescriptorProto_Label._(super.v, super.n);
}
@@ -137,9 +138,10 @@
LITE_RUNTIME,
];
- static final $core.Map<$core.int, FileOptions_OptimizeMode> _byValue =
- $pb.ProtobufEnum.initByValue(values);
- static FileOptions_OptimizeMode? valueOf($core.int value) => _byValue[value];
+ static final $core.List<FileOptions_OptimizeMode?> _byValue =
+ $pb.ProtobufEnum.$_initByValueList(values, 3);
+ static FileOptions_OptimizeMode? valueOf($core.int value) =>
+ value < 0 || value >= _byValue.length ? null : _byValue[value];
const FileOptions_OptimizeMode._(super.v, super.n);
}
@@ -159,9 +161,10 @@
STRING_PIECE,
];
- static final $core.Map<$core.int, FieldOptions_CType> _byValue =
- $pb.ProtobufEnum.initByValue(values);
- static FieldOptions_CType? valueOf($core.int value) => _byValue[value];
+ static final $core.List<FieldOptions_CType?> _byValue =
+ $pb.ProtobufEnum.$_initByValueList(values, 2);
+ static FieldOptions_CType? valueOf($core.int value) =>
+ value < 0 || value >= _byValue.length ? null : _byValue[value];
const FieldOptions_CType._(super.v, super.n);
}
@@ -185,9 +188,10 @@
JS_NUMBER,
];
- static final $core.Map<$core.int, FieldOptions_JSType> _byValue =
- $pb.ProtobufEnum.initByValue(values);
- static FieldOptions_JSType? valueOf($core.int value) => _byValue[value];
+ static final $core.List<FieldOptions_JSType?> _byValue =
+ $pb.ProtobufEnum.$_initByValueList(values, 2);
+ static FieldOptions_JSType? valueOf($core.int value) =>
+ value < 0 || value >= _byValue.length ? null : _byValue[value];
const FieldOptions_JSType._(super.v, super.n);
}
@@ -212,10 +216,10 @@
IDEMPOTENT,
];
- static final $core.Map<$core.int, MethodOptions_IdempotencyLevel> _byValue =
- $pb.ProtobufEnum.initByValue(values);
+ static final $core.List<MethodOptions_IdempotencyLevel?> _byValue =
+ $pb.ProtobufEnum.$_initByValueList(values, 2);
static MethodOptions_IdempotencyLevel? valueOf($core.int value) =>
- _byValue[value];
+ value < 0 || value >= _byValue.length ? null : _byValue[value];
const MethodOptions_IdempotencyLevel._(super.v, super.n);
}
diff --git a/protoc_plugin/lib/src/generated/plugin.pbenum.dart b/protoc_plugin/lib/src/generated/plugin.pbenum.dart
index 399158c..2f427c9 100644
--- a/protoc_plugin/lib/src/generated/plugin.pbenum.dart
+++ b/protoc_plugin/lib/src/generated/plugin.pbenum.dart
@@ -27,10 +27,10 @@
FEATURE_PROTO3_OPTIONAL,
];
- static final $core.Map<$core.int, CodeGeneratorResponse_Feature> _byValue =
- $pb.ProtobufEnum.initByValue(values);
+ static final $core.List<CodeGeneratorResponse_Feature?> _byValue =
+ $pb.ProtobufEnum.$_initByValueList(values, 1);
static CodeGeneratorResponse_Feature? valueOf($core.int value) =>
- _byValue[value];
+ value < 0 || value >= _byValue.length ? null : _byValue[value];
const CodeGeneratorResponse_Feature._(super.v, super.n);
}
diff --git a/protoc_plugin/test/generated_message_test.dart b/protoc_plugin/test/generated_message_test.dart
index a25f6a4..63f8f1e 100644
--- a/protoc_plugin/test/generated_message_test.dart
+++ b/protoc_plugin/test/generated_message_test.dart
@@ -10,6 +10,7 @@
import '../out/protos/constructor_args/google/protobuf/unittest_import.pb.dart'
as constructor_args_unittest_import;
import '../out/protos/duplicate_names_import.pb.dart';
+import '../out/protos/enums.pb.dart';
import '../out/protos/google/protobuf/unittest.pb.dart';
import '../out/protos/google/protobuf/unittest_import.pb.dart';
import '../out/protos/google/protobuf/unittest_optimize_for.pb.dart';
@@ -890,4 +891,27 @@
// constructor arguments, to be able to reuse `assertAllFieldsSet`.
assertAllFieldsSet(TestAllTypes.fromBuffer(value.writeToBuffer()));
});
+
+ test('Handling enums defined out of order', () {
+ final message = MessageWithEnums();
+ for (final enum_ in DenseEnum.values) {
+ message.denseEnums.add(enum_);
+ }
+ for (final enum_ in DenseEnumOutOfOrder.values) {
+ message.denseOutOfOrderEnums.add(enum_);
+ }
+ for (final enum_ in SparseEnum.values) {
+ message.sparseEnums.add(enum_);
+ }
+ for (final enum_ in SparseEnumOutOfOrder.values) {
+ message.sparseOutOfOrderEnums.add(enum_);
+ }
+
+ final encoded = message.writeToBuffer();
+ final decoded = MessageWithEnums.fromBuffer(encoded);
+ expect(decoded.denseEnums, DenseEnum.values);
+ expect(decoded.denseOutOfOrderEnums, DenseEnumOutOfOrder.values);
+ expect(decoded.sparseEnums, SparseEnum.values);
+ expect(decoded.sparseOutOfOrderEnums, SparseEnumOutOfOrder.values);
+ });
}
diff --git a/protoc_plugin/test/goldens/deprecations.pbenum b/protoc_plugin/test/goldens/deprecations.pbenum
index 30bc5d7..c21797e 100644
--- a/protoc_plugin/test/goldens/deprecations.pbenum
+++ b/protoc_plugin/test/goldens/deprecations.pbenum
@@ -24,9 +24,10 @@
A2,
];
- static final $core.Map<$core.int, A> _byValue =
- $pb.ProtobufEnum.initByValue(values);
- static A? valueOf($core.int value) => _byValue[value];
+ static final $core.List<A?> _byValue =
+ $pb.ProtobufEnum.$_initByValueList(values, 1);
+ static A? valueOf($core.int value) =>
+ value < 0 || value >= _byValue.length ? null : _byValue[value];
const A._(super.v, super.n);
}
diff --git a/protoc_plugin/test/goldens/doc_comments.pbenum b/protoc_plugin/test/goldens/doc_comments.pbenum
index 7cde50e..1a35f24 100644
--- a/protoc_plugin/test/goldens/doc_comments.pbenum
+++ b/protoc_plugin/test/goldens/doc_comments.pbenum
@@ -26,9 +26,10 @@
A2,
];
- static final $core.Map<$core.int, A> _byValue =
- $pb.ProtobufEnum.initByValue(values);
- static A? valueOf($core.int value) => _byValue[value];
+ static final $core.List<A?> _byValue =
+ $pb.ProtobufEnum.$_initByValueList(values, 1);
+ static A? valueOf($core.int value) =>
+ value < 0 || value >= _byValue.length ? null : _byValue[value];
const A._(super.v, super.n);
}
diff --git a/protoc_plugin/test/goldens/enum b/protoc_plugin/test/goldens/enum
index 812247d..2dca720 100644
--- a/protoc_plugin/test/goldens/enum
+++ b/protoc_plugin/test/goldens/enum
@@ -11,8 +11,8 @@
WORK,
];
- static final $core.Map<$core.int, PhoneType> _byValue = $pb.ProtobufEnum.initByValue(values);
- static PhoneType? valueOf($core.int value) => _byValue[value];
+ static final $core.List<PhoneType?> _byValue = $pb.ProtobufEnum.$_initByValueList(values, 2);
+ static PhoneType? valueOf($core.int value) => value < 0 || value >= _byValue.length ? null : _byValue[value];
const PhoneType._(super.v, super.n);
}
diff --git a/protoc_plugin/test/goldens/messageGeneratorEnums b/protoc_plugin/test/goldens/messageGeneratorEnums
index 3469c7b..3d673da 100644
--- a/protoc_plugin/test/goldens/messageGeneratorEnums
+++ b/protoc_plugin/test/goldens/messageGeneratorEnums
@@ -11,8 +11,8 @@
WORK,
];
- static final $core.Map<$core.int, PhoneNumber_PhoneType> _byValue = $pb.ProtobufEnum.initByValue(values);
- static PhoneNumber_PhoneType? valueOf($core.int value) => _byValue[value];
+ static final $core.List<PhoneNumber_PhoneType?> _byValue = $pb.ProtobufEnum.$_initByValueList(values, 2);
+ static PhoneNumber_PhoneType? valueOf($core.int value) => value < 0 || value >= _byValue.length ? null : _byValue[value];
const PhoneNumber_PhoneType._(super.v, super.n);
}
diff --git a/protoc_plugin/test/goldens/topLevelEnum.pbenum b/protoc_plugin/test/goldens/topLevelEnum.pbenum
index 30d81a2..69cb70d 100644
--- a/protoc_plugin/test/goldens/topLevelEnum.pbenum
+++ b/protoc_plugin/test/goldens/topLevelEnum.pbenum
@@ -26,8 +26,8 @@
WORK,
];
- static final $core.Map<$core.int, PhoneType> _byValue = $pb.ProtobufEnum.initByValue(values);
- static PhoneType? valueOf($core.int value) => _byValue[value];
+ static final $core.List<PhoneType?> _byValue = $pb.ProtobufEnum.$_initByValueList(values, 2);
+ static PhoneType? valueOf($core.int value) => value < 0 || value >= _byValue.length ? null : _byValue[value];
const PhoneType._(super.v, super.n);
}
diff --git a/protoc_plugin/test/protos/enums.proto b/protoc_plugin/test/protos/enums.proto
new file mode 100644
index 0000000..03647c9
--- /dev/null
+++ b/protoc_plugin/test/protos/enums.proto
@@ -0,0 +1,54 @@
+enum DenseEnum {
+ DENSE_ENUM_0 = 0;
+ DENSE_ENUM_1 = 1;
+ DENSE_ENUM_2 = 2;
+ DENSE_ENUM_3 = 3;
+ DENSE_ENUM_4 = 4;
+}
+
+enum DenseEnumOutOfOrder {
+ DENSE_ENUM_OOO_0 = 0;
+ DENSE_ENUM_OOO_2 = 2;
+ DENSE_ENUM_OOO_4 = 4;
+ DENSE_ENUM_OOO_3 = 3;
+ DENSE_ENUM_OOO_1 = 1;
+}
+
+enum SparseEnum {
+ SPARSE_ENUM_ZERO = 0;
+ SPARSE_ENUM_MIN_INT = -2147483648;
+ SPARSE_ENUM_1 = -1000000000;
+ SPARSE_ENUM_2 = -100000000;
+ SPARSE_ENUM_3 = -10000000;
+ SPARSE_ENUM_4 = -1000000;
+ SPARSE_ENUM_5 = -100000;
+ SPARSE_ENUM_6 = 100000;
+ SPARSE_ENUM_7 = 1000000;
+ SPARSE_ENUM_8 = 10000000;
+ SPARSE_ENUM_9 = 100000000;
+ SPARSE_ENUM_10 = 1000000000;
+ SPARSE_ENUM_MAX_INT = 2147483647;
+}
+
+enum SparseEnumOutOfOrder {
+ SPARSE_ENUM_OOO_ZERO = 0;
+ SPARSE_ENUM_OOO_1 = -1000000000;
+ SPARSE_ENUM_OOO_MAX_INT = 2147483647;
+ SPARSE_ENUM_OOO_4 = -1000000;
+ SPARSE_ENUM_OOO_6 = 100000;
+ SPARSE_ENUM_OOO_MIN_INT = -2147483648;
+ SPARSE_ENUM_OOO_7 = 1000000;
+ SPARSE_ENUM_OOO_5 = -100000;
+ SPARSE_ENUM_OOO_8 = 10000000;
+ SPARSE_ENUM_OOO_3 = -10000000;
+ SPARSE_ENUM_OOO_10 = 1000000000;
+ SPARSE_ENUM_OOO_2 = -100000000;
+ SPARSE_ENUM_OOO_9 = 100000000;
+}
+
+message MessageWithEnums {
+ repeated DenseEnum denseEnums = 1;
+ repeated DenseEnumOutOfOrder denseOutOfOrderEnums = 2;
+ repeated SparseEnum sparseEnums = 3;
+ repeated SparseEnumOutOfOrder sparseOutOfOrderEnums = 4;
+}
