Skip to content

Commit 19e5a25

Browse files
authored
Implement deepCopy as copying, instead of merge (#742)
Currently `deepCopy` and `clone` create an empty message, and then merge the cloned message into the empty message. This is slow as merge needs to validate each element added to the message. In this PR we instead directly copy the field set, and deeply copy map, list, and message fields. deepCopy benchmark results: | | Before | After | Diff | |------------------|---------|---------|--------| | AOT | 238 us | 151 us | -36% | | dart2js -O4 | 242 us | 140 us | -42% | | dart2wasm -O2 | 230 us | 123 us | -46% | | JIT | 189 us | 104 us | -44% | This CL is tested internally in cl/794080754. Fixes #60.
1 parent 6e9c9f4 commit 19e5a25

29 files changed

+472
-175
lines changed

benchmarks/bin/deep_copy.dart

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ import 'package:protobuf_benchmarks/generated/google_message1_proto3.pb.dart'
99
as p3;
1010
import 'package:protobuf_benchmarks/generated/google_message2.pb.dart';
1111
import 'package:protobuf_benchmarks/readfile.dart';
12+
import 'package:protobuf/protobuf.dart';
13+
14+
GeneratedMessage? sink1;
15+
GeneratedMessage? sink2;
16+
GeneratedMessage? sink3;
1217

1318
class Benchmark extends BenchmarkBase {
1419
final p2.GoogleMessage1 _message1Proto2;
@@ -26,12 +31,9 @@ class Benchmark extends BenchmarkBase {
2631

2732
@override
2833
void run() {
29-
// ignore: unused_result
30-
_message1Proto2.deepCopy();
31-
// ignore: unused_result
32-
_message1Proto3.deepCopy();
33-
// ignore: unused_result
34-
_message2.deepCopy();
34+
sink1 = _message1Proto2.deepCopy();
35+
sink2 = _message1Proto3.deepCopy();
36+
sink3 = _message2.deepCopy();
3537
}
3638
}
3739

@@ -49,4 +51,10 @@ void main() {
4951
message1Proto3Input,
5052
message2Input,
5153
).report();
54+
55+
if (int.parse('1') == 0) {
56+
print(sink1);
57+
print(sink2);
58+
print(sink3);
59+
}
5260
}

protobuf/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
## 5.0.0-wip
22

3+
* Improve performance of `GeneratedMessage.deepCopy`. ([#742])
4+
5+
[#742]: https://github.com/google/protobuf.dart/pull/742
6+
37
## 4.2.0
48

59
* Internal refactoring to split the package into libraries. This allows

protobuf/lib/src/protobuf/extension_field_set.dart

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,15 @@ part of 'internal.dart';
66

77
class ExtensionFieldSet {
88
final FieldSet _parent;
9-
final Map<int, Extension> _info = <int, Extension>{};
10-
final Map<int, dynamic> _values = <int, dynamic>{};
9+
final Map<int, Extension> _info;
10+
final Map<int, dynamic> _values;
1111
bool _isReadOnly = false;
1212

13-
ExtensionFieldSet(this._parent);
13+
ExtensionFieldSet(this._parent)
14+
: _info = <int, Extension>{},
15+
_values = <int, dynamic>{};
16+
17+
ExtensionFieldSet._(this._parent, this._info, this._values);
1418

1519
Extension? _getInfoOrNull(int tagNumber) => _info[tagNumber];
1620

@@ -211,6 +215,34 @@ class ExtensionFieldSet {
211215
);
212216
}
213217
}
218+
219+
ExtensionFieldSet _deepCopy(FieldSet parent) {
220+
final newExtensionFieldSet = ExtensionFieldSet._(
221+
parent,
222+
Map.from(_info),
223+
Map.from(_values),
224+
);
225+
226+
final newValues = newExtensionFieldSet._values;
227+
228+
for (final entry in _values.entries) {
229+
final tag = entry.key;
230+
final value = entry.value;
231+
final fieldInfo = _info[tag]!;
232+
if (fieldInfo.isMapField) {
233+
final PbMap? map = value;
234+
newValues[tag] = map?._deepCopy();
235+
} else if (fieldInfo.isRepeated) {
236+
final PbList? list = value;
237+
newValues[tag] = list?._deepCopy();
238+
} else if (fieldInfo.isGroupOrMessage) {
239+
final GeneratedMessage? message = value;
240+
newValues[tag] = message?.deepCopy();
241+
}
242+
}
243+
244+
return newExtensionFieldSet;
245+
}
214246
}
215247

216248
extension ExtensionFieldSetInternalExtension on ExtensionFieldSet {

protobuf/lib/src/protobuf/field_set.dart

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -916,6 +916,64 @@ class FieldSet {
916916

917917
_oneofCases?.addAll(original._oneofCases!);
918918
}
919+
920+
// This assumes that [this] is fresh, i.e. no extensions, no values set.
921+
//
922+
// The reason why this updates [this] instead of returning a new [FieldSet] is
923+
// that to start cloning we need to create an empty instance via
924+
// `createEmptyInstance`, which already creates an empty [FieldSet], which we
925+
// reuse here.
926+
void _deepCopyFrom(FieldSet original) {
927+
final info = _meta;
928+
929+
assert(_values.length == original._values.length);
930+
931+
// memcpy the original's values to avoid redundant bounds checks below by
932+
// copying scalar fields one by one.
933+
_values.setAll(0, original._values);
934+
935+
for (var index = 0; index < info.byIndex.length; index++) {
936+
final fieldInfo = info.byIndex[index];
937+
if (fieldInfo.isMapField) {
938+
final PbMap? originalMap = original._values[index];
939+
if (originalMap == null) continue;
940+
_values[index] = originalMap._deepCopy();
941+
} else if (fieldInfo.isRepeated) {
942+
final PbList? originalList = original._values[index];
943+
if (originalList == null) continue;
944+
_values[index] = originalList._deepCopy();
945+
} else if (fieldInfo.isGroupOrMessage) {
946+
final GeneratedMessage? message = original._values[index];
947+
_values[index] = message?.deepCopy();
948+
}
949+
950+
// Scalar fields are already copied above with `setAll`.
951+
}
952+
953+
assert(_extensions == null);
954+
final originalExtensions = original._extensions;
955+
if (originalExtensions != null) {
956+
_extensions = originalExtensions._deepCopy(this);
957+
}
958+
959+
assert(_unknownFields == null);
960+
final originalUnknownFields = original._unknownFields;
961+
if (originalUnknownFields != null) {
962+
_unknownFields = originalUnknownFields._deepCopy();
963+
}
964+
965+
assert(_unknownJsonData == null);
966+
final originalUnknownJsonData = original._unknownJsonData;
967+
if (originalUnknownJsonData != null) {
968+
_unknownJsonData = Map.from(originalUnknownJsonData);
969+
}
970+
971+
assert(_oneofCases == null || _oneofCases.isEmpty);
972+
final originalOneofCases = original._oneofCases;
973+
if (originalOneofCases != null) {
974+
_oneofCases!.addAll(originalOneofCases);
975+
}
976+
}
919977
}
920978

921979
extension FieldSetInternalExtension on FieldSet {

protobuf/lib/src/protobuf/generated_message.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ abstract class GeneratedMessage {
4848
BuilderInfo get info_;
4949

5050
/// Creates a deep copy of the fields in this message.
51-
/// (The generated code uses [mergeFromMessage].)
5251
@Deprecated(
5352
'Using this can add significant size overhead to your binary. '
5453
'Use [GeneratedMessageGenericExtensions.deepCopy] instead. '
@@ -625,7 +624,11 @@ extension GeneratedMessageGenericExtensions<T extends GeneratedMessage> on T {
625624
'[GeneratedMessageGenericExtensions.deepCopy] '
626625
'does not update the message, returns a new message',
627626
)
628-
T deepCopy() => info_.createEmptyInstance!() as T..mergeFromMessage(this);
627+
T deepCopy() {
628+
final newMessage = info_.createEmptyInstance!();
629+
newMessage._fieldSet._deepCopyFrom(_fieldSet);
630+
return newMessage as T;
631+
}
629632
}
630633

631634
extension GeneratedMessageInternalExtension on GeneratedMessage {

protobuf/lib/src/protobuf/pb_list.dart

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,4 +234,20 @@ class PbList<E> extends ListBase<E> {
234234
static Never _readOnlyError(String methodName) {
235235
throw UnsupportedError("'$methodName' on a read-only list");
236236
}
237+
238+
PbList<E> _deepCopy() {
239+
final newList = PbList<E>(check: _check);
240+
final wrappedList = _wrappedList;
241+
final newWrappedList = newList._wrappedList;
242+
if (wrappedList.isNotEmpty) {
243+
if (wrappedList[0] is GeneratedMessage) {
244+
for (final message in wrappedList) {
245+
newWrappedList.add((message as GeneratedMessage).deepCopy() as E);
246+
}
247+
} else {
248+
newWrappedList.addAll(wrappedList);
249+
}
250+
}
251+
return newList;
252+
}
237253
}

protobuf/lib/src/protobuf/pb_map.dart

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,4 +131,19 @@ class PbMap<K, V> extends MapBase<K, V> {
131131
}
132132
return this;
133133
}
134+
135+
PbMap<K, V> _deepCopy() {
136+
final newMap = PbMap<K, V>(keyFieldType, valueFieldType);
137+
final wrappedMap = _wrappedMap;
138+
final newWrappedMap = newMap._wrappedMap;
139+
if (PbFieldType.isGroupOrMessage(valueFieldType)) {
140+
for (final entry in wrappedMap.entries) {
141+
newWrappedMap[entry.key] =
142+
(entry.value as GeneratedMessage).deepCopy() as V;
143+
}
144+
} else {
145+
newWrappedMap.addAll(wrappedMap);
146+
}
147+
return newMap;
148+
}
134149
}

protobuf/lib/src/protobuf/unknown_field_set.dart

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,15 @@ part of 'internal.dart';
88
class UnknownFieldSet {
99
static final UnknownFieldSet emptyUnknownFieldSet =
1010
UnknownFieldSet().._markReadOnly();
11-
final Map<int, UnknownFieldSetField> _fields = <int, UnknownFieldSetField>{};
1211

13-
UnknownFieldSet();
12+
final Map<int, UnknownFieldSetField> _fields;
1413

15-
UnknownFieldSet._clone(UnknownFieldSet unknownFieldSet) {
14+
UnknownFieldSet() : _fields = <int, UnknownFieldSetField>{};
15+
16+
UnknownFieldSet._(this._fields);
17+
18+
UnknownFieldSet._clone(UnknownFieldSet unknownFieldSet)
19+
: _fields = <int, UnknownFieldSetField>{} {
1620
mergeFromUnknownFieldSet(unknownFieldSet);
1721
}
1822

@@ -195,6 +199,16 @@ class UnknownFieldSet {
195199
_throwFrozenMessageModificationError('UnknownFieldSet', methodName);
196200
}
197201
}
202+
203+
UnknownFieldSet _deepCopy() {
204+
Map<int, UnknownFieldSetField> newFields = {};
205+
for (final entry in _fields.entries) {
206+
final key = entry.key;
207+
final value = entry.value;
208+
newFields[key] = value._deepCopy();
209+
}
210+
return UnknownFieldSet._(newFields);
211+
}
198212
}
199213

200214
/// An unknown field in a [UnknownFieldSet].
@@ -211,6 +225,21 @@ class UnknownFieldSetField {
211225
List<Int64> get fixed64s => _fixed64s;
212226
List<UnknownFieldSet> get groups => _groups;
213227

228+
UnknownFieldSetField()
229+
: _lengthDelimited = <List<int>>[],
230+
_varints = <Int64>[],
231+
_fixed32s = <int>[],
232+
_fixed64s = <Int64>[],
233+
_groups = <UnknownFieldSet>[];
234+
235+
UnknownFieldSetField._(
236+
this._lengthDelimited,
237+
this._varints,
238+
this._fixed32s,
239+
this._fixed64s,
240+
this._groups,
241+
);
242+
214243
bool _isReadOnly = false;
215244

216245
void _markReadOnly() {
@@ -309,4 +338,24 @@ class UnknownFieldSetField {
309338
void addVarint(Int64 value) {
310339
varints.add(value);
311340
}
341+
342+
UnknownFieldSetField _deepCopy() {
343+
final List<List<int>> newLengthDelimited = List.from(_lengthDelimited);
344+
final List<Int64> newVarints = List.from(_varints);
345+
final List<int> newFixed32s = List.from(_fixed32s);
346+
final List<Int64> newFixed64s = List.from(_fixed64s);
347+
348+
final List<UnknownFieldSet> newGroups = [];
349+
for (final group in _groups) {
350+
newGroups.add(group._deepCopy());
351+
}
352+
353+
return UnknownFieldSetField._(
354+
newLengthDelimited,
355+
newVarints,
356+
newFixed32s,
357+
newFixed64s,
358+
newGroups,
359+
);
360+
}
312361
}

protoc_plugin/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
## 23.0.0
22

33
* Update generated code for protobuf 5.0.0.
4+
* Update generated `clone` members to take advantage of faster `deepCopy`
5+
implementation in protobuf 5.0.0. ([#742])
6+
7+
[#742]: https://github.com/google/protobuf.dart/pull/742
48

59
## 22.5.0
610

protoc_plugin/lib/src/gen/dart_options.pb.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class DartMixin extends $pb.GeneratedMessage {
5050
..hasRequiredFields = false;
5151

5252
@$core.Deprecated('See https://github.com/google/protobuf.dart/issues/998.')
53-
DartMixin clone() => DartMixin()..mergeFromMessage(this);
53+
DartMixin clone() => deepCopy();
5454
@$core.Deprecated('See https://github.com/google/protobuf.dart/issues/998.')
5555
DartMixin copyWith(void Function(DartMixin) updates) =>
5656
super.copyWith((message) => updates(message as DartMixin)) as DartMixin;
@@ -130,7 +130,7 @@ class Imports extends $pb.GeneratedMessage {
130130
..hasRequiredFields = false;
131131

132132
@$core.Deprecated('See https://github.com/google/protobuf.dart/issues/998.')
133-
Imports clone() => Imports()..mergeFromMessage(this);
133+
Imports clone() => deepCopy();
134134
@$core.Deprecated('See https://github.com/google/protobuf.dart/issues/998.')
135135
Imports copyWith(void Function(Imports) updates) =>
136136
super.copyWith((message) => updates(message as Imports)) as Imports;

0 commit comments

Comments
 (0)