Skip to content

Commit

Permalink
Revert "[vm] Only call .hashCode once when adding to Map and Set"
Browse files Browse the repository at this point in the history
This reverts commit 438c1ed.

Reason for revert: b/231617607 and b/230945329.
Will reland after b/230945329 is addressed.

Original change's description:
> [vm] Only call `.hashCode` once when adding to `Map` and `Set`
>
> The methods to add to hash maps and hash sets are recursive: if the
> index needs to be rehashed then the same method is called again after
> rehashing.
>
> This CL nests the actual implementation in a private method that takes
> the full hashCode as an extra argument.
>
> No significant code size or run time changes are reported on our
> benchmarks. (Our benchmarks do not contain purposefully slow hashCodes.)
>
> Note that hashCode can be called again later if rehashing of the index
> is required on adding subsequent elements.
>
> Bug: #48948
> Change-Id: Ia3ccff9e592d675b4922ac78c4aa7ee0287ecb50
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/243623
> Reviewed-by: Martin Kustermann <kustermann@google.com>
> Commit-Queue: Daco Harkes <dacoharkes@google.com>

TBR=kustermann@google.com,dacoharkes@google.com

Change-Id: I214251b65ea89e7f729564a125e226f2e6d580c0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: #48948
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/243900
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
  • Loading branch information
dcharkes authored and Commit Bot committed May 6, 2022
1 parent b488bd1 commit efa7439
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 85 deletions.
34 changes: 0 additions & 34 deletions runtime/tests/vm/dart/regress_48948_test.dart

This file was deleted.

36 changes: 0 additions & 36 deletions runtime/tests/vm/dart_2/regress_48948_test.dart

This file was deleted.

23 changes: 8 additions & 15 deletions sdk/lib/_internal/vm/lib/compact_hash.dart
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ mixin _ImmutableLinkedHashMapMixin<K, V>

for (int j = 0; j < _usedData; j += 2) {
final key = _data[j];
final value = _data[j + 1];

final fullHash = _hashCode(key);
final hashPattern = _HashBase._hashPattern(fullHash, hashMask, size);
Expand Down Expand Up @@ -451,10 +452,10 @@ mixin _LinkedHashMapMixin<K, V> on _HashBase, _EqualsAndHashCode {
}
}

void _insert(K key, V value, int fullHash, int hashPattern, int i) {
void _insert(K key, V value, int hashPattern, int i) {
if (_usedData == _data.length) {
_rehash();
_set(key, value, fullHash);
this[key] = value;
} else {
assert(1 <= hashPattern && hashPattern < (1 << 32));
final int index = _usedData >> 1;
Expand Down Expand Up @@ -495,20 +496,16 @@ mixin _LinkedHashMapMixin<K, V> on _HashBase, _EqualsAndHashCode {
}

void operator []=(K key, V value) {
final int fullHash = _hashCode(key);
_set(key, value, fullHash);
}

void _set(K key, V value, int fullHash) {
final int size = _index.length;
final int fullHash = _hashCode(key);
final int hashPattern = _HashBase._hashPattern(fullHash, _hashMask, size);
final int d =
_findValueOrInsertPoint(key, fullHash, hashPattern, size, _index);
if (d > 0) {
_data[d] = value;
} else {
final int i = -d;
_insert(key, value, fullHash, hashPattern, i);
_insert(key, value, hashPattern, i);
}
}

Expand All @@ -529,7 +526,7 @@ mixin _LinkedHashMapMixin<K, V> on _HashBase, _EqualsAndHashCode {
this[key] = value;
} else {
final int i = -d;
_insert(key, value, fullHash, hashPattern, i);
_insert(key, value, hashPattern, i);
}
return value;
}
Expand Down Expand Up @@ -838,14 +835,10 @@ mixin _LinkedHashSetMixin<E> on _HashBase, _EqualsAndHashCode {
}

bool add(E key) {
final int fullHash = _hashCode(key);
return _add(key, fullHash);
}

bool _add(E key, int fullHash) {
final int size = _index.length;
final int sizeMask = size - 1;
final int maxEntries = size >> 1;
final int fullHash = _hashCode(key);
final int hashPattern = _HashBase._hashPattern(fullHash, _hashMask, size);
int i = _HashBase._firstProbe(fullHash, sizeMask);
int firstDeleted = -1;
Expand All @@ -866,7 +859,7 @@ mixin _LinkedHashSetMixin<E> on _HashBase, _EqualsAndHashCode {
}
if (_usedData == _data.length) {
_rehash();
_add(key, fullHash);
add(key);
} else {
final int insertionPoint = (firstDeleted >= 0) ? firstDeleted : i;
assert(1 <= hashPattern && hashPattern < (1 << 32));
Expand Down

0 comments on commit efa7439

Please sign in to comment.