Skip to content

Commit 23a98d8

Browse files
rakudramaCommit Queue
rakudrama
authored and
Commit Queue
committed
[vm, wasm] Separate Iterables for _Map.{keys,values,entries}
The Iterables have been separated and specialized. The `keys` Iterable specialized so that the `contains` method uses the Map's `containsKey` method. This is required for correctness and usually much faster that the linear scan of `Iterable.contains`. The default `MapBase.entries` maps over the keys and does lookups. It is faster to scan the data like `get keys` and `get values`. Issue: #52909 Change-Id: Id3400c3efd8e657c74b41b3fce71e0713dc790bc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/389266 Reviewed-by: Lasse Nielsen <[email protected]> Reviewed-by: Ömer Ağacan <[email protected]> Commit-Queue: Stephen Adams <[email protected]>
1 parent 7f31e31 commit 23a98d8

File tree

3 files changed

+252
-43
lines changed

3 files changed

+252
-43
lines changed

‎sdk/lib/_internal/vm_shared/lib/compact_hash.dart‎

Lines changed: 90 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,14 @@ library dart._compact_hash;
55

66
import "dart:_internal" as internal;
77

8-
import "dart:_internal" show patch, IterableElementError, ClassID, TypeTest;
8+
import "dart:_internal"
9+
show
10+
patch,
11+
EfficientLengthIterable,
12+
HideEfficientLengthIterable,
13+
IterableElementError,
14+
ClassID,
15+
TypeTest;
916

1017
import "dart:collection" show MapMixin, SetMixin, LinkedHashMap, LinkedHashSet;
1118

@@ -753,8 +760,9 @@ mixin _LinkedHashMapMixin<K, V> on _HashBase, _EqualsAndHashCode {
753760
}
754761
}
755762

756-
Iterable<K> get keys => _CompactIterable<K>(this, -2, 2);
757-
Iterable<V> get values => _CompactIterable<V>(this, -1, 2);
763+
Iterable<K> get keys => _CompactKeysIterable<K>(this);
764+
Iterable<V> get values => _CompactValuesIterable<V>(this);
765+
Iterable<MapEntry<K, V>> get entries => _CompactEntriesIterable<K, V>(this);
758766
}
759767

760768
base class CompactLinkedIdentityHashMap<K, V> extends _HashFieldBase
@@ -796,28 +804,38 @@ base class CompactLinkedCustomHashMap<K, V> extends _HashFieldBase
796804
) : _validKey = validKey ?? TypeTest<K>().test;
797805
}
798806

799-
// Iterates through _data[_offset + _step], _data[_offset + 2*_step], ...
800-
// and checks for concurrent modification.
801-
class _CompactIterable<E> extends Iterable<E> {
807+
class _CompactKeysIterable<E> extends Iterable<E>
808+
implements EfficientLengthIterable<E>, HideEfficientLengthIterable<E> {
809+
final _LinkedHashMapMixin _table;
810+
811+
_CompactKeysIterable(this._table);
812+
813+
Iterator<E> get iterator =>
814+
_CompactIterator<E>(_table, _table._data, _table._usedData, -2, 2);
815+
816+
int get length => _table.length;
817+
bool get isEmpty => length == 0;
818+
bool get isNotEmpty => !isEmpty;
819+
820+
bool contains(Object? element) => _table.containsKey(element);
821+
}
822+
823+
class _CompactValuesIterable<E> extends Iterable<E>
824+
implements EfficientLengthIterable<E>, HideEfficientLengthIterable<E> {
802825
final _HashBase _table;
803-
final int _offset;
804-
final int _step;
805826

806-
_CompactIterable(this._table, this._offset, this._step);
827+
_CompactValuesIterable(this._table);
807828

808-
Iterator<E> get iterator => _CompactIterator<E>(
809-
_table,
810-
_table._data,
811-
_table._usedData,
812-
_offset,
813-
_step,
814-
);
829+
Iterator<E> get iterator =>
830+
_CompactIterator<E>(_table, _table._data, _table._usedData, -1, 2);
815831

816832
int get length => _table.length;
817833
bool get isEmpty => length == 0;
818834
bool get isNotEmpty => !isEmpty;
819835
}
820836

837+
// Iterates through _data[_offset + _step], _data[_offset + 2*_step], ...
838+
// and checks for concurrent modification.
821839
class _CompactIterator<E> implements Iterator<E> {
822840
final _HashBase _table;
823841
// dart:core#_List (sdk/lib/_internal/vm/lib/array.dart).
@@ -829,11 +847,11 @@ class _CompactIterator<E> implements Iterator<E> {
829847
E? _current;
830848

831849
_CompactIterator(this._table, this._data, this._len, this._offset, this._step)
832-
: _checkSum = _table._checkSum;
850+
: _checkSum = _table._checkSum;
833851

834852
bool moveNext() {
835853
if (_table._isModifiedSince(_data, _checkSum)) {
836-
throw new ConcurrentModificationError(_table);
854+
throw ConcurrentModificationError(_table);
837855
}
838856
do {
839857
_offset += _step;
@@ -850,6 +868,58 @@ class _CompactIterator<E> implements Iterator<E> {
850868
E get current => _current as E;
851869
}
852870

871+
// Iterates through map creating a MapEntry for each key-value pair, and checks
872+
// for concurrent modification.
873+
class _CompactEntriesIterable<K, V> extends Iterable<MapEntry<K, V>>
874+
implements
875+
EfficientLengthIterable<MapEntry<K, V>>,
876+
HideEfficientLengthIterable<MapEntry<K, V>> {
877+
final _HashBase _table;
878+
879+
_CompactEntriesIterable(this._table);
880+
881+
Iterator<MapEntry<K, V>> get iterator =>
882+
_CompactEntriesIterator<K, V>(_table, _table._data, _table._usedData);
883+
884+
int get length => _table.length;
885+
bool get isEmpty => length == 0;
886+
bool get isNotEmpty => !isEmpty;
887+
}
888+
889+
class _CompactEntriesIterator<K, V> implements Iterator<MapEntry<K, V>> {
890+
final _HashBase _table;
891+
// dart:core#_List (sdk/lib/_internal/vm/lib/array.dart).
892+
final List _data;
893+
final int _len;
894+
int _offset = -2;
895+
final int _checkSum;
896+
MapEntry<K, V>? _current;
897+
898+
_CompactEntriesIterator(this._table, this._data, this._len)
899+
: _checkSum = _table._checkSum;
900+
901+
bool moveNext() {
902+
if (_table._isModifiedSince(_data, _checkSum)) {
903+
throw ConcurrentModificationError(_table);
904+
}
905+
do {
906+
_offset += 2;
907+
} while (_offset < _len && _HashBase._isDeleted(_data, _data[_offset]));
908+
if (_offset < _len) {
909+
final key = internal.unsafeCast<K>(_data[_offset]);
910+
final value = internal.unsafeCast<V>(_data[_offset + 1]);
911+
_current = MapEntry<K, V>(key, value);
912+
return true;
913+
} else {
914+
_current = null;
915+
return false;
916+
}
917+
}
918+
919+
MapEntry<K, V> get current =>
920+
_current ?? (throw IterableElementError.noElement());
921+
}
922+
853923
// Iterates through _data[_offset + _step], _data[_offset + 2*_step], ...
854924
//
855925
// Does not check for concurrent modification since the table
@@ -1245,9 +1315,8 @@ base class CompactLinkedCustomHashSet<E> extends _HashFieldBase
12451315
) : _validKey = validKey ?? TypeTest<E>().test;
12461316

12471317
Set<R> cast<R>() => Set.castFrom<E, R>(this);
1248-
Set<E> toSet() =>
1249-
CompactLinkedCustomHashSet<E>(_equality, _hasher, _validKey)
1250-
..addAll(this);
1318+
Set<E> toSet() => CompactLinkedCustomHashSet<E>(_equality, _hasher, _validKey)
1319+
..addAll(this);
12511320
}
12521321

12531322
/// Expose [_Map] as [DefaultMap] and [_Set] as [DefaultSet] so that

‎sdk/lib/_internal/wasm/lib/compact_hash.dart‎

Lines changed: 90 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@
33
// BSD-style license that can be found in the LICENSE file.
44

55
import "dart:_internal"
6-
show IterableElementError, ClassID, TypeTest, unsafeCast;
6+
show
7+
EfficientLengthIterable,
8+
HideEfficientLengthIterable,
9+
IterableElementError,
10+
ClassID,
11+
TypeTest,
12+
unsafeCast;
713
import "dart:_list" show GrowableList;
814
import "dart:_wasm";
915

@@ -670,8 +676,9 @@ mixin _LinkedHashMapMixin<K, V> on _HashBase, _EqualsAndHashCode {
670676
}
671677
}
672678

673-
Iterable<K> get keys => _CompactIterable<K>(this, -2, 2);
674-
Iterable<V> get values => _CompactIterable<V>(this, -1, 2);
679+
Iterable<K> get keys => _CompactKeysIterable<K>(this);
680+
Iterable<V> get values => _CompactValuesIterable<V>(this);
681+
Iterable<MapEntry<K, V>> get entries => _CompactEntriesIterable<K, V>(this);
675682
}
676683

677684
base class CompactLinkedIdentityHashMap<K, V> extends _HashFieldBase
@@ -713,28 +720,38 @@ base class CompactLinkedCustomHashMap<K, V> extends _HashFieldBase
713720
) : _validKey = validKey ?? TypeTest<K>().test;
714721
}
715722

716-
// Iterates through _data[_offset + _step], _data[_offset + 2*_step], ...
717-
// and checks for concurrent modification.
718-
class _CompactIterable<E> extends Iterable<E> {
723+
class _CompactKeysIterable<E> extends Iterable<E>
724+
implements EfficientLengthIterable<E>, HideEfficientLengthIterable<E> {
725+
final _LinkedHashMapMixin _table;
726+
727+
_CompactKeysIterable(this._table);
728+
729+
Iterator<E> get iterator =>
730+
_CompactIterator<E>(_table, _table._data, _table._usedData, -2, 2);
731+
732+
int get length => _table.length;
733+
bool get isEmpty => length == 0;
734+
bool get isNotEmpty => !isEmpty;
735+
736+
bool contains(Object? element) => _table.containsKey(element);
737+
}
738+
739+
class _CompactValuesIterable<E> extends Iterable<E>
740+
implements EfficientLengthIterable<E>, HideEfficientLengthIterable<E> {
719741
final _HashBase _table;
720-
final int _offset;
721-
final int _step;
722742

723-
_CompactIterable(this._table, this._offset, this._step);
743+
_CompactValuesIterable(this._table);
724744

725-
Iterator<E> get iterator => _CompactIterator<E>(
726-
_table,
727-
_table._data,
728-
_table._usedData,
729-
_offset,
730-
_step,
731-
);
745+
Iterator<E> get iterator =>
746+
_CompactIterator<E>(_table, _table._data, _table._usedData, -1, 2);
732747

733748
int get length => _table.length;
734749
bool get isEmpty => length == 0;
735750
bool get isNotEmpty => !isEmpty;
736751
}
737752

753+
// Iterates through _data[_offset + _step], _data[_offset + 2*_step], ...
754+
// and checks for concurrent modification.
738755
class _CompactIterator<E> implements Iterator<E> {
739756
final _HashBase _table;
740757

@@ -746,11 +763,11 @@ class _CompactIterator<E> implements Iterator<E> {
746763
E? _current;
747764

748765
_CompactIterator(this._table, this._data, this._len, this._offset, this._step)
749-
: _checkSum = _table._checkSum;
766+
: _checkSum = _table._checkSum;
750767

751768
bool moveNext() {
752769
if (_table._isModifiedSince(_data, _checkSum)) {
753-
throw new ConcurrentModificationError(_table);
770+
throw ConcurrentModificationError(_table);
754771
}
755772
do {
756773
_offset += _step;
@@ -767,6 +784,57 @@ class _CompactIterator<E> implements Iterator<E> {
767784
E get current => _current as E;
768785
}
769786

787+
// Iterates through map creating a MapEntry for each key-value pair, and checks
788+
// for concurrent modification.
789+
class _CompactEntriesIterable<K, V> extends Iterable<MapEntry<K, V>>
790+
implements
791+
EfficientLengthIterable<MapEntry<K, V>>,
792+
HideEfficientLengthIterable<MapEntry<K, V>> {
793+
final _HashBase _table;
794+
795+
_CompactEntriesIterable(this._table);
796+
797+
Iterator<MapEntry<K, V>> get iterator =>
798+
_CompactEntriesIterator<K, V>(_table, _table._data, _table._usedData);
799+
800+
int get length => _table.length;
801+
bool get isEmpty => length == 0;
802+
bool get isNotEmpty => !isEmpty;
803+
}
804+
805+
class _CompactEntriesIterator<K, V> implements Iterator<MapEntry<K, V>> {
806+
final _HashBase _table;
807+
808+
final WasmArray<Object?> _data;
809+
final int _len;
810+
int _offset = -2;
811+
final int _checkSum;
812+
MapEntry<K, V>? _current;
813+
814+
_CompactEntriesIterator(this._table, this._data, this._len)
815+
: _checkSum = _table._checkSum;
816+
817+
bool moveNext() {
818+
if (_table._isModifiedSince(_data, _checkSum)) {
819+
throw ConcurrentModificationError(_table);
820+
}
821+
do {
822+
_offset += 2;
823+
} while (_offset < _len && _HashBase._isDeleted(_data[_offset]));
824+
if (_offset < _len) {
825+
final key = unsafeCast<K>(_data[_offset]);
826+
final value = unsafeCast<V>(_data[_offset + 1]);
827+
_current = MapEntry<K, V>(key, value);
828+
return true;
829+
} else {
830+
_current = null;
831+
return false;
832+
}
833+
}
834+
835+
MapEntry<K, V> get current => _current!;
836+
}
837+
770838
// Iterates through _data[_offset + _step], _data[_offset + 2*_step], ...
771839
//
772840
// Does not check for concurrent modification since the table
@@ -1180,13 +1248,13 @@ base class CompactLinkedCustomHashSet<E> extends _HashFieldBase
11801248
) : _validKey = validKey ?? TypeTest<E>().test;
11811249

11821250
Set<R> cast<R>() => Set.castFrom<E, R>(this);
1183-
Set<E> toSet() =>
1184-
CompactLinkedCustomHashSet<E>(_equality, _hasher, _validKey)
1185-
..addAll(this);
1251+
Set<E> toSet() => CompactLinkedCustomHashSet<E>(_equality, _hasher, _validKey)
1252+
..addAll(this);
11861253
}
11871254

11881255
@pragma('wasm:prefer-inline')
11891256
Map<K, V> createMapFromKeyValueListUnsafe<K, V>(
11901257
WasmArray<Object?> keyValuePairData,
11911258
int usedData,
1192-
) => DefaultMap<K, V>().._populateUnsafe(keyValuePairData, usedData);
1259+
) =>
1260+
DefaultMap<K, V>().._populateUnsafe(keyValuePairData, usedData);

0 commit comments

Comments
 (0)