Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions benchmarks/bin/deep_copy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ import 'package:protobuf_benchmarks/generated/google_message1_proto3.pb.dart'
as p3;
import 'package:protobuf_benchmarks/generated/google_message2.pb.dart';
import 'package:protobuf_benchmarks/readfile.dart';
import 'package:protobuf/protobuf.dart';

GeneratedMessage? sink1;
GeneratedMessage? sink2;
GeneratedMessage? sink3;

class Benchmark extends BenchmarkBase {
final p2.GoogleMessage1 _message1Proto2;
Expand All @@ -26,12 +31,9 @@ class Benchmark extends BenchmarkBase {

@override
void run() {
// ignore: unused_result
_message1Proto2.deepCopy();
// ignore: unused_result
_message1Proto3.deepCopy();
// ignore: unused_result
_message2.deepCopy();
sink1 = _message1Proto2.deepCopy();
sink2 = _message1Proto3.deepCopy();
sink3 = _message2.deepCopy();
}
}

Expand All @@ -49,4 +51,10 @@ void main() {
message1Proto3Input,
message2Input,
).report();

if (int.parse('1') == 0) {
print(sink1);
print(sink2);
print(sink3);
}
}
4 changes: 4 additions & 0 deletions protobuf/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## 5.0.0-wip

* Improve performance of `GeneratedMessage.deepCopy`. ([#742])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update the pubspec version to agree w/ the changelog (=> 5.0.0-wip). If we do plan to publish after landing this (no particular opinion), we can change both versions to 5.0.0).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't we discuss this already? #1037 (comment)

5.0.0-wip breaks the build because it's not considered 5.0.0, and plugin now needs 5.0.0.

We can update the changelog before or even after releasing 5.0.0.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, I'd forgotten.


[#742]: https://github.com/google/protobuf.dart/pull/742

## 4.2.0

* Internal refactoring to split the package into libraries. This allows
Expand Down
38 changes: 35 additions & 3 deletions protobuf/lib/src/protobuf/extension_field_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,15 @@ part of 'internal.dart';

class ExtensionFieldSet {
final FieldSet _parent;
final Map<int, Extension> _info = <int, Extension>{};
final Map<int, dynamic> _values = <int, dynamic>{};
final Map<int, Extension> _info;
final Map<int, dynamic> _values;
bool _isReadOnly = false;

ExtensionFieldSet(this._parent);
ExtensionFieldSet(this._parent)
: _info = <int, Extension>{},
_values = <int, dynamic>{};

ExtensionFieldSet._(this._parent, this._info, this._values);

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

Expand Down Expand Up @@ -211,6 +215,34 @@ class ExtensionFieldSet {
);
}
}

ExtensionFieldSet _deepCopy(FieldSet parent) {
final newExtensionFieldSet = ExtensionFieldSet._(
parent,
Map.from(_info),
Map.from(_values),
);

final newValues = newExtensionFieldSet._values;

for (final entry in _values.entries) {
final tag = entry.key;
final value = entry.value;
final fieldInfo = _info[tag]!;
if (fieldInfo.isMapField) {
final PbMap? map = value;
newValues[tag] = map?._deepCopy();
} else if (fieldInfo.isRepeated) {
final PbList? list = value;
newValues[tag] = list?._deepCopy();
} else if (fieldInfo.isGroupOrMessage) {
final GeneratedMessage? message = value;
newValues[tag] = message?.deepCopy();
}
}

return newExtensionFieldSet;
}
}

extension ExtensionFieldSetInternalExtension on ExtensionFieldSet {
Expand Down
58 changes: 58 additions & 0 deletions protobuf/lib/src/protobuf/field_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -916,6 +916,64 @@ class FieldSet {

_oneofCases?.addAll(original._oneofCases!);
}

// This assumes that [this] is fresh, i.e. no extensions, no values set.
//
// The reason why this updates [this] instead of returning a new [FieldSet] is
// that to start cloning we need to create an empty instance via
// `createEmptyInstance`, which already creates an empty [FieldSet], which we
// reuse here.
void _deepCopyFrom(FieldSet original) {
final info = _meta;

assert(_values.length == original._values.length);

// memcpy the original's values to avoid redundant bounds checks below by
// copying scalar fields one by one.
_values.setAll(0, original._values);

for (var index = 0; index < info.byIndex.length; index++) {
final fieldInfo = info.byIndex[index];
if (fieldInfo.isMapField) {
final PbMap? originalMap = original._values[index];
if (originalMap == null) continue;
_values[index] = originalMap._deepCopy();
} else if (fieldInfo.isRepeated) {
final PbList? originalList = original._values[index];
if (originalList == null) continue;
_values[index] = originalList._deepCopy();
} else if (fieldInfo.isGroupOrMessage) {
final GeneratedMessage? message = original._values[index];
_values[index] = message?.deepCopy();
}

// Scalar fields are already copied above with `setAll`.
}

assert(_extensions == null);
final originalExtensions = original._extensions;
if (originalExtensions != null) {
_extensions = originalExtensions._deepCopy(this);
}

assert(_unknownFields == null);
final originalUnknownFields = original._unknownFields;
if (originalUnknownFields != null) {
_unknownFields = originalUnknownFields._deepCopy();
}

assert(_unknownJsonData == null);
final originalUnknownJsonData = original._unknownJsonData;
if (originalUnknownJsonData != null) {
_unknownJsonData = Map.from(originalUnknownJsonData);
}

assert(_oneofCases == null || _oneofCases.isEmpty);
final originalOneofCases = original._oneofCases;
if (originalOneofCases != null) {
_oneofCases!.addAll(originalOneofCases);
}
}
}

extension FieldSetInternalExtension on FieldSet {
Expand Down
7 changes: 5 additions & 2 deletions protobuf/lib/src/protobuf/generated_message.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ abstract class GeneratedMessage {
BuilderInfo get info_;

/// Creates a deep copy of the fields in this message.
/// (The generated code uses [mergeFromMessage].)
@Deprecated(
'Using this can add significant size overhead to your binary. '
'Use [GeneratedMessageGenericExtensions.deepCopy] instead. '
Expand Down Expand Up @@ -625,7 +624,11 @@ extension GeneratedMessageGenericExtensions<T extends GeneratedMessage> on T {
'[GeneratedMessageGenericExtensions.deepCopy] '
'does not update the message, returns a new message',
)
T deepCopy() => info_.createEmptyInstance!() as T..mergeFromMessage(this);
T deepCopy() {
final newMessage = info_.createEmptyInstance!();
newMessage._fieldSet._deepCopyFrom(_fieldSet);
return newMessage as T;
}
}

extension GeneratedMessageInternalExtension on GeneratedMessage {
Expand Down
16 changes: 16 additions & 0 deletions protobuf/lib/src/protobuf/pb_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -234,4 +234,20 @@ class PbList<E> extends ListBase<E> {
static Never _readOnlyError(String methodName) {
throw UnsupportedError("'$methodName' on a read-only list");
}

PbList<E> _deepCopy() {
final newList = PbList<E>(check: _check);
final wrappedList = _wrappedList;
final newWrappedList = newList._wrappedList;
if (wrappedList.isNotEmpty) {
if (wrappedList[0] is GeneratedMessage) {
for (final message in wrappedList) {
newWrappedList.add((message as GeneratedMessage).deepCopy() as E);
}
} else {
newWrappedList.addAll(wrappedList);
}
}
return newList;
}
}
15 changes: 15 additions & 0 deletions protobuf/lib/src/protobuf/pb_map.dart
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,19 @@ class PbMap<K, V> extends MapBase<K, V> {
}
return this;
}

PbMap<K, V> _deepCopy() {
final newMap = PbMap<K, V>(keyFieldType, valueFieldType);
final wrappedMap = _wrappedMap;
final newWrappedMap = newMap._wrappedMap;
if (PbFieldType.isGroupOrMessage(valueFieldType)) {
for (final entry in wrappedMap.entries) {
newWrappedMap[entry.key] =
(entry.value as GeneratedMessage).deepCopy() as V;
}
} else {
newWrappedMap.addAll(wrappedMap);
}
return newMap;
}
}
55 changes: 52 additions & 3 deletions protobuf/lib/src/protobuf/unknown_field_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,15 @@ part of 'internal.dart';
class UnknownFieldSet {
static final UnknownFieldSet emptyUnknownFieldSet =
UnknownFieldSet().._markReadOnly();
final Map<int, UnknownFieldSetField> _fields = <int, UnknownFieldSetField>{};

UnknownFieldSet();
final Map<int, UnknownFieldSetField> _fields;

UnknownFieldSet._clone(UnknownFieldSet unknownFieldSet) {
UnknownFieldSet() : _fields = <int, UnknownFieldSetField>{};

UnknownFieldSet._(this._fields);

UnknownFieldSet._clone(UnknownFieldSet unknownFieldSet)
: _fields = <int, UnknownFieldSetField>{} {
mergeFromUnknownFieldSet(unknownFieldSet);
}

Expand Down Expand Up @@ -195,6 +199,16 @@ class UnknownFieldSet {
_throwFrozenMessageModificationError('UnknownFieldSet', methodName);
}
}

UnknownFieldSet _deepCopy() {
Map<int, UnknownFieldSetField> newFields = {};
for (final entry in _fields.entries) {
final key = entry.key;
final value = entry.value;
newFields[key] = value._deepCopy();
}
return UnknownFieldSet._(newFields);
}
}

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

UnknownFieldSetField()
: _lengthDelimited = <List<int>>[],
_varints = <Int64>[],
_fixed32s = <int>[],
_fixed64s = <Int64>[],
_groups = <UnknownFieldSet>[];

UnknownFieldSetField._(
this._lengthDelimited,
this._varints,
this._fixed32s,
this._fixed64s,
this._groups,
);

bool _isReadOnly = false;

void _markReadOnly() {
Expand Down Expand Up @@ -309,4 +338,24 @@ class UnknownFieldSetField {
void addVarint(Int64 value) {
varints.add(value);
}

UnknownFieldSetField _deepCopy() {
final List<List<int>> newLengthDelimited = List.from(_lengthDelimited);
final List<Int64> newVarints = List.from(_varints);
final List<int> newFixed32s = List.from(_fixed32s);
final List<Int64> newFixed64s = List.from(_fixed64s);

final List<UnknownFieldSet> newGroups = [];
for (final group in _groups) {
newGroups.add(group._deepCopy());
}

return UnknownFieldSetField._(
newLengthDelimited,
newVarints,
newFixed32s,
newFixed64s,
newGroups,
);
}
}
4 changes: 4 additions & 0 deletions protoc_plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
## 23.0.0

* Update generated code for protobuf 5.0.0.
* Update generated `clone` members to take advantage of faster `deepCopy`
implementation in protobuf 5.0.0. ([#742])

[#742]: https://github.com/google/protobuf.dart/pull/742

## 22.5.0

Expand Down
4 changes: 2 additions & 2 deletions protoc_plugin/lib/src/gen/dart_options.pb.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class DartMixin extends $pb.GeneratedMessage {
..hasRequiredFields = false;

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

@$core.Deprecated('See https://github.com/google/protobuf.dart/issues/998.')
Imports clone() => Imports()..mergeFromMessage(this);
Imports clone() => deepCopy();
@$core.Deprecated('See https://github.com/google/protobuf.dart/issues/998.')
Imports copyWith(void Function(Imports) updates) =>
super.copyWith((message) => updates(message as Imports)) as Imports;
Expand Down
Loading
Loading