Make wrapped lists in PbList monomorphic (#902)

In `PbList`, the list field becomes monomorphic growable list (from the list
base class). This makes `add` calls monomorphic and inlinable, and avoids
double mutability checks (once in `PbList.add`, again in `_wrappedList.add`).

Also simplifies immutable `PbMap` allocations.

`PbMap._isReadonly` is renamed to `_isReadOnly` for consistency with
`PbList._isReadOnly`.

protobuf/CHANGELOG.md

@@ -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
 

protobuf/lib/src/protobuf/field_set.dart

@@ -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();

protobuf/lib/src/protobuf/pb_list.dart

@@ -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;
@@ -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;
 

protobuf/lib/src/protobuf/pb_map.dart

@@ -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');
@@ -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();
@@ -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);
@@ -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();