Use an MD5 hash instead of Object.hash for model hashes (#147)

We can easily swap out the actual hash algorithm here now, in one place.

This is a bit slower (unsurprisingly) than using `Object.hash` etc, a few hundred milliseconds in total per iteration, but is a fairer representation of what we actually want.

Author: jakemac53

pkgs/_macro_host/lib/src/macro_cache.dart

@@ -2,6 +2,7 @@
 // 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:crypto/crypto.dart';
 import 'package:dart_model/dart_model.dart';
 import 'package:macro_service/macro_service.dart';
 
@@ -32,14 +33,14 @@ class MacroResultsCache {
       phase: request.phase,
     )] = (
       queries: queryResults.map((q) => q.query),
-      resultsHash:
+      resultsDigest:
           queryResults
               .skip(1)
               .fold(
                 queryResults.first.response,
                 (model, next) => model.mergeWith(next.response),
               )
-              .fingerprint,
+              .digest,
       response: response,
     );
   }
@@ -67,15 +68,15 @@ class MacroResultsCache {
         ),
       ),
     );
-    final newResultsHash =
+    final newResultsDigest =
         queryResults
             .skip(1)
             .fold(
               queryResults.first.model,
               (model, next) => model.mergeWith(next.model),
             )
-            .fingerprint;
-    if (newResultsHash != cached.resultsHash) {
+            .digest;
+    if (newResultsDigest != cached.resultsDigest) {
       _cache.remove(cacheKey);
       return null;
     }
@@ -91,8 +92,8 @@ typedef _MacroResultsCacheValue =
       /// All queries done by a macro in a given phase.
       Iterable<Query> queries,
 
-      /// The `identityHash` of the merged model from all query responses.
-      int resultsHash,
+      /// The [Digest] of the merged model from all query responses.
+      Digest resultsDigest,
 
       /// The macro augmentation response that was cached.
       AugmentResponse response,

pkgs/dart_model/lib/src/dart_model.dart

@@ -2,6 +2,8 @@
 // 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:crypto/crypto.dart';
+
 import 'dart_model.g.dart';
 import 'json_buffer/json_buffer_builder.dart';
 import 'lazy_merged_map.dart';
@@ -31,10 +33,10 @@ extension ModelExtension on Model {
   /// An identity hash for `this`, used for comparing query results.
   ///
   /// TODO: A faster/better implementation?
-  int get fingerprint {
+  Digest get digest {
     // TODO: Implementation for non-buffer maps?
     var node = this.node as MapInBuffer;
-    return node.buffer.fingerprint(
+    return node.buffer.digest(
       node.pointer,
       type: Type.typedMapPointer,
       alreadyDereferenced: true,

pkgs/dart_model/lib/src/json_buffer/closed_list.dart

@@ -75,13 +75,11 @@ class _ClosedList with ListMixin<Object?> {
     throw UnsupportedError('This JsonBufferBuilder list is read-only.');
   }
 
-  int get fingerprint {
-    var iterator = _ClosedListHashIterator(_buffer, _pointer, length);
-    var hash = 0;
+  void buildDigest(ByteConversionSink byteSink) {
+    var iterator = _ClosedListPointerIterator(_buffer, _pointer, length);
     while (iterator.moveNext()) {
-      hash = Object.hash(hash, iterator.current);
+      _buffer._buildDigest(iterator.current, byteSink);
     }
-    return hash;
   }
 }
 
@@ -108,9 +106,9 @@ class _ClosedListIterator<T extends Object?> implements Iterator<T> {
   }
 }
 
-class _ClosedListHashIterator extends _ClosedListIterator<int> {
-  _ClosedListHashIterator(super.buffer, super.pointer, super.length);
+class _ClosedListPointerIterator extends _ClosedListIterator<_Pointer> {
+  _ClosedListPointerIterator(super.buffer, super.pointer, super.length);
 
   @override
-  int get current => _buffer.fingerprint(_pointer);
+  _Pointer get current => _pointer;
 }

pkgs/dart_model/lib/src/json_buffer/closed_map.dart

@@ -123,13 +123,16 @@ class _ClosedMap
   @override
   int get hashCode => Object.hash(buffer, pointer);
 
-  int get fingerprint {
-    var iterator = _ClosedMapHashIterator(buffer, null, pointer, length);
-    var hash = 0;
+  void buildDigest(ByteConversionSink byteSink) {
+    var iterator = _ClosedMapPointerIterator(buffer, null, pointer, length);
     while (iterator.moveNext()) {
-      hash = Object.hash(hash, iterator.current);
+      buffer._buildDigest(
+        iterator.current.$1,
+        byteSink,
+        type: Type.stringPointer,
+      );
+      buffer._buildDigest(iterator.current.$2, byteSink);
     }
-    return hash;
   }
 }
 
@@ -199,17 +202,18 @@ class _ClosedMapEntryIterator
   MapEntry<String, Object?> get current => MapEntry(_currentKey, _currentValue);
 }
 
-class _ClosedMapHashIterator extends _ClosedMapIterator<int> {
-  _ClosedMapHashIterator(
+class _ClosedMapPointerIterator
+    extends _ClosedMapIterator<(int keyPointer, int valuePointer)> {
+  _ClosedMapPointerIterator(
     super._buffer,
     super._parent,
     super.pointer,
     super.length,
   );
 
   @override
-  int get current => Object.hash(
-    _buffer._fingerprint(_pointer, Type.stringPointer),
-    _buffer.fingerprint(_pointer + ClosedMaps._keySize),
+  (int keyPointer, int valuePointer) get current => (
+    _pointer,
+    _pointer + ClosedMaps._keySize,
   );
 }

pkgs/dart_model/lib/src/json_buffer/growable_map.dart

@@ -176,13 +176,16 @@ class _GrowableMap<V>
   @override
   int get hashCode => Object.hash(buffer, pointer);
 
-  int get fingerprint {
-    var iterator = _GrowableMapHashIterator(buffer, null, pointer);
-    var hash = 0;
+  void buildDigest(ByteConversionSink byteSink) {
+    var iterator = _GrowableMapPointerIterator(buffer, null, pointer);
     while (iterator.moveNext()) {
-      hash = Object.hash(hash, iterator.current);
+      buffer._buildDigest(
+        iterator.current.$1,
+        byteSink,
+        type: Type.stringPointer,
+      );
+      buffer._buildDigest(iterator.current.$2, byteSink);
     }
-    return hash;
   }
 }
 
@@ -233,12 +236,13 @@ class _GrowableMapEntryIterator<V>
   MapEntry<String, V> get current => MapEntry(_currentKey, _currentValue as V);
 }
 
-class _GrowableMapHashIterator extends _GrowableMapIterator<int> {
-  _GrowableMapHashIterator(super._buffer, super._parent, super._pointer);
+class _GrowableMapPointerIterator
+    extends _GrowableMapIterator<(int keyPointer, int valuePointer)> {
+  _GrowableMapPointerIterator(super._buffer, super._parent, super._pointer);
 
   @override
-  int get current => Object.hash(
-    _buffer._fingerprint(_pointer + _pointerSize, Type.stringPointer),
-    _buffer.fingerprint(_pointer + _pointerSize + GrowableMaps._keySize),
+  (int keyPointer, int valuePointer) get current => (
+    _pointer + _pointerSize,
+    _pointer + _pointerSize + GrowableMaps._keySize,
   );
 }

pkgs/dart_model/lib/src/json_buffer/json_buffer_builder.dart

@@ -6,6 +6,9 @@ import 'dart:collection';
 import 'dart:convert';
 import 'dart:typed_data';
 
+import 'package:convert/convert.dart';
+import 'package:crypto/crypto.dart';
+
 part 'closed_list.dart';
 part 'closed_map.dart';
 part 'explanations.dart';
@@ -62,29 +65,30 @@ class JsonBufferBuilder {
   /// If [alreadyDereferenced] is `true`, then for types which are pointers,
   /// [pointer] already points at the top of the object, and should not be
   /// followed before reading the object.
-  int fingerprint(int pointer, {Type? type, bool alreadyDereferenced = false}) {
-    if (type == null) {
-      type = _readType(pointer);
-      pointer += _typeSize;
-    }
-    return _fingerprint(
+  Digest digest(int pointer, {Type? type, bool alreadyDereferenced = false}) {
+    var output = AccumulatorSink<Digest>();
+    var input = md5.startChunkedConversion(output);
+    _buildDigest(
       pointer,
-      type,
+      input,
+      type: type,
       alreadyDereferenced: alreadyDereferenced,
     );
+    input.close();
+    return output.events.single;
   }
 
-  /// Computes the identity hash of the object at [pointer] with a known [type]
-  /// from its raw bytes.
-  ///
-  /// If [alreadyDereferenced] is `true`, then for types which are pointers,
-  /// [pointer] already points at the top of the object, and should not be
-  /// followed before reading the object.
-  int _fingerprint(
-    _Pointer pointer,
-    Type type, {
+  /// See [digest].
+  void _buildDigest(
+    int pointer,
+    ByteConversionSink byteSink, {
+    Type? type,
     bool alreadyDereferenced = false,
   }) {
+    if (type == null) {
+      type = _readType(pointer);
+      pointer += _typeSize;
+    }
     // Dereference [pointer] if it is a pointer type, and hasn't already been
     // dereferenced.
     if (type.isPointer && !alreadyDereferenced) {
@@ -93,29 +97,30 @@ class JsonBufferBuilder {
 
     switch (type) {
       case Type.nil:
-        return null.hashCode;
+        return byteSink.add(const [0]);
       case Type.type:
-        return _buffer[pointer];
+        byteSink.addSlice(_buffer, pointer, pointer + _typeSize, false);
       case Type.pointer:
-        return fingerprint(pointer);
+        _buildDigest(pointer, byteSink);
       case Type.uint32:
-        return _readUint32(pointer);
+        byteSink.addSlice(_buffer, pointer, pointer + 4, false);
       case Type.boolean:
-        return _buffer[pointer];
+        // We use [1] and [2] because [0] is `null`.
+        byteSink.add(_readBoolean(pointer) ? const [2] : const [1]);
       case Type.anyPointer:
-        return fingerprint(pointer);
+        _buildDigest(pointer, byteSink);
       case Type.stringPointer:
         final length = _readLength(pointer);
         pointer += _lengthSize;
-        return Object.hashAll(_buffer.sublist(pointer, pointer + length));
+        byteSink.addSlice(_buffer, pointer, pointer + length, false);
       case Type.closedListPointer:
-        return _ClosedList(this, pointer).fingerprint;
+        _ClosedList(this, pointer).buildDigest(byteSink);
       case Type.closedMapPointer:
-        return _ClosedMap(this, pointer, null).fingerprint;
+        return _ClosedMap(this, pointer, null).buildDigest(byteSink);
       case Type.growableMapPointer:
-        return _GrowableMap<Object?>(this, pointer, null).fingerprint;
+        return _GrowableMap<Object?>(this, pointer, null).buildDigest(byteSink);
       case Type.typedMapPointer:
-        return _TypedMap(this, pointer, null).fingerprint;
+        return _TypedMap(this, pointer, null).buildDigest(byteSink);
     }
   }
 

pkgs/dart_model/lib/src/json_buffer/typed_map.dart

@@ -468,19 +468,40 @@ class _TypedMap
   @override
   int get hashCode => Object.hash(buffer, pointer);
 
-  int get fingerprint {
+  /// Up to 16 fields supported, increase this to support more.
+  static final _indexBytes = Uint8List(2);
+
+  void buildDigest(ByteConversionSink byteSink) {
+    assert(
+      _schema._fieldSetSize <= _indexBytes.length,
+      'Update indexBytes to support schemas with more fields',
+    );
+
     // Note: we could include the schema but don't need to. If something can
     // be one of multiple types, that type will be included in a `type` field
-    // in the map.
-    var hash = 0;
-    final iterator =
-        _schema._isAllBooleans
-            ? _AllBoolsTypedMapHashIterator(this)
-            : _PartialTypedMapHashIterator(this);
-    while (iterator.moveNext()) {
-      hash = Object.hash(hash, iterator.current);
+    // in the map
+    if (_schema._isAllBooleans) {
+      var iterator = _AllBoolsTypedMapPointerIterator(this);
+      while (iterator.moveNext()) {
+        _indexBytes[0] = iterator.current.$1;
+        _indexBytes[1] = iterator.current.$1 >> 8;
+        byteSink.add(_indexBytes);
+        // We use [1] and [2] because [0] is `null`.
+        byteSink.add(iterator.current.$2 ? const [2] : const [1]);
+      }
+    } else {
+      var iterator = _PartialTypedMapPointerIterator(this);
+      while (iterator.moveNext()) {
+        _indexBytes[0] = iterator.current.$1;
+        _indexBytes[1] = iterator.current.$1 >> 8;
+        byteSink.add(_indexBytes);
+        buffer._buildDigest(
+          iterator.current.$2,
+          byteSink,
+          type: iterator.current.$3,
+        );
+      }
     }
-    return hash;
   }
 }
 
@@ -557,13 +578,15 @@ class _PartialTypedMapEntryIterator
   MapEntry<String, Object?> get current => MapEntry(_currentKey, _currentValue);
 }
 
-class _PartialTypedMapHashIterator extends _PartialTypedMapIterator<int> {
-  _PartialTypedMapHashIterator(super._map);
+class _PartialTypedMapPointerIterator
+    extends _PartialTypedMapIterator<(int index, int valuePointer, Type type)> {
+  _PartialTypedMapPointerIterator(super._map);
 
   @override
-  int get current => Object.hash(
-    _currentKey,
-    _buffer._fingerprint(_valuesPointer + _offset, _schema._valueTypes[_index]),
+  (int index, int valuePointer, Type type) get current => (
+    _index,
+    _valuesPointer + _offset,
+    _schema._valueTypes[_index],
   );
 }
 
@@ -596,7 +619,7 @@ abstract class _AllBoolsTypedMapIterator<T> implements Iterator<T> {
   T get current;
 
   String get _currentKey => _schema._keys[_index];
-  Object? get _currentValue =>
+  bool get _currentValue =>
       _buffer._readBit(_valuesPointer + _intOffset, _bitOffset);
 
   @override
@@ -631,7 +654,7 @@ class _AllBoolsTypedMapValueIterator
   _AllBoolsTypedMapValueIterator(super._map);
 
   @override
-  Object? get current => _currentValue;
+  bool get current => _currentValue;
 }
 
 class _AllBoolsTypedMapEntryIterator
@@ -642,9 +665,10 @@ class _AllBoolsTypedMapEntryIterator
   MapEntry<String, Object?> get current => MapEntry(_currentKey, _currentValue);
 }
 
-class _AllBoolsTypedMapHashIterator extends _AllBoolsTypedMapIterator<int> {
-  _AllBoolsTypedMapHashIterator(super._map);
+class _AllBoolsTypedMapPointerIterator
+    extends _AllBoolsTypedMapIterator<(int index, bool value)> {
+  _AllBoolsTypedMapPointerIterator(super._map);
 
   @override
-  int get current => Object.hash(_currentKey, _currentValue);
+  (int index, bool value) get current => (_index, _currentValue);
 }