Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make wrapped lists in PbList monomorphic #902

Merged
merged 6 commits into from
Nov 23, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions protobuf/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,15 @@
`@useResult` and will generate a warning when its result is not used.
([#896])

* **Breaking:** `PbMap.unmodifiable` now takes key and value field types as
arguments, instead of another `PbMap`.

To migrate, use `PbMap.unmodifiable(map.keyFieldType, map.valueFieldType)`
instead of `PbMap.unmodifiable(map)`. ([#902])

[#738]: https://github.com/google/protobuf.dart/issues/738
[#896]: https://github.com/google/protobuf.dart/issues/896
[#902]: https://github.com/google/protobuf.dart/issues/902

## 3.1.0

Expand Down
3 changes: 1 addition & 2 deletions protobuf/lib/src/protobuf/field_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,7 @@ class _FieldSet {
assert(fi.isMapField);

if (_isReadOnly) {
return PbMap<K, V>.unmodifiable(
PbMap<K, V>(fi.keyFieldType, fi.valueFieldType));
return PbMap<K, V>.unmodifiable(fi.keyFieldType, fi.valueFieldType);
}

final map = fi._createMapField();
Expand Down
17 changes: 15 additions & 2 deletions protobuf/lib/src/protobuf/pb_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,20 @@ typedef CheckFunc<E> = void Function(E? x);

/// A [ListBase] implementation used for protobuf `repeated` fields.
class PbList<E> extends ListBase<E> {
/// The actual list storing the elements.
///
/// Note: We want only one [List] implementation class to be stored here to
/// make sure the list operations are monomorphic and can be inlined. In
/// constructors make sure initializers for this field all return the same
/// implementation class. (e.g. `_GrowableList` on the VM)
final List<E> _wrappedList;

/// A growable list, to be used in `unmodifiable` constructor to avoid
/// allocating a list every time.
///
/// We can't use `const []` as it makes the `_wrappedList` field polymorphic.
static final _emptyList = <Never>[];

final CheckFunc<E> _check;

bool _isReadOnly = false;
Expand All @@ -23,11 +36,11 @@ class PbList<E> extends ListBase<E> {
_check = check;

PbList.unmodifiable()
: _wrappedList = const [],
: _wrappedList = _emptyList,
_check = _checkNotNull,
_isReadOnly = true;

PbList.from(List from)
PbList.from(List<E> from)
: _wrappedList = List<E>.from(from),
_check = _checkNotNull;

Expand Down
29 changes: 19 additions & 10 deletions protobuf/lib/src/protobuf/pb_map.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,33 @@ class PbMap<K, V> extends MapBase<K, V> {
static const int _keyFieldNumber = 1;
static const int _valueFieldNumber = 2;

/// The actual list storing the elements.
///
/// Note: We want only one [Map] implementation class to be stored here to
/// make sure the map operations are monomorphic and can be inlined. In
/// constructors make sure initializers for this field all return the same
/// implementation class.
final Map<K, V> _wrappedMap;

bool _isReadonly = false;
/// The map instance to be used in `PbMap.unmodifiable`.
///
/// We can't use `const {}` as it makes the `_wrappedMap` field polymorphic.
static final _emptyMap = <Never, Never>{};

bool _isReadOnly = false;

PbMap(this.keyFieldType, this.valueFieldType) : _wrappedMap = <K, V>{};

PbMap.unmodifiable(PbMap other)
: keyFieldType = other.keyFieldType,
valueFieldType = other.valueFieldType,
_wrappedMap = Map.unmodifiable(other._wrappedMap),
_isReadonly = true;
PbMap.unmodifiable(this.keyFieldType, this.valueFieldType)
: _wrappedMap = _emptyMap,
_isReadOnly = true;

@override
V? operator [](Object? key) => _wrappedMap[key];

@override
void operator []=(K key, V value) {
if (_isReadonly) {
if (_isReadOnly) {
throw UnsupportedError('Attempted to change a read-only map field');
}
ArgumentError.checkNotNull(key, 'key');
Expand Down Expand Up @@ -77,7 +86,7 @@ class PbMap<K, V> extends MapBase<K, V> {

@override
void clear() {
if (_isReadonly) {
if (_isReadOnly) {
throw UnsupportedError('Attempted to change a read-only map field');
}
_wrappedMap.clear();
Expand All @@ -88,7 +97,7 @@ class PbMap<K, V> extends MapBase<K, V> {

@override
V? remove(Object? key) {
if (_isReadonly) {
if (_isReadOnly) {
throw UnsupportedError('Attempted to change a read-only map field');
}
return _wrappedMap.remove(key);
Expand All @@ -111,7 +120,7 @@ class PbMap<K, V> extends MapBase<K, V> {
}

PbMap freeze() {
_isReadonly = true;
_isReadOnly = true;
if (_isGroupOrMessage(valueFieldType)) {
for (final subMessage in values as Iterable<GeneratedMessage>) {
subMessage.freeze();
Expand Down