[vm/dart2wasm] Brush up mixin structure of hash implementation classes

This changes all mixins in the VM's hash map/set implementation to be
declared as mixins and introduces appropriate interfaces needed to
concisely express the prerequisites of the various mixins in the stack.

It also separates most of the implementation of immutable maps and sets
into mixins to enable sharing the implementation with dart2wasm.

Tested: ci + manual testing of dart2wasm
Change-Id: Ife1e7195aba35741629aa4dffedb929b1028a85a
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/236180
Reviewed-by: Daco Harkes <dacoharkes@google.com>

pkg/dart2wasm/lib/translator.dart

@@ -190,7 +190,6 @@ class Translator {
     mapFactory = lookupCollection("LinkedHashMap").procedures.firstWhere(
         (p) => p.kind == ProcedureKind.Factory && p.name.text == "_default");
     mapPut = lookupCollection("_CompactLinkedCustomHashMap")
-        .superclass! // _HashBase
         .superclass! // _LinkedHashMapMixin<K, V>
         .procedures
         .firstWhere((p) => p.name.text == "[]=");

pkg/vm/testcases/transformations/type_flow/transformer/class_generics_case1.dart.expect

@@ -15,7 +15,7 @@ class InheritedElement extends self::Element {
     : self::InheritedElement::_dependents = <self::Element, core::Object?>{}, super self::Element::•()
     ;
 [@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:2,getterSelectorId:3]  method setDependencies([@vm.inferred-type.metadata=!] self::Element dependent, [@vm.inferred-type.metadata=dart.core::_Smi?] core::Object? value) → void {
-    [@vm.call-site-attributes.metadata=receiverType:dart.core::Map<#lib::Element, dart.core::Object?>] [@vm.direct-call.metadata=dart.collection::__InternalLinkedHashMap&_HashVMBase&MapMixin&_LinkedHashMapMixin.[]=] [@vm.inferred-type.metadata=!? (skip check)] [@vm.direct-call.metadata=#lib::InheritedElement._dependents] [@vm.inferred-type.metadata=dart.collection::_InternalLinkedHashMap<#lib::Element, dart.core::Object?>] this.{self::InheritedElement::_dependents}{core::Map<self::Element, core::Object?>}.{core::Map::[]=}(dependent, value){(self::Element, core::Object?) → void};
+    [@vm.call-site-attributes.metadata=receiverType:dart.core::Map<#lib::Element, dart.core::Object?>] [@vm.direct-call.metadata=dart.collection::__InternalLinkedHashMap&_HashVMBase&MapMixin&_HashBase&_OperatorEqualsAndHashCode&_LinkedHashMapMixin.[]=] [@vm.inferred-type.metadata=!? (skip check)] [@vm.direct-call.metadata=#lib::InheritedElement._dependents] [@vm.inferred-type.metadata=dart.collection::_InternalLinkedHashMap<#lib::Element, dart.core::Object?>] this.{self::InheritedElement::_dependents}{core::Map<self::Element, core::Object?>}.{core::Map::[]=}(dependent, value){(self::Element, core::Object?) → void};
   }
 }
 static method main() → dynamic {

pkg/vm/testcases/transformations/type_flow/transformer/const_map.dart.expect

@@ -11,7 +11,7 @@ class _Attribute extends core::Object {
     : super core::Object::•()
     ;
   static method fromReader() → self::_Attribute {
-    final self::_AttributeName name = [@vm.direct-call.metadata=dart.collection::_InternalImmutableLinkedHashMap.[]] [@vm.inferred-type.metadata=#lib::_AttributeName? (skip check)] #C8.{core::Map::[]}(#C1){(core::Object?) → self::_AttributeName?}!;
+    final self::_AttributeName name = [@vm.direct-call.metadata=dart.collection::__InternalImmutableLinkedHashMap&_HashVMImmutableBase&MapMixin&_HashBase&_OperatorEqualsAndCanonicalHashCode&_LinkedHashMapMixin&_UnmodifiableMapMixin&_ImmutableLinkedHashMapMixin.[]] [@vm.inferred-type.metadata=#lib::_AttributeName? (skip check)] #C8.{core::Map::[]}(#C1){(core::Object?) → self::_AttributeName?}!;
     return new self::_Attribute::_();
   }
 }

pkg/vm/testcases/transformations/type_flow/transformer/const_set.dart.expect

@@ -11,7 +11,7 @@ class _Attribute extends core::Object {
     : super core::Object::•()
     ;
   static method fromReader() → self::_Attribute {
-    final core::bool name = [@vm.direct-call.metadata=dart.collection::_CompactImmutableLinkedHashSet.contains] [@vm.inferred-type.metadata=!? (skip check)] #C3.{core::Set::contains}(#C2){(core::Object?) → core::bool};
+    final core::bool name = [@vm.direct-call.metadata=dart.collection::__CompactImmutableLinkedHashSet&_HashVMImmutableBase&SetMixin&_HashBase&_OperatorEqualsAndCanonicalHashCode&_LinkedHashSetMixin&_UnmodifiableSetMixin&_ImmutableLinkedHashSetMixin.contains] [@vm.inferred-type.metadata=!? (skip check)] #C3.{core::Set::contains}(#C2){(core::Object?) → core::bool};
     return let final self::_AttributeName #t1 = #C8.{core::List::[]}(#C2){(core::int) → self::_AttributeName} in new self::_Attribute::_();
   }
 }

sdk/lib/_internal/vm/lib/compact_hash.dart

@@ -16,7 +16,29 @@ void _rehashObjects(List objects) {
   }
 }
 
-abstract class _HashFieldBase {
+// Common interface for [_HashFieldBase] and [_HashVMBase].
+abstract class _HashAbstractBase {
+  Uint32List get _index;
+  void set _index(Uint32List value);
+
+  int get _hashMask;
+  void set _hashMask(int value);
+
+  List get _data;
+  void set _data(List value);
+
+  int get _usedData;
+  void set _usedData(int value);
+
+  int get _deletedKeys;
+  void set _deletedKeys(int value);
+}
+
+abstract class _HashAbstractImmutableBase extends _HashAbstractBase {
+  Uint32List? get _indexNullable;
+}
+
+abstract class _HashFieldBase implements _HashAbstractBase {
   // Each occupied entry in _index is a fixed-size integer that encodes a pair:
   //   [ hash pattern for key | index of entry in _data ]
   // The hash pattern is based on hashCode, but is guaranteed to be non-zero.
@@ -43,11 +65,11 @@ abstract class _HashFieldBase {
   // Note: All fields are initialized in a single constructor so that the VM
   // recognizes they cannot hold null values. This makes a big (20%) performance
   // difference on some operations.
-  _HashFieldBase(int dataSize);
+  _HashFieldBase();
 }
 
 // Base class for VM-internal classes; keep in sync with _HashFieldBase.
-abstract class _HashVMBase {
+abstract class _HashVMBase implements _HashAbstractBase {
   @pragma("vm:recognized", "other")
   @pragma("vm:exact-result-type", "dart:typed_data#_Uint32List")
   @pragma("vm:prefer-inline")
@@ -99,8 +121,9 @@ abstract class _HashVMBase {
   external void set _deletedKeys(int value);
 }
 
-// Base class for VM-internal classes; keep in sync with _HashFieldBase.
-abstract class _HashVMImmutableBase extends _HashVMBase {
+// Base class for immutable VM-internal classes.
+abstract class _HashVMImmutableBase extends _HashVMBase
+    implements _HashAbstractImmutableBase {
   // The data is an immutable list rather than a mutable list.
   @pragma("vm:recognized", "other")
   @pragma("vm:exact-result-type", "dart:core#_ImmutableList")
@@ -123,8 +146,7 @@ abstract class _HashVMImmutableBase extends _HashVMBase {
 // This mixin can be applied to _HashFieldBase or _HashVMBase (for
 // normal and VM-internalized classes, respectiveley), which provide the
 // actual fields/accessors that this mixin assumes.
-// TODO(koda): Consider moving field comments to _HashFieldBase.
-abstract class _HashBase implements _HashVMBase {
+mixin _HashBase on _HashAbstractBase {
   // The number of bits used for each component is determined by table size.
   // If initialized, the length of _index is (at least) twice the number of
   // entries in _data, and both are doubled when _data is full. Thus, _index
@@ -208,17 +230,22 @@ abstract class _HashBase implements _HashVMBase {
   }
 }
 
-class _OperatorEqualsAndHashCode {
+abstract class _EqualsAndHashCode {
+  int _hashCode(e);
+  bool _equals(e1, e2);
+}
+
+mixin _OperatorEqualsAndHashCode implements _EqualsAndHashCode {
   int _hashCode(e) => e.hashCode;
   bool _equals(e1, e2) => e1 == e2;
 }
 
-class _IdenticalAndIdentityHashCode {
+mixin _IdenticalAndIdentityHashCode implements _EqualsAndHashCode {
   int _hashCode(e) => identityHashCode(e);
   bool _equals(e1, e2) => identical(e1, e2);
 }
 
-class _OperatorEqualsAndCanonicalHashCode {
+mixin _OperatorEqualsAndCanonicalHashCode implements _EqualsAndHashCode {
   static final int cidSymbol = ClassID.getID(#a);
 
   int _hashCode(e) {
@@ -232,6 +259,14 @@ class _OperatorEqualsAndCanonicalHashCode {
   bool _equals(e1, e2) => e1 == e2;
 }
 
+mixin _CustomEqualsAndHashCode implements _EqualsAndHashCode {
+  dynamic get _hasher;
+  dynamic get _equality;
+
+  int _hashCode(e) => _hasher(e);
+  bool _equals(e1, e2) => _equality(e1, e2);
+}
+
 final _uninitializedIndex = new Uint32List(_HashBase._UNINITIALIZED_INDEX_SIZE);
 // Note: not const. Const arrays are made immutable by having a different class
 // than regular arrays that throws on element assignment. We want the data field
@@ -243,9 +278,9 @@ final _uninitializedData = new List.filled(0, null);
 class _InternalLinkedHashMap<K, V> extends _HashVMBase
     with
         MapMixin<K, V>,
-        _LinkedHashMapMixin<K, V>,
         _HashBase,
-        _OperatorEqualsAndHashCode
+        _OperatorEqualsAndHashCode,
+        _LinkedHashMapMixin<K, V>
     implements LinkedHashMap<K, V> {
   _InternalLinkedHashMap() {
     _index = _uninitializedIndex;
@@ -273,15 +308,19 @@ class _InternalLinkedHashMap<K, V> extends _HashVMBase
 class _InternalImmutableLinkedHashMap<K, V> extends _HashVMImmutableBase
     with
         MapMixin<K, V>,
-        _LinkedHashMapMixin<K, V>,
         _HashBase,
         _OperatorEqualsAndCanonicalHashCode,
-        _UnmodifiableMapMixin<K, V>
+        _LinkedHashMapMixin<K, V>,
+        _UnmodifiableMapMixin<K, V>,
+        _ImmutableLinkedHashMapMixin<K, V>
     implements LinkedHashMap<K, V> {
   factory _InternalImmutableLinkedHashMap._uninstantiable() {
     throw new UnsupportedError("ImmutableMap can only be allocated by the VM");
   }
+}
 
+mixin _ImmutableLinkedHashMapMixin<K, V>
+    on _LinkedHashMapMixin<K, V>, _HashAbstractImmutableBase {
   bool containsKey(Object? key) {
     if (_indexNullable == null) {
       _createIndex();
@@ -345,12 +384,7 @@ int _roundUpToPowerOfTwo(int x) {
   return x + 1;
 }
 
-abstract class _LinkedHashMapMixin<K, V> implements _HashBase {
-  int _hashCode(e);
-  bool _equals(e1, e2);
-  int get _checkSum;
-  bool _isModifiedSince(List oldData, int oldCheckSum);
-
+mixin _LinkedHashMapMixin<K, V> on _HashBase, _EqualsAndHashCode {
   int get length => (_usedData >> 1) - _deletedKeys;
   bool get isEmpty => length == 0;
   bool get isNotEmpty => !isEmpty;
@@ -592,12 +626,10 @@ abstract class _LinkedHashMapMixin<K, V> implements _HashBase {
 class _CompactLinkedIdentityHashMap<K, V> extends _HashFieldBase
     with
         MapMixin<K, V>,
-        _LinkedHashMapMixin<K, V>,
         _HashBase,
-        _IdenticalAndIdentityHashCode
+        _IdenticalAndIdentityHashCode,
+        _LinkedHashMapMixin<K, V>
     implements LinkedHashMap<K, V> {
-  _CompactLinkedIdentityHashMap() : super(_HashBase._INITIAL_INDEX_SIZE);
-
   void addAll(Map<K, V> other) {
     if (other is _CompactLinkedIdentityHashMap) {
       final otherBase = other as _CompactLinkedIdentityHashMap;
@@ -610,15 +642,15 @@ class _CompactLinkedIdentityHashMap<K, V> extends _HashFieldBase
 }
 
 class _CompactLinkedCustomHashMap<K, V> extends _HashFieldBase
-    with MapMixin<K, V>, _LinkedHashMapMixin<K, V>, _HashBase
+    with
+        MapMixin<K, V>,
+        _HashBase,
+        _CustomEqualsAndHashCode,
+        _LinkedHashMapMixin<K, V>
     implements LinkedHashMap<K, V> {
-  final _equality;
-  final _hasher;
-  final _validKey;
-
-  // TODO(koda): Ask gbracha why I cannot have fields _equals/_hashCode.
-  int _hashCode(e) => _hasher(e);
-  bool _equals(e1, e2) => _equality(e1, e2);
+  final dynamic _equality;
+  final dynamic _hasher;
+  final dynamic _validKey;
 
   bool containsKey(Object? o) => _validKey(o) ? super.containsKey(o) : false;
   V? operator [](Object? o) => _validKey(o) ? super[o] : null;
@@ -628,8 +660,7 @@ class _CompactLinkedCustomHashMap<K, V> extends _HashFieldBase
   void operator []=(K key, V value);
 
   _CompactLinkedCustomHashMap(this._equality, this._hasher, validKey)
-      : _validKey = (validKey != null) ? validKey : new _TypeTest<K>().test,
-        super(_HashBase._INITIAL_INDEX_SIZE);
+      : _validKey = (validKey != null) ? validKey : new _TypeTest<K>().test;
 }
 
 // Iterates through _data[_offset + _step], _data[_offset + 2*_step], ...
@@ -739,12 +770,7 @@ class _CompactIteratorImmutable<E> implements Iterator<E> {
   E get current => _current as E;
 }
 
-abstract class _LinkedHashSetMixin<E> implements _HashBase {
-  int _hashCode(e);
-  bool _equals(e1, e2);
-  int get _checkSum;
-  bool _isModifiedSince(List oldData, int oldCheckSum);
-
+mixin _LinkedHashSetMixin<E> on _HashBase, _EqualsAndHashCode {
   bool get isEmpty => length == 0;
   bool get isNotEmpty => !isEmpty;
   int get length => _usedData - _deletedKeys;
@@ -910,14 +936,14 @@ abstract class _LinkedHashSetMixin<E> implements _HashBase {
   }
 }
 
-// Set implementation, analogous to _CompactLinkedHashMap.
+// Set implementation, analogous to _InternalLinkedHashMap.
 @pragma('vm:entry-point')
 class _CompactLinkedHashSet<E> extends _HashVMBase
     with
         SetMixin<E>,
-        _LinkedHashSetMixin<E>,
         _HashBase,
-        _OperatorEqualsAndHashCode
+        _OperatorEqualsAndHashCode,
+        _LinkedHashSetMixin<E>
     implements LinkedHashSet<E> {
   _CompactLinkedHashSet() {
     _index = _uninitializedIndex;
@@ -951,15 +977,19 @@ class _CompactLinkedHashSet<E> extends _HashVMBase
 class _CompactImmutableLinkedHashSet<E> extends _HashVMImmutableBase
     with
         SetMixin<E>,
-        _LinkedHashSetMixin<E>,
         _HashBase,
         _OperatorEqualsAndCanonicalHashCode,
-        _UnmodifiableSetMixin<E>
+        _LinkedHashSetMixin<E>,
+        _UnmodifiableSetMixin<E>,
+        _ImmutableLinkedHashSetMixin<E>
     implements LinkedHashSet<E> {
   factory _CompactImmutableLinkedHashSet._uninstantiable() {
     throw new UnsupportedError("ImmutableSet can only be allocated by the VM");
   }
+}
 
+mixin _ImmutableLinkedHashSetMixin<E>
+    on Set<E>, _LinkedHashSetMixin<E>, _HashAbstractImmutableBase {
   E? lookup(Object? key) {
     if (_indexNullable == null) {
       _createIndex();
@@ -1029,12 +1059,10 @@ class _CompactImmutableLinkedHashSet<E> extends _HashVMImmutableBase
 class _CompactLinkedIdentityHashSet<E> extends _HashFieldBase
     with
         SetMixin<E>,
-        _LinkedHashSetMixin<E>,
         _HashBase,
-        _IdenticalAndIdentityHashCode
+        _IdenticalAndIdentityHashCode,
+        _LinkedHashSetMixin<E>
     implements LinkedHashSet<E> {
-  _CompactLinkedIdentityHashSet() : super(_HashBase._INITIAL_INDEX_SIZE);
-
   Set<E> toSet() => new _CompactLinkedIdentityHashSet<E>()..addAll(this);
 
   static Set<R> _newEmpty<R>() => new _CompactLinkedIdentityHashSet<R>();
@@ -1053,22 +1081,22 @@ class _CompactLinkedIdentityHashSet<E> extends _HashFieldBase
 }
 
 class _CompactLinkedCustomHashSet<E> extends _HashFieldBase
-    with SetMixin<E>, _LinkedHashSetMixin<E>, _HashBase
+    with
+        SetMixin<E>,
+        _HashBase,
+        _CustomEqualsAndHashCode,
+        _LinkedHashSetMixin<E>
     implements LinkedHashSet<E> {
-  final _equality;
-  final _hasher;
-  final _validKey;
-
-  int _hashCode(e) => _hasher(e);
-  bool _equals(e1, e2) => _equality(e1, e2);
+  final dynamic _equality;
+  final dynamic _hasher;
+  final dynamic _validKey;
 
   bool contains(Object? o) => _validKey(o) ? super.contains(o) : false;
   E? lookup(Object? o) => _validKey(o) ? super.lookup(o) : null;
   bool remove(Object? o) => _validKey(o) ? super.remove(o) : false;
 
   _CompactLinkedCustomHashSet(this._equality, this._hasher, validKey)
-      : _validKey = (validKey != null) ? validKey : new _TypeTest<E>().test,
-        super(_HashBase._INITIAL_INDEX_SIZE);
+      : _validKey = (validKey != null) ? validKey : new _TypeTest<E>().test;
 
   Set<R> cast<R>() => Set.castFrom<E, R>(this);
   Set<E> toSet() =>