From a0976daab41305df9e12b4a8d01e2ce2f3862261 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Tue, 24 May 2022 09:41:40 +0200 Subject: [PATCH 01/14] Implement fast path when merging fields of same message Fixes #60 --- protobuf/lib/src/protobuf/field_set.dart | 63 ++++++++++++++++++++---- 1 file changed, 53 insertions(+), 10 deletions(-) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index ff3628274..0439b064e 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -768,14 +768,16 @@ class _FieldSet { /// in this message. Repeated fields are appended. Singular sub-messages are /// recursively merged. void _mergeFromMessage(_FieldSet other) { - // TODO(https://github.com/google/protobuf.dart/issues/60): Recognize - // when `this` and [other] are the same protobuf (e.g. from cloning). In - // this case, we can merge the non-extension fields without field lookups or - // validation checks. - + final sameMessage = identical(_meta, other._meta); for (var fi in other._infosSortedByTag) { var value = other._values[fi.index!]; - if (value != null) _mergeField(fi, value, isExtension: false); + if (value != null) { + if (sameMessage) { + _mergeNonExtensionFieldUnchecked(fi, value); + } else { + _mergeField(fi, value, isExtension: false); + } + } } if (other._hasExtensions) { var others = other._extensions!; @@ -791,14 +793,55 @@ class _FieldSet { } } - void _mergeField(FieldInfo otherFi, fieldValue, {bool? isExtension}) { + void _mergeNonExtensionFieldUnchecked(FieldInfo fi, dynamic fieldValue) { + if (fi.isMapField) { + final mapInfo = fi as MapFieldInfo; + final map = + mapInfo._ensureMapField(_meta, this) as PbMap; + if (_isGroupOrMessage(mapInfo.valueFieldType)) { + for (MapEntry entry in fieldValue.entries) { + map[entry.key] = (entry.value as GeneratedMessage).deepCopy(); + } + } else { + map.addAll(fieldValue); + } + return; + } + + if (fi.isRepeated) { + if (_isGroupOrMessage(fi.type)) { + PbListBase pbList = fieldValue; + var repeatedFields = fi._ensureRepeatedField(_meta, this); + for (var i = 0; i < pbList.length; ++i) { + repeatedFields.add(pbList[i].deepCopy()); + } + } else { + PbListBase pbList = fieldValue; + fi._ensureRepeatedField(_meta, this).addAll(pbList); + } + return; + } + + if (fi.isGroupOrMessage) { + final currentFieldValue = _values[fi.index!]; + if (currentFieldValue == null) { + fieldValue = (fieldValue as GeneratedMessage).deepCopy(); + } else { + fieldValue = currentFieldValue..mergeFromMessage(fieldValue); + } + } + + _setNonExtensionFieldUnchecked(_meta, fi, fieldValue); + } + + void _mergeField(FieldInfo otherFi, fieldValue, {required bool isExtension}) { final tagNumber = otherFi.tagNumber; // Determine the FieldInfo to use. // Don't allow regular fields to be overwritten by extensions. final meta = _meta; var fi = _nonExtensionInfo(meta, tagNumber); - if (fi == null && isExtension!) { + if (fi == null && isExtension) { // This will overwrite any existing extension field info. fi = otherFi; } @@ -836,7 +879,7 @@ class _FieldSet { } if (otherFi.isGroupOrMessage) { - final currentFi = isExtension! + final currentFi = isExtension ? _ensureExtensions()._getFieldOrNull(fi as Extension) : _values[fi.index!]; @@ -847,7 +890,7 @@ class _FieldSet { } } - if (isExtension!) { + if (isExtension) { _ensureExtensions() ._setFieldAndInfo(fi as Extension, fieldValue); } else { From 1ba216cf61c8737473cd010d7d4e10315b697362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Fri, 5 Aug 2022 08:59:20 +0200 Subject: [PATCH 02/14] Post merge fixups, add a benchmark program --- benchmarks/bin/deep_copy.dart | 43 ++++++++++++++++++++++++ protobuf/lib/src/protobuf/field_set.dart | 4 +-- 2 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 benchmarks/bin/deep_copy.dart diff --git a/benchmarks/bin/deep_copy.dart b/benchmarks/bin/deep_copy.dart new file mode 100644 index 000000000..9dbb9fe85 --- /dev/null +++ b/benchmarks/bin/deep_copy.dart @@ -0,0 +1,43 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// 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. + +import 'package:protobuf/protobuf.dart'; +import 'package:protobuf_benchmarks/benchmark_base.dart'; +import 'package:protobuf_benchmarks/generated/google_message1_proto2.pb.dart' + as p2; +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'; + +class Benchmark extends BenchmarkBase { + final p2.GoogleMessage1 _message1Proto2; + final p3.GoogleMessage1 _message1Proto3; + final GoogleMessage2 _message2; + + Benchmark(String name, List message1Proto2Input, + List message1Proto3Input, List message2Input) + : _message1Proto2 = p2.GoogleMessage1.fromBuffer(message1Proto2Input), + _message1Proto3 = p3.GoogleMessage1.fromBuffer(message1Proto3Input), + _message2 = GoogleMessage2.fromBuffer(message2Input), + super(name); + + @override + void run() { + _message1Proto2.deepCopy(); + _message1Proto3.deepCopy(); + _message2.deepCopy(); + } +} + +void main() { + List message1Proto2Input = + readfile('datasets/google_message1_proto2.pb'); + List message1Proto3Input = + readfile('datasets/google_message1_proto3.pb'); + List message2Input = readfile('datasets/google_message2.pb'); + Benchmark( + 'deep_copy', message1Proto2Input, message1Proto3Input, message2Input) + .report(); +} diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index 7d81d2198..19f89ae28 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -752,13 +752,13 @@ class _FieldSet { if (fi.isRepeated) { if (_isGroupOrMessage(fi.type)) { - PbListBase pbList = fieldValue; + PbList pbList = fieldValue; var repeatedFields = fi._ensureRepeatedField(_meta, this); for (var i = 0; i < pbList.length; ++i) { repeatedFields.add(pbList[i].deepCopy()); } } else { - PbListBase pbList = fieldValue; + PbList pbList = fieldValue; fi._ensureRepeatedField(_meta, this).addAll(pbList); } return; From cd64518dba3379e240dae10b5509c4483787132b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Fri, 5 Aug 2022 09:00:48 +0200 Subject: [PATCH 03/14] Rename sameMessage -> sameMessageType --- protobuf/lib/src/protobuf/field_set.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index 19f89ae28..1dcbef2bb 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -710,11 +710,11 @@ class _FieldSet { /// in this message. Repeated fields are appended. Singular sub-messages are /// recursively merged. void _mergeFromMessage(_FieldSet other) { - final sameMessage = identical(_meta, other._meta); + final sameMessageType = identical(_meta, other._meta); for (var fi in other._infosSortedByTag) { var value = other._values[fi.index!]; if (value != null) { - if (sameMessage) { + if (sameMessageType) { _mergeNonExtensionFieldUnchecked(fi, value); } else { _mergeField(fi, value, isExtension: false); From e45dd01ba1e58f8b16404f635639a7097bd02e01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Fri, 5 Aug 2022 09:17:16 +0200 Subject: [PATCH 04/14] Remove dynamic calls --- protobuf/lib/src/protobuf/field_set.dart | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index 1dcbef2bb..94892c170 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -706,9 +706,9 @@ class _FieldSet { /// Merges the contents of the [other] into this message. /// - /// Singular fields that are set in [other] overwrite the corresponding fields - /// in this message. Repeated fields are appended. Singular sub-messages are - /// recursively merged. + /// Singular fields that are set in [other] overwrite the corresponding + /// fields in this message. Repeated fields are appended. Singular + /// sub-messages are recursively merged. void _mergeFromMessage(_FieldSet other) { final sameMessageType = identical(_meta, other._meta); for (var fi in other._infosSortedByTag) { @@ -721,6 +721,7 @@ class _FieldSet { } } } + if (other._hasExtensions) { var others = other._extensions!; for (var tagNumber in others._tagNumbers) { @@ -741,8 +742,10 @@ class _FieldSet { final map = mapInfo._ensureMapField(_meta, this) as PbMap; if (_isGroupOrMessage(mapInfo.valueFieldType)) { - for (MapEntry entry in fieldValue.entries) { - map[entry.key] = (entry.value as GeneratedMessage).deepCopy(); + Map pbMap = fieldValue; + for (final entry in pbMap.entries) { + GeneratedMessage value = entry.value; + map[entry.key] = value.deepCopy(); } } else { map.addAll(fieldValue); @@ -769,7 +772,8 @@ class _FieldSet { if (currentFieldValue == null) { fieldValue = (fieldValue as GeneratedMessage).deepCopy(); } else { - fieldValue = currentFieldValue..mergeFromMessage(fieldValue); + final GeneratedMessage currentMsg = currentFieldValue; + fieldValue = currentMsg..mergeFromMessage(fieldValue); } } From 682301c6bd6691b9b5abb3753820cba80d2e9603 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Mon, 12 Sep 2022 14:07:46 +0200 Subject: [PATCH 05/14] Assume same message type in mergeFromMessage --- protobuf/lib/src/protobuf/field_set.dart | 42 ++++++++---------------- 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index c6b5d3243..ebc7821a1 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -723,20 +723,17 @@ class _FieldSet { /// fields in this message. Repeated fields are appended. Singular /// sub-messages are recursively merged. void _mergeFromMessage(_FieldSet other) { - // TODO(https://github.com/google/protobuf.dart/issues/60): Recognize - // when `this` and [other] are the same protobuf (e.g. from cloning). In - // this case, we can merge the non-extension fields without field lookups or - // validation checks. _ensureWritable(); - final sameMessageType = identical(_meta, other._meta); + + if (!identical(_meta, other._meta)) { + throw ArgumentError( + 'Merging messages with different types (BuilderInfos)', 'other'); + } + for (var fi in other._infosSortedByTag) { var value = other._values[fi.index!]; if (value != null) { - if (sameMessageType) { - _mergeNonExtensionFieldUnchecked(fi, value); - } else { - _mergeField(fi, value, isExtension: false); - } + _mergeNonExtensionFieldUnchecked(fi, value); } } @@ -745,7 +742,7 @@ class _FieldSet { for (var tagNumber in otherExtensions._tagNumbers) { var extension = otherExtensions._getInfoOrNull(tagNumber)!; var value = otherExtensions._getFieldOrNull(extension); - _mergeField(extension, value, isExtension: true); + _mergeExtensionField(extension, value); } } @@ -799,19 +796,15 @@ class _FieldSet { _setNonExtensionFieldUnchecked(_meta, fi, fieldValue); } - void _mergeField(FieldInfo otherFi, fieldValue, {required bool isExtension}) { + void _mergeExtensionField(FieldInfo otherFi, fieldValue) { final tagNumber = otherFi.tagNumber; // Determine the FieldInfo to use. // Don't allow regular fields to be overwritten by extensions. final meta = _meta; - var fi = _nonExtensionInfo(meta, tagNumber); - if (fi == null && isExtension) { - // This will overwrite any existing extension field info. - fi = otherFi; - } + var fi = _nonExtensionInfo(meta, tagNumber) ?? otherFi; - if (fi!.isMapField) { + if (fi.isMapField) { if (fieldValue == null) { return; } @@ -846,9 +839,8 @@ class _FieldSet { } if (otherFi.isGroupOrMessage) { - final currentFi = isExtension - ? _ensureExtensions()._getFieldOrNull(fi as Extension) - : _values[fi.index!]; + final currentFi = + _ensureExtensions()._getFieldOrNull(fi as Extension); GeneratedMessage msg = fieldValue; if (currentFi == null) { @@ -859,13 +851,7 @@ class _FieldSet { } } - if (isExtension) { - _ensureExtensions() - ._setFieldAndInfo(fi as Extension, fieldValue); - } else { - _validateField(fi, fieldValue); - _setNonExtensionFieldUnchecked(meta, fi, fieldValue); - } + _ensureExtensions()._setFieldAndInfo(fi as Extension, fieldValue); } // Error-checking From 5d4d5d0ed74c1fa1bd979016a63dd0d5a4993fa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Mon, 12 Sep 2022 14:14:25 +0200 Subject: [PATCH 06/14] Rename a variable in mergeNonExtensionField for consistency with mergeExtensionField --- protobuf/lib/src/protobuf/field_set.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index ebc7821a1..b758e4970 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -733,7 +733,7 @@ class _FieldSet { for (var fi in other._infosSortedByTag) { var value = other._values[fi.index!]; if (value != null) { - _mergeNonExtensionFieldUnchecked(fi, value); + _mergeNonExtensionField(fi, value); } } @@ -752,7 +752,7 @@ class _FieldSet { } } - void _mergeNonExtensionFieldUnchecked(FieldInfo fi, dynamic fieldValue) { + void _mergeNonExtensionField(FieldInfo fi, dynamic fieldValue) { if (fi.isMapField) { final mapInfo = fi as MapFieldInfo; final map = @@ -839,14 +839,14 @@ class _FieldSet { } if (otherFi.isGroupOrMessage) { - final currentFi = + final currentFieldValue = _ensureExtensions()._getFieldOrNull(fi as Extension); GeneratedMessage msg = fieldValue; - if (currentFi == null) { + if (currentFieldValue == null) { fieldValue = msg.deepCopy(); } else { - final GeneratedMessage currentMsg = currentFi; + final GeneratedMessage currentMsg = currentFieldValue; fieldValue = currentMsg..mergeFromMessage(msg); } } From da3971e35f054a136127f9634fc487a451653c81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Mon, 12 Sep 2022 14:16:23 +0200 Subject: [PATCH 07/14] Refactor null handling when merging extension fields for consistency with non-extension fields --- protobuf/lib/src/protobuf/field_set.dart | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index b758e4970..d0ac29d86 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -742,7 +742,9 @@ class _FieldSet { for (var tagNumber in otherExtensions._tagNumbers) { var extension = otherExtensions._getInfoOrNull(tagNumber)!; var value = otherExtensions._getFieldOrNull(extension); - _mergeExtensionField(extension, value); + if (value != null) { + _mergeExtensionField(extension, value); + } } } @@ -805,9 +807,6 @@ class _FieldSet { var fi = _nonExtensionInfo(meta, tagNumber) ?? otherFi; if (fi.isMapField) { - if (fieldValue == null) { - return; - } final MapFieldInfo f = fi as dynamic; final PbMap map = f._ensureMapField(meta, this) as dynamic; From cacd006cb1ea93c4e224cd568b98b77da561c420 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Mon, 12 Sep 2022 14:28:02 +0200 Subject: [PATCH 08/14] Refactor merge methods for exts and non-exts; they're almost identical now --- protobuf/lib/src/protobuf/field_set.dart | 48 ++++++++++-------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index d0ac29d86..49ada08b3 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -756,13 +756,12 @@ class _FieldSet { void _mergeNonExtensionField(FieldInfo fi, dynamic fieldValue) { if (fi.isMapField) { - final mapInfo = fi as MapFieldInfo; - final map = - mapInfo._ensureMapField(_meta, this) as PbMap; + final MapFieldInfo mapInfo = fi as dynamic; + final map = mapInfo._ensureMapField(_meta, this); if (_isGroupOrMessage(mapInfo.valueFieldType)) { - Map pbMap = fieldValue; - for (final entry in pbMap.entries) { - GeneratedMessage value = entry.value; + final Map fieldValueMap = fieldValue; + for (final entry in fieldValueMap.entries) { + final GeneratedMessage value = entry.value; map[entry.key] = value.deepCopy(); } } else { @@ -774,12 +773,12 @@ class _FieldSet { if (fi.isRepeated) { if (_isGroupOrMessage(fi.type)) { PbList pbList = fieldValue; - var repeatedFields = fi._ensureRepeatedField(_meta, this); + final repeatedFields = fi._ensureRepeatedField(_meta, this); for (var i = 0; i < pbList.length; ++i) { repeatedFields.add(pbList[i].deepCopy()); } } else { - PbList pbList = fieldValue; + final PbList pbList = fieldValue; fi._ensureRepeatedField(_meta, this).addAll(pbList); } return; @@ -798,22 +797,15 @@ class _FieldSet { _setNonExtensionFieldUnchecked(_meta, fi, fieldValue); } - void _mergeExtensionField(FieldInfo otherFi, fieldValue) { - final tagNumber = otherFi.tagNumber; - - // Determine the FieldInfo to use. - // Don't allow regular fields to be overwritten by extensions. - final meta = _meta; - var fi = _nonExtensionInfo(meta, tagNumber) ?? otherFi; - + void _mergeExtensionField(FieldInfo fi, fieldValue) { if (fi.isMapField) { - final MapFieldInfo f = fi as dynamic; - final PbMap map = - f._ensureMapField(meta, this) as dynamic; - if (_isGroupOrMessage(f.valueFieldType)) { - PbMap fieldValueMap = fieldValue; + final MapFieldInfo mapInfo = fi as dynamic; + final map = mapInfo._ensureMapField(_meta, this); + if (_isGroupOrMessage(mapInfo.valueFieldType)) { + final Map fieldValueMap = fieldValue; for (final entry in fieldValueMap.entries) { - map[entry.key] = (entry.value as GeneratedMessage).deepCopy(); + final GeneratedMessage value = entry.value; + map[entry.key] = value.deepCopy(); } } else { map.addAll(fieldValue); @@ -822,22 +814,20 @@ class _FieldSet { } if (fi.isRepeated) { - if (_isGroupOrMessage(otherFi.type)) { - // fieldValue must be a PbList of GeneratedMessage. + if (_isGroupOrMessage(fi.type)) { PbList pbList = fieldValue; - var repeatedFields = fi._ensureRepeatedField(meta, this); + final repeatedFields = fi._ensureRepeatedField(_meta, this); for (var i = 0; i < pbList.length; ++i) { repeatedFields.add(pbList[i].deepCopy()); } } else { - // fieldValue must be at least a PbList. - PbList pbList = fieldValue; - fi._ensureRepeatedField(meta, this).addAll(pbList); + final PbList pbList = fieldValue; + fi._ensureRepeatedField(_meta, this).addAll(pbList); } return; } - if (otherFi.isGroupOrMessage) { + if (fi.isGroupOrMessage) { final currentFieldValue = _ensureExtensions()._getFieldOrNull(fi as Extension); From 3cccb8fd9b8c3d19b0851e00011205c309bd2e8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Mon, 12 Sep 2022 14:30:13 +0200 Subject: [PATCH 09/14] More refactoring --- protobuf/lib/src/protobuf/field_set.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index 49ada08b3..0e5dd0de0 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -787,7 +787,8 @@ class _FieldSet { if (fi.isGroupOrMessage) { final currentFieldValue = _values[fi.index!]; if (currentFieldValue == null) { - fieldValue = (fieldValue as GeneratedMessage).deepCopy(); + GeneratedMessage msg = fieldValue; + fieldValue = msg.deepCopy(); } else { final GeneratedMessage currentMsg = currentFieldValue; fieldValue = currentMsg..mergeFromMessage(fieldValue); From 8d5aa0336781c31301a8744bb9852214e77575bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Mon, 12 Sep 2022 14:46:31 +0200 Subject: [PATCH 10/14] Make Dart 2.12 happy --- protobuf/lib/src/protobuf/field_set.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index 0e5dd0de0..e031ce96b 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -727,7 +727,7 @@ class _FieldSet { if (!identical(_meta, other._meta)) { throw ArgumentError( - 'Merging messages with different types (BuilderInfos)', 'other'); + 'Merging messages with different types (BuilderInfos)'); } for (var fi in other._infosSortedByTag) { From 1dddc1bdb6c885ba40012ad93d1f14f104dfd134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Mon, 12 Sep 2022 14:54:48 +0200 Subject: [PATCH 11/14] Tweak type casts, add `final` to locals --- protobuf/lib/src/protobuf/field_set.dart | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index e031ce96b..009050673 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -772,14 +772,14 @@ class _FieldSet { if (fi.isRepeated) { if (_isGroupOrMessage(fi.type)) { - PbList pbList = fieldValue; + final List list = fieldValue; final repeatedFields = fi._ensureRepeatedField(_meta, this); - for (var i = 0; i < pbList.length; ++i) { - repeatedFields.add(pbList[i].deepCopy()); + for (var i = 0; i < list.length; ++i) { + repeatedFields.add(list[i].deepCopy()); } } else { - final PbList pbList = fieldValue; - fi._ensureRepeatedField(_meta, this).addAll(pbList); + final List list = fieldValue; + fi._ensureRepeatedField(_meta, this).addAll(list); } return; } @@ -787,7 +787,7 @@ class _FieldSet { if (fi.isGroupOrMessage) { final currentFieldValue = _values[fi.index!]; if (currentFieldValue == null) { - GeneratedMessage msg = fieldValue; + final GeneratedMessage msg = fieldValue; fieldValue = msg.deepCopy(); } else { final GeneratedMessage currentMsg = currentFieldValue; @@ -816,14 +816,14 @@ class _FieldSet { if (fi.isRepeated) { if (_isGroupOrMessage(fi.type)) { - PbList pbList = fieldValue; + final List list = fieldValue; final repeatedFields = fi._ensureRepeatedField(_meta, this); - for (var i = 0; i < pbList.length; ++i) { - repeatedFields.add(pbList[i].deepCopy()); + for (var i = 0; i < list.length; ++i) { + repeatedFields.add(list[i].deepCopy()); } } else { - final PbList pbList = fieldValue; - fi._ensureRepeatedField(_meta, this).addAll(pbList); + final List list = fieldValue; + fi._ensureRepeatedField(_meta, this).addAll(list); } return; } @@ -832,7 +832,7 @@ class _FieldSet { final currentFieldValue = _ensureExtensions()._getFieldOrNull(fi as Extension); - GeneratedMessage msg = fieldValue; + final GeneratedMessage msg = fieldValue; if (currentFieldValue == null) { fieldValue = msg.deepCopy(); } else { From aa492a9c896f834ef7ea488110e5ba8e0e6a5eb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Mon, 12 Sep 2022 15:04:02 +0200 Subject: [PATCH 12/14] More refactoring --- protobuf/lib/src/protobuf/field_set.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index 009050673..a62c67d07 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -786,12 +786,12 @@ class _FieldSet { if (fi.isGroupOrMessage) { final currentFieldValue = _values[fi.index!]; + final GeneratedMessage msg = fieldValue; if (currentFieldValue == null) { - final GeneratedMessage msg = fieldValue; fieldValue = msg.deepCopy(); } else { final GeneratedMessage currentMsg = currentFieldValue; - fieldValue = currentMsg..mergeFromMessage(fieldValue); + fieldValue = currentMsg..mergeFromMessage(msg); } } From d2959397957666dc5249627b055c34f1ea32f345 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Mon, 12 Sep 2022 15:06:38 +0200 Subject: [PATCH 13/14] Merge merge field methods --- protobuf/lib/src/protobuf/field_set.dart | 62 +++++------------------- 1 file changed, 12 insertions(+), 50 deletions(-) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index a62c67d07..5c39c41f1 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -733,7 +733,7 @@ class _FieldSet { for (var fi in other._infosSortedByTag) { var value = other._values[fi.index!]; if (value != null) { - _mergeNonExtensionField(fi, value); + _mergeField(fi, value, isExtension: false); } } @@ -743,7 +743,7 @@ class _FieldSet { var extension = otherExtensions._getInfoOrNull(tagNumber)!; var value = otherExtensions._getFieldOrNull(extension); if (value != null) { - _mergeExtensionField(extension, value); + _mergeField(extension, value, isExtension: true); } } } @@ -754,7 +754,7 @@ class _FieldSet { } } - void _mergeNonExtensionField(FieldInfo fi, dynamic fieldValue) { + void _mergeField(FieldInfo fi, fieldValue, {required bool isExtension}) { if (fi.isMapField) { final MapFieldInfo mapInfo = fi as dynamic; final map = mapInfo._ensureMapField(_meta, this); @@ -785,52 +785,9 @@ class _FieldSet { } if (fi.isGroupOrMessage) { - final currentFieldValue = _values[fi.index!]; - final GeneratedMessage msg = fieldValue; - if (currentFieldValue == null) { - fieldValue = msg.deepCopy(); - } else { - final GeneratedMessage currentMsg = currentFieldValue; - fieldValue = currentMsg..mergeFromMessage(msg); - } - } - - _setNonExtensionFieldUnchecked(_meta, fi, fieldValue); - } - - void _mergeExtensionField(FieldInfo fi, fieldValue) { - if (fi.isMapField) { - final MapFieldInfo mapInfo = fi as dynamic; - final map = mapInfo._ensureMapField(_meta, this); - if (_isGroupOrMessage(mapInfo.valueFieldType)) { - final Map fieldValueMap = fieldValue; - for (final entry in fieldValueMap.entries) { - final GeneratedMessage value = entry.value; - map[entry.key] = value.deepCopy(); - } - } else { - map.addAll(fieldValue); - } - return; - } - - if (fi.isRepeated) { - if (_isGroupOrMessage(fi.type)) { - final List list = fieldValue; - final repeatedFields = fi._ensureRepeatedField(_meta, this); - for (var i = 0; i < list.length; ++i) { - repeatedFields.add(list[i].deepCopy()); - } - } else { - final List list = fieldValue; - fi._ensureRepeatedField(_meta, this).addAll(list); - } - return; - } - - if (fi.isGroupOrMessage) { - final currentFieldValue = - _ensureExtensions()._getFieldOrNull(fi as Extension); + final currentFieldValue = isExtension + ? _ensureExtensions()._getFieldOrNull(fi as Extension) + : _values[fi.index!]; final GeneratedMessage msg = fieldValue; if (currentFieldValue == null) { @@ -841,7 +798,12 @@ class _FieldSet { } } - _ensureExtensions()._setFieldAndInfo(fi as Extension, fieldValue); + if (isExtension) { + _ensureExtensions() + ._setFieldAndInfo(fi as Extension, fieldValue); + } else { + _setNonExtensionFieldUnchecked(_meta, fi, fieldValue); + } } // Error-checking From 88c3d85e8017633097f16f8ebe6e0cc700499c1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Mon, 12 Sep 2022 15:18:57 +0200 Subject: [PATCH 14/14] Update docs --- protobuf/lib/src/protobuf/field_set.dart | 5 +++++ protobuf/lib/src/protobuf/generated_message.dart | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index 5c39c41f1..22992cd2b 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -722,6 +722,11 @@ class _FieldSet { /// Singular fields that are set in [other] overwrite the corresponding /// fields in this message. Repeated fields are appended. Singular /// sub-messages are recursively merged. + /// + /// Messages are expected to have the same [BuilderInfo]. + /// + /// Throws [ArgumentError] when the messages don't have the same + /// [BuilderInfo]. void _mergeFromMessage(_FieldSet other) { _ensureWritable(); diff --git a/protobuf/lib/src/protobuf/generated_message.dart b/protobuf/lib/src/protobuf/generated_message.dart index c77a17e2e..6a0323c96 100644 --- a/protobuf/lib/src/protobuf/generated_message.dart +++ b/protobuf/lib/src/protobuf/generated_message.dart @@ -373,6 +373,11 @@ abstract class GeneratedMessage { /// Singular fields that are set in [other] overwrite the corresponding fields /// in this message. Repeated fields are appended. Singular sub-messages are /// recursively merged. + /// + /// Messages are expected to have the same [BuilderInfo]. + /// + /// Throws [ArgumentError] when the messages don't have the same + /// [BuilderInfo]. @pragma('dart2js:noInline') void mergeFromMessage(GeneratedMessage other) => _fieldSet._mergeFromMessage(other._fieldSet);