Skip to content

Commit 0a63d23

Browse files
rmacnak-googlecommit-bot@chromium.org
authored andcommitted
[vm] Don't allocate a backing store for maps until the first insert.
Empty maps are fairly common; delaying allocation of the backing store saves time and memory for empty maps. Non-empty maps probe an extra time for the first insert. Most maps also have few associations, so reducing the initial backing store size also saves memory on balance. The best value for dart2js would be 2 associations, but this CL changes it to 4 as a compromise with other benchmarks on Golem. Runtime as Score geomean 2.620% MemoryUse geomean -5.233% dart2js CompileSwarmLatest 0% dart2js CompileSwarmLastedMemoryUse -10.51% TEST=ci Bug: #26081 Change-Id: I80a925f698f3df44fae5e97e1804c8dff2ce0c60 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/176583 Reviewed-by: Alexander Markov <[email protected]> Reviewed-by: Stephen Adams <[email protected]> Commit-Queue: Ryan Macnak <[email protected]>
1 parent 0f76981 commit 0a63d23

File tree

3 files changed

+24
-77
lines changed

3 files changed

+24
-77
lines changed

runtime/vm/object.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10507,7 +10507,7 @@ class LinkedHashMap : public Instance {
1050710507
FINAL_HEAP_OBJECT_IMPLEMENTATION(LinkedHashMap, Instance);
1050810508

1050910509
// Keep this in sync with Dart implementation (lib/compact_hash.dart).
10510-
static const intptr_t kInitialIndexBits = 3;
10510+
static const intptr_t kInitialIndexBits = 2;
1051110511
static const intptr_t kInitialIndexSize = 1 << (kInitialIndexBits + 1);
1051210512

1051310513
// Allocate a map, but leave all fields set to null.

runtime/vm/object_test.cc

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -4667,73 +4667,6 @@ TEST_CASE(HashCode) {
46674667
EXPECT(result.IsIdenticalTo(expected));
46684668
}
46694669

4670-
static void CheckIdenticalHashStructure(Thread* T,
4671-
const Instance& a,
4672-
const Instance& b) {
4673-
const char* kScript =
4674-
"(a, b) {\n"
4675-
" if (a._usedData != b._usedData ||\n"
4676-
" a._deletedKeys != b._deletedKeys ||\n"
4677-
" a._hashMask != b._hashMask ||\n"
4678-
" a._index.length != b._index.length ||\n"
4679-
" a._data.length != b._data.length) {\n"
4680-
" return false;\n"
4681-
" }\n"
4682-
" for (var i = 0; i < a._index.length; ++i) {\n"
4683-
" if (a._index[i] != b._index[i]) {\n"
4684-
" return false;\n"
4685-
" }\n"
4686-
" }\n"
4687-
" for (var i = 0; i < a._data.length; ++i) {\n"
4688-
" var ad = a._data[i];\n"
4689-
" var bd = b._data[i];\n"
4690-
" if (!identical(ad, bd) && !(ad == a && bd == b)) {\n"
4691-
" return false;\n"
4692-
" }\n"
4693-
" }\n"
4694-
" return true;\n"
4695-
"}(a, b)";
4696-
String& name = String::Handle();
4697-
Array& param_names = Array::Handle(Array::New(2));
4698-
name = String::New("a");
4699-
param_names.SetAt(0, name);
4700-
name = String::New("b");
4701-
param_names.SetAt(1, name);
4702-
Array& param_values = Array::Handle(Array::New(2));
4703-
param_values.SetAt(0, a);
4704-
param_values.SetAt(1, b);
4705-
name = String::New(kScript);
4706-
Library& lib = Library::Handle(Library::CollectionLibrary());
4707-
EXPECT(Api::UnwrapHandle(TestCase::EvaluateExpression(
4708-
lib, name, param_names, param_values)) == Bool::True().raw());
4709-
}
4710-
4711-
TEST_CASE(LinkedHashMap) {
4712-
// Check that initial index size and hash mask match in Dart vs. C++.
4713-
// 1. Create an empty custom linked hash map in Dart.
4714-
const char* kScript =
4715-
"import 'dart:collection';\n"
4716-
"makeMap() {\n"
4717-
" bool Function(dynamic, dynamic) eq = (a, b) => true;\n"
4718-
" int Function(dynamic) hc = (a) => 42;\n"
4719-
" return new LinkedHashMap(equals: eq, hashCode: hc);\n"
4720-
"}";
4721-
Dart_Handle h_lib = TestCase::LoadTestScript(kScript, NULL);
4722-
EXPECT_VALID(h_lib);
4723-
Dart_Handle h_result = Dart_Invoke(h_lib, NewString("makeMap"), 0, NULL);
4724-
EXPECT_VALID(h_result);
4725-
4726-
TransitionNativeToVM transition(thread);
4727-
4728-
// 2. Create an empty internalized LinkedHashMap in C++.
4729-
Instance& dart_map = Instance::Handle();
4730-
dart_map ^= Api::UnwrapHandle(h_result);
4731-
LinkedHashMap& cc_map = LinkedHashMap::Handle(LinkedHashMap::NewDefault());
4732-
4733-
// 3. Expect them to have identical structure.
4734-
CheckIdenticalHashStructure(thread, dart_map, cc_map);
4735-
}
4736-
47374670
TEST_CASE(LinkedHashMap_iteration) {
47384671
const char* kScript =
47394672
"makeMap() {\n"

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

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,13 @@ abstract class _HashFieldBase {
2424
// least one unoccupied entry.
2525
// NOTE: When maps are deserialized, their _index and _hashMask is regenerated
2626
// eagerly by _regenerateIndex.
27-
Uint32List _index = new Uint32List(_HashBase._INITIAL_INDEX_SIZE);
27+
Uint32List _index = _initialIndex;
2828

2929
// Cached in-place mask for the hash pattern component.
30-
int _hashMask = _HashBase._indexSizeToHashMask(_HashBase._INITIAL_INDEX_SIZE);
30+
int _hashMask = 0;
3131

3232
// Fixed-length list of keys (set) or key/value at even/odd indices (map).
33-
List _data;
33+
List _data = _initialData;
3434

3535
// Length of _data that is used (i.e., keys + values for a map).
3636
int _usedData = 0;
@@ -41,7 +41,7 @@ abstract class _HashFieldBase {
4141
// Note: All fields are initialized in a single constructor so that the VM
4242
// recognizes they cannot hold null values. This makes a big (20%) performance
4343
// difference on some operations.
44-
_HashFieldBase(int dataSize) : this._data = new List.filled(dataSize, null);
44+
_HashFieldBase(int dataSize);
4545
}
4646

4747
// Base class for VM-internal classes; keep in sync with _HashFieldBase.
@@ -97,7 +97,7 @@ abstract class _HashBase implements _HashVMBase {
9797
// are doubled when _data is full. Thus, _index will have a max load factor
9898
// of 1/2, which enables one more bit to be used for the hash.
9999
// TODO(koda): Consider growing _data by factor sqrt(2), twice as often.
100-
static const int _INITIAL_INDEX_BITS = 3;
100+
static const int _INITIAL_INDEX_BITS = 2;
101101
static const int _INITIAL_INDEX_SIZE = 1 << (_INITIAL_INDEX_BITS + 1);
102102

103103
// Unused and deleted entries are marked by 0 and 1, respectively.
@@ -155,6 +155,12 @@ class _IdenticalAndIdentityHashCode {
155155
bool _equals(e1, e2) => identical(e1, e2);
156156
}
157157

158+
final _initialIndex = new Uint32List(1);
159+
// Note: not const. Const arrays are made immutable by having a different class
160+
// than regular arrays that throws on element assignment. We want the data field
161+
// in maps and sets to be monomorphic.
162+
final _initialData = new List.filled(0, null);
163+
158164
// VM-internalized implementation of a default-constructed LinkedHashMap.
159165
@pragma("vm:entry-point")
160166
class _InternalLinkedHashMap<K, V> extends _HashVMBase
@@ -165,9 +171,9 @@ class _InternalLinkedHashMap<K, V> extends _HashVMBase
165171
_OperatorEqualsAndHashCode
166172
implements LinkedHashMap<K, V> {
167173
_InternalLinkedHashMap() {
168-
_index = new Uint32List(_HashBase._INITIAL_INDEX_SIZE);
169-
_hashMask = _HashBase._indexSizeToHashMask(_HashBase._INITIAL_INDEX_SIZE);
170-
_data = new List.filled(_HashBase._INITIAL_INDEX_SIZE, null);
174+
_index = _initialIndex;
175+
_hashMask = 0;
176+
_data = _initialData;
171177
_usedData = 0;
172178
_deletedKeys = 0;
173179
}
@@ -202,6 +208,10 @@ abstract class _LinkedHashMapMixin<K, V> implements _HashBase {
202208

203209
// Allocate new _index and _data, and optionally copy existing contents.
204210
void _init(int size, int hashMask, List? oldData, int oldUsed) {
211+
if (size < _HashBase._INITIAL_INDEX_SIZE) {
212+
size = _HashBase._INITIAL_INDEX_SIZE;
213+
hashMask = _HashBase._indexSizeToHashMask(size);
214+
}
205215
assert(size & (size - 1) == 0);
206216
assert(_HashBase._UNUSED_PAIR == 0);
207217
_index = new Uint32List(size);
@@ -222,7 +232,7 @@ abstract class _LinkedHashMapMixin<K, V> implements _HashBase {
222232

223233
// This method is called by [_rehashObjects] (see above).
224234
void _regenerateIndex() {
225-
_index = new Uint32List(_data.length);
235+
_index = _data.length == 0 ? _initialIndex : new Uint32List(_data.length);
226236
assert(_hashMask == 0);
227237
_hashMask = _HashBase._indexSizeToHashMask(_index.length);
228238
final int tmpUsed = _usedData;
@@ -526,6 +536,10 @@ class _CompactLinkedHashSet<E> extends _HashFieldBase
526536
}
527537

528538
void _init(int size, int hashMask, List? oldData, int oldUsed) {
539+
if (size < _HashBase._INITIAL_INDEX_SIZE) {
540+
size = _HashBase._INITIAL_INDEX_SIZE;
541+
hashMask = _HashBase._indexSizeToHashMask(size);
542+
}
529543
_index = new Uint32List(size);
530544
_hashMask = hashMask;
531545
_data = new List.filled(size >> 1, null);

0 commit comments

Comments
 (0)