languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-09-05 00:13:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "[vm] Only call .hashCode once when adding to Map and Set"

Browse source 
This reverts commit 438c1ed2ba.

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: https://github.com/dart-lang/sdk/issues/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: https://github.com/dart-lang/sdk/issues/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>
This commit is contained in:
Daco Harkes 2022-05-06 10:24:29 +00:00 committed by Commit Bot
parent b488bd1a75
commit efa7439c16
3 changed files with 8 additions and 85 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

34
runtime/tests/vm/dart/regress_48948_test.dart
View file

@ -1,34 +0,0 @@
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
// 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.
// Regression test for https://github.com/dart-lang/sdk/issues/48948.
import 'package:expect/expect.dart';
class Foo {
const Foo(this.x, this.y);
final int x;
final int y;
static int hashCodeCounter = 0;
@override
int get hashCode {
hashCodeCounter++;
return x.hashCode ^ y.hashCode;
}
}
void main() {
final Map<Foo, int> someMap = {};
final Set<Foo> someSet = {};
Expect.equals(0, Foo.hashCodeCounter);
someMap[Foo(1, 100)] = 2;
Expect.equals(1, Foo.hashCodeCounter);
someSet.add(Foo(1, 100));
Expect.equals(2, Foo.hashCodeCounter);
}

36
runtime/tests/vm/dart_2/regress_48948_test.dart
View file

@ -1,36 +0,0 @@
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
// 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.
// Regression test for https://github.com/dart-lang/sdk/issues/48948.
// @dart = 2.9
import 'package:expect/expect.dart';
class Foo {
const Foo(this.x, this.y);
final int x;
final int y;
static int hashCodeCounter = 0;
@override
int get hashCode {
hashCodeCounter++;
return x.hashCode ^ y.hashCode;
}
}
void main() {
final Map<Foo, int> someMap = {};
final Set<Foo> someSet = {};
Expect.equals(0, Foo.hashCodeCounter);
someMap[Foo(1, 100)] = 2;
Expect.equals(1, Foo.hashCodeCounter);
someSet.add(Foo(1, 100));
Expect.equals(2, Foo.hashCodeCounter);
}

23
sdk/lib/_internal/vm/lib/compact_hash.dart
View file

@ -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);
@ -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;
@ -495,12 +496,8 @@ 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);
@ -508,7 +505,7 @@ mixin _LinkedHashMapMixin<K, V> on _HashBase, _EqualsAndHashCode {
_data[d] = value;
} else {
final int i = -d;
_insert(key, value, fullHash, hashPattern, i);
_insert(key, value, hashPattern, i);
}
}
@ -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;
}
@ -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;
@ -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));