Skip to content

Commit 971bcae

Browse files
osa1osa1
authored
Use a map to cache the valueOf functions for enums (#1047)
(This is cl/658131451 ported to open source, not my CL) CL description: (slightly edited) This is a generated-code size optimization. The main change is to generate the valueOf functions for protobuf enums programatically, rather than (1) use the per-enum `FooPbEnum.valueOf` static function tear-foo which (2) uses a per-enum static variable that is lazily initialized to map from int value to the protobuf enum value. With ~1000 protobuf enums, this allows most of the `valueOf` static function tear-off closures to tree-shaken, together with their functions and the code for the initializer of the map. The secondary change is to use more specialized 'add' methods to `BuilderInfo`, allowing many fields to be 'defined' with a call with fewer arguments. Together, these changes reduce the size of the main unit of an app by about 1.3%, and would have a similar effect on native targets. This change requires dart-lang/sdk@175dc05, which was released with Dart 3.6.0.
1 parent 8750ed7 commit 971bcae

File tree

8 files changed

+262
-90
lines changed

8 files changed

+262
-90
lines changed

‎protobuf/CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
* Improve performance of `GeneratedMessage.deepCopy`. ([#742])
44
* Fix unknown enum handling in `GeneratedMessage.mergeFromProto3Json` when
55
the `ignoreUnknownFields` optional argument is `true`. ([#853])
6+
* Add `BuilderInfo` methods to support protoc-plugin 23.0.0. ([#1047])
67

78
[#742]: https://github.com/google/protobuf.dart/pull/742
89
[#853]: https://github.com/google/protobuf.dart/pull/853
10+
[#1047]: https://github.com/google/protobuf.dart/pull/1047
911

1012
## 4.2.0
1113

‎protobuf/lib/src/protobuf/builder_info.dart‎

Lines changed: 167 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -64,22 +64,29 @@ class BuilderInfo {
6464
List<ProtobufEnum>? enumValues, {
6565
String? protoName,
6666
}) {
67+
if (tagNumber == 0) {
68+
addUnused();
69+
} else {
70+
final index = byIndex.length;
71+
final fieldInfo = FieldInfo<T>(
72+
name,
73+
tagNumber,
74+
index,
75+
fieldType!,
76+
defaultOrMaker: defaultOrMaker,
77+
subBuilder: subBuilder,
78+
valueOf: valueOf,
79+
enumValues: enumValues,
80+
protoName: protoName,
81+
);
82+
_addField(fieldInfo);
83+
}
84+
}
85+
86+
// Support for tree-shaking of unused fields.
87+
void addUnused() {
6788
final index = byIndex.length;
68-
final fieldInfo =
69-
(tagNumber == 0)
70-
? FieldInfo.dummy(index)
71-
: FieldInfo<T>(
72-
name,
73-
tagNumber,
74-
index,
75-
fieldType!,
76-
defaultOrMaker: defaultOrMaker,
77-
subBuilder: subBuilder,
78-
valueOf: valueOf,
79-
enumValues: enumValues,
80-
protoName: protoName,
81-
);
82-
_addField(fieldInfo);
89+
_addField(FieldInfo.dummy(index));
8390
}
8491

8592
void addMapField<K, V>(
@@ -243,6 +250,46 @@ class BuilderInfo {
243250
);
244251
}
245252

253+
/// Adds a double field.
254+
void aD(
255+
int tagNumber,
256+
String name, {
257+
int fieldType = PbFieldType.OD,
258+
dynamic defaultOrMaker,
259+
String? protoName,
260+
}) {
261+
add<double>(
262+
tagNumber,
263+
name,
264+
fieldType,
265+
defaultOrMaker,
266+
null,
267+
null,
268+
null,
269+
protoName: protoName,
270+
);
271+
}
272+
273+
/// Adds an int field.
274+
void aI(
275+
int tagNumber,
276+
String name, {
277+
int fieldType = PbFieldType.O3,
278+
dynamic defaultOrMaker,
279+
String? protoName,
280+
}) {
281+
add<int>(
282+
tagNumber,
283+
name,
284+
fieldType,
285+
defaultOrMaker,
286+
null,
287+
null,
288+
null,
289+
protoName: protoName,
290+
);
291+
}
292+
246293
// Enum.
247294
void e<T>(
248295
int tagNumber,
@@ -265,6 +312,30 @@ class BuilderInfo {
265312
);
266313
}
267314

315+
// Enum, updated version.
316+
void aE<E extends ProtobufEnum>(
317+
int tagNumber,
318+
String name, {
319+
int fieldType = PbFieldType.OE,
320+
dynamic defaultOrMaker,
321+
ValueOfFunc? valueOf,
322+
required List<E> enumValues,
323+
String? protoName,
324+
}) {
325+
defaultOrMaker ??= enumValues.first;
326+
valueOf ??= _findValueOfEnumFunction<E>(enumValues);
327+
add<E>(
328+
tagNumber,
329+
name,
330+
fieldType,
331+
defaultOrMaker,
332+
null,
333+
valueOf,
334+
enumValues,
335+
protoName: protoName,
336+
);
337+
}
338+
268339
// Repeated, not a message, group, or enum.
269340
void p<T>(int tagNumber, String name, int fieldType, {String? protoName}) {
270341
assert(
@@ -310,6 +381,33 @@ class BuilderInfo {
310381
);
311382
}
312383

384+
// Repeated enum.
385+
void pPE<E extends ProtobufEnum>(
386+
int tagNumber,
387+
String name, {
388+
int fieldType = PbFieldType.PE,
389+
ValueOfFunc? valueOf,
390+
required List<E> enumValues,
391+
ProtobufEnum? defaultEnumValue,
392+
String? protoName,
393+
}) {
394+
assert(PbFieldType.isEnum(fieldType));
395+
defaultEnumValue ??= enumValues.first;
396+
valueOf ??= _findValueOfEnumFunction<E>(enumValues);
397+
addRepeated<E>(
398+
tagNumber,
399+
name,
400+
fieldType,
401+
_checkNotNull,
402+
null,
403+
valueOf,
404+
enumValues,
405+
defaultEnumValue: defaultEnumValue,
406+
protoName: protoName,
407+
);
408+
}
409+
410+
// Optional Message.
313411
void aOM<T extends GeneratedMessage>(
314412
int tagNumber,
315413
String name, {
@@ -328,6 +426,7 @@ class BuilderInfo {
328426
);
329427
}
330428

429+
// reQuried Message.
331430
void aQM<T extends GeneratedMessage>(
332431
int tagNumber,
333432
String name, {
@@ -346,6 +445,25 @@ class BuilderInfo {
346445
);
347446
}
348447

448+
// rePeated Message, specialization of pc<T>.
449+
void pPM<T extends GeneratedMessage>(
450+
int tagNumber,
451+
String name, {
452+
required T Function() subBuilder,
453+
String? protoName,
454+
}) {
455+
addRepeated<T>(
456+
tagNumber,
457+
name,
458+
PbFieldType.PM,
459+
_checkNotNull,
460+
subBuilder,
461+
null,
462+
null,
463+
protoName: protoName,
464+
);
465+
}
466+
349467
// oneof declarations.
350468
void oo(int oneofIndex, List<int> tags) {
351469
for (final tag in tags) {
@@ -370,17 +488,9 @@ class BuilderInfo {
370488
}) {
371489
final mapEntryBuilderInfo =
372490
BuilderInfo(entryClassName, package: packageName)
491+
..add(mapKeyFieldNumber, 'key', keyFieldType, null, null, null, null)
373492
..add(
374-
PbMap._keyFieldNumber,
375-
'key',
376-
keyFieldType,
377-
null,
378-
null,
379-
null,
380-
null,
381-
)
382-
..add(
383-
PbMap._valueFieldNumber,
493+
mapValueFieldNumber,
384494
'value',
385495
valueFieldType,
386496
valueDefaultOrMaker,
@@ -498,3 +608,35 @@ extension BuilderInfoInternalExtension on BuilderInfo {
498608
int rawValue,
499609
) => _decodeEnum(tagNumber, registry, rawValue);
500610
}
611+
612+
E? Function(int) _findValueOfEnumFunction<E extends ProtobufEnum>(
613+
List<E> enumValues,
614+
) {
615+
final function = _valueOfFunctions[enumValues];
616+
if (function != null) {
617+
// 'as dynamic' causes an implicit downcast to `E? Function(int)`, which is
618+
// removed by dart2js at `-O3`.
619+
return function as dynamic;
620+
}
621+
return _valueOfFunctions[enumValues] = _makeValueOfEnumFunction<E>(
622+
enumValues,
623+
);
624+
}
625+
626+
/// Map for finding 'valueOf' functions for enums. An identity map is used since
627+
/// the `const` 'values' list for the enum is canonicalized and distinct for
628+
/// each enum.
629+
final _valueOfFunctions =
630+
Map<List<ProtobufEnum>, ProtobufEnum? Function(int)>.identity();
631+
632+
E? Function(int) _makeValueOfEnumFunction<E extends ProtobufEnum>(
633+
List<E> values,
634+
) {
635+
Map<int, E>? map;
636+
637+
E? intToEnumValue(int value) {
638+
return (map ??= ProtobufEnum.initByValue(values))[value];
639+
}
640+
641+
return intToEnumValue;
642+
}

‎protoc_plugin/CHANGELOG.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
1-
## 23.0.0
1+
## 23.0.0-wip
2+
3+
Note: this version requires protobuf 5.0.0.
24

35
* Update generated code for protobuf 5.0.0.
46
* Update generated `clone` members to take advantage of faster `deepCopy`
57
implementation in protobuf 5.0.0. ([#742])
8+
* Code size improvements for enum fields. ([#1047])
69

710
[#742]: https://github.com/google/protobuf.dart/pull/742
11+
[#1047]: https://github.com/google/protobuf.dart/pull/1047
812

913
## 22.5.0
1014

‎protoc_plugin/lib/src/protobuf_field.dart‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,16 @@ class ProtobufField {
262262
} else if (isRepeated) {
263263
if (typeConstant == '$protobufImportPrefix.PbFieldType.PS') {
264264
invocation = 'pPS';
265+
} else if (typeConstant == '$protobufImportPrefix.PbFieldType.PE') {
266+
invocation = 'pPE<$type>';
267+
named['enumValues'] = '$type.values';
268+
final makeDefault = generateDefaultFunction(omitIfFirstEnumValue: true);
269+
if (makeDefault != null) {
270+
named['defaultEnumValue'] = makeDefault;
271+
}
272+
} else if (typeConstant == '$protobufImportPrefix.PbFieldType.PM') {
273+
invocation = 'pPM<$type>';
274+
named['subBuilder'] = '$type.create';
265275
} else {
266276
args.add(typeConstant);
267277
if (baseType.isMessage || baseType.isGroup || baseType.isEnum) {
@@ -280,14 +290,27 @@ class ProtobufField {
280290
}
281291
} else {
282292
// Singular field.
283-
final makeDefault = generateDefaultFunction();
293+
final makeDefault = generateDefaultFunction(omitIfFirstEnumValue: true);
284294

285295
if (baseType.isEnum) {
286-
args.add(typeConstant);
287-
named['defaultOrMaker'] = makeDefault;
288-
named['valueOf'] = '$type.valueOf';
296+
invocation = 'aE<$type>';
297+
if (typeConstant != '$protobufImportPrefix.PbFieldType.OE') {
298+
named['fieldType'] = typeConstant;
299+
}
300+
if (makeDefault != null) named['defaultOrMaker'] = makeDefault;
289301
named['enumValues'] = '$type.values';
290-
invocation = 'e<$type>';
302+
} else if (type == '$coreImportPrefix.int') {
303+
invocation = 'aI';
304+
if (typeConstant != '$protobufImportPrefix.PbFieldType.O3') {
305+
named['fieldType'] = typeConstant;
306+
}
307+
if (makeDefault != null) named['defaultOrMaker'] = makeDefault;
308+
} else if (type == '$coreImportPrefix.double') {
309+
invocation = 'aD';
310+
if (typeConstant != '$protobufImportPrefix.PbFieldType.OD') {
311+
named['fieldType'] = typeConstant;
312+
}
313+
if (makeDefault != null) named['defaultOrMaker'] = makeDefault;
291314
} else if (makeDefault == null) {
292315
switch (type) {
293316
case '$coreImportPrefix.String':
@@ -360,7 +383,7 @@ class ProtobufField {
360383
}
361384

362385
/// Returns a function expression that returns the field's default value.
363-
String? generateDefaultFunction() {
386+
String? generateDefaultFunction({bool omitIfFirstEnumValue = false}) {
364387
assert(!isRepeated);
365388
switch (descriptor.type) {
366389
case FieldDescriptorProto_Type.TYPE_BOOL:
@@ -422,6 +445,7 @@ class ProtobufField {
422445
descriptor.defaultValue.isNotEmpty) {
423446
return '$className.${descriptor.defaultValue}';
424447
} else if (gen._canonicalValues.isNotEmpty) {
448+
if (omitIfFirstEnumValue) return null;
425449
return '$className.${gen.dartNames[gen._canonicalValues[0].name]}';
426450
}
427451
return null;

‎protoc_plugin/test/goldens/messageGenerator.pb.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ class PhoneNumber extends $pb.GeneratedMessage {
88

99
static final $pb.BuilderInfo _i = $pb.BuilderInfo(_omitMessageNames ? '' : 'PhoneNumber', createEmptyInstance: create)
1010
..aQS(1, _omitFieldNames ? '' : 'number')
11-
..e<PhoneNumber_PhoneType>(2, _omitFieldNames ? '' : 'type', $pb.PbFieldType.OE, defaultOrMaker: PhoneNumber_PhoneType.MOBILE, valueOf: PhoneNumber_PhoneType.valueOf, enumValues: PhoneNumber_PhoneType.values)
11+
..aE<PhoneNumber_PhoneType>(2, _omitFieldNames ? '' : 'type', enumValues: PhoneNumber_PhoneType.values)
1212
..a<$core.String>(3, _omitFieldNames ? '' : 'name', $pb.PbFieldType.OS, defaultOrMaker: '\$')
1313
..aOS(4, _omitFieldNames ? '' : 'deprecatedField')
1414
;

0 commit comments

Comments
 (0)