Skip to content

Commit 9de3739

Browse files
eregoneregon
committed
Cleanup
1 parent 2f78a96 commit 9de3739

File tree

3 files changed

+41
-61
lines changed

3 files changed

+41
-61
lines changed

‎src/main/java/org/truffleruby/core/hash/HashLiteralNode.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@ protected HashLiteralNode(RubyNode[] keyValues) {
3030
this.keyValues = keyValues;
3131
}
3232

33-
public static HashLiteralNode create(RubyNode[] keyValues, RubyLanguage rubyLanguage) {
33+
public static HashLiteralNode create(RubyNode[] keyValues, RubyLanguage language) {
3434
if (keyValues.length == 0) {
3535
return new EmptyHashStore.EmptyHashLiteralNode();
3636
} else if (keyValues.length <= PackedHashStoreLibrary.MAX_ENTRIES * 2) {
3737
return PackedHashStoreLibraryFactory.SmallHashLiteralNodeGen.create(keyValues);
3838
} else {
39-
return rubyLanguage.options.BIG_HASH_STRATEGY_IS_BUCKETS
39+
return language.options.BIG_HASH_STRATEGY_IS_BUCKETS
4040
? new BucketsHashStore.BucketHashLiteralNode(keyValues)
4141
: new CompactHashStore.CompactHashLiteralNode(keyValues);
4242
}

‎src/main/java/org/truffleruby/core/hash/library/CompactHashStore.java

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,10 @@
4949
import com.oracle.truffle.api.profiles.InlinedConditionProfile;
5050
import com.oracle.truffle.api.profiles.InlinedLoopConditionProfile;
5151

52-
/* The Compact hash strategy from Hash Maps That Dont Hate You
53-
* See https://blog.toit.io/hash-maps-that-dont-hate-you-1a96150b492a for more details (archived at
54-
* https://archive.ph/wip/sucRY) */
52+
/** The Compact hash strategy from Hash Maps That Don't Hate You. See
53+
* https://blog.toit.io/hash-maps-that-dont-hate-you-1a96150b492a for more details (archived at
54+
* https://archive.ph/wip/sucRY). Specifically that blog post has good visualizations of the index and kvStore arrays.
55+
* We do not currently implement the deletion optimization. */
5556
@ExportLibrary(value = HashStoreLibrary.class)
5657
@GenerateUncached
5758
public final class CompactHashStore {
@@ -77,12 +78,12 @@ public final class CompactHashStore {
7778
// multiple not-equal hashes can map to the same position in the index array, that's a different sort of
7879
// "collision" in addition to the usual collision of equal hash values coming from different keys. Both kinds
7980
// of collisions actually result in the same outcome: contention on an index slot. So both are automatically
80-
// handled by the same collision-resolution strategy: open-addressing-with linear probing.)
81+
// handled by the same collision-resolution strategy: open-addressing with linear probing.)
8182
// -----------------------------------------------
8283
// (3) tl;dr: Compact Hashes give you both insertion-order iteration AND the usual const-time guarantees
8384
// ---------------------------------------------------------------------------------------------------------------
8485

85-
/** We view the index array as consisting of "Slots", each slot is 2 array positions. index[0] and index[1] is one
86+
/** We view the index array as consisting of "slots", each slot is 2 array positions. index[0] and index[1] is one
8687
* slot, index[2] and index[3] is another,... The first position of a slot holds the hash of a key and the second
8788
* holds an offsetted index into the KV store (index + 1) where the key is stored. The reason we store an offsetted
8889
* index and not the index itself is so that 0 is not a valid index, thus we can use it as unused slot marker, and
@@ -98,8 +99,8 @@ public final class CompactHashStore {
9899
* factor of 0.75 and an index of total size 64 slots, that number is 48 slots. (Technically, that's redundant
99100
* information that is derived from the load factor and the index size, but deriving it requires a float
100101
* multiplication or division, and we want to check for it on every insertion, so it's inefficient to keep
101-
* calculating it every time its needed) */
102-
private int numSlotsForIndexRebuild;
102+
* calculating it every time it is needed) */
103+
private int indexGrowthThreshold;
103104

104105
/** Each slot in the index array can be in one of 3 states, depending on the value of its second (offset) field:
105106
* <li>Offset >= 1: Filled, the data in the hash field and the offset field is valid. Subtracting one from the
@@ -112,27 +113,12 @@ public final class CompactHashStore {
112113
private static final int INDEX_SLOT_UNUSED = 0;
113114
private static final int INDEX_SLOT_DELETED = -1;
114115

115-
// returned by methods doing array search which don't find what they're looking for
116+
// Returned by methods doing array search which don't find what they are looking for
116117
static final int KEY_NOT_FOUND = -2;
117-
private static final int HASH_NOT_FOUND = KEY_NOT_FOUND;
118-
119-
// In hash entries, not array positions (in general, capacities and sizes are always in entries)
120-
public static final int DEFAULT_INITIAL_CAPACITY = 8;
118+
static final int HASH_NOT_FOUND = KEY_NOT_FOUND;
121119

122120
public static final float THRESHOLD_LOAD_FACTOR_FOR_INDEX_REBUILD = 0.75f;
123121

124-
public CompactHashStore() {
125-
this(DEFAULT_INITIAL_CAPACITY);
126-
}
127-
128-
// private non-allocating constructor for .copy(), not part of the interface
129-
private CompactHashStore(int[] index, Object[] kvs, int kvStoreInsertionPos, int numSlotsForIndexRebuild) {
130-
this.index = index;
131-
this.kvStore = kvs;
132-
this.kvStoreInsertionPos = kvStoreInsertionPos;
133-
this.numSlotsForIndexRebuild = numSlotsForIndexRebuild;
134-
}
135-
136122
public CompactHashStore(int capacity) {
137123
if (capacity < 1 || capacity >= (1 << 28)) {
138124
throw shouldNotReachHere();
@@ -144,17 +130,27 @@ public CompactHashStore(int capacity) {
144130
// This way the initial load factor (for capacity entries) is between and 0.25 and 0.5.
145131
int indexCapacity = 2 * kvCapacity;
146132

147-
// All 0s by default, so all slots are marked empty for free
133+
// All zeros by default, so all slots are marked empty for free
148134
this.index = new int[2 * indexCapacity];
149135
this.kvStore = new Object[2 * kvCapacity];
150-
this.kvStoreInsertionPos = 0; // always increases by 2, never decreases
151-
this.numSlotsForIndexRebuild = (int) (indexCapacity * THRESHOLD_LOAD_FACTOR_FOR_INDEX_REBUILD);
136+
this.kvStoreInsertionPos = 0;
137+
this.indexGrowthThreshold = (int) (indexCapacity * THRESHOLD_LOAD_FACTOR_FOR_INDEX_REBUILD);
138+
}
139+
140+
// private non-allocating constructor for .copy()
141+
private CompactHashStore(int[] index, Object[] kvStore, int kvStoreInsertionPos, int indexGrowthThreshold) {
142+
this.index = index;
143+
this.kvStore = kvStore;
144+
this.kvStoreInsertionPos = kvStoreInsertionPos;
145+
this.indexGrowthThreshold = indexGrowthThreshold;
152146
}
153147

148+
/** Rounds up powers of 2 themselves : 1 => 2, 2 => 4, 3 => 4, 4 => 8, ... */
154149
private static int roundUpwardsToNearestPowerOf2(int num) {
155-
return Integer.highestOneBit(num) << 1; // rounds powers of 2 themselves : 1 => 2, 2 => 4, 3 => 4, 4 => 8, ...
150+
return Integer.highestOneBit(num) << 1;
156151
}
157152

153+
// For promoting from packed to compact
158154
public void putHashKeyValue(int hashcode, Object key, Object value) {
159155
int pos = kvStoreInsertionPos;
160156
SetKvAtNode.insertIntoKv(this, key, value);
@@ -313,12 +309,12 @@ void rehash(RubyHash hash,
313309
@Cached @Exclusive InlinedLoopConditionProfile loopProfile,
314310
@CachedLibrary("this") HashStoreLibrary hashlib,
315311
@Bind("$node") Node node) {
316-
Object[] oldKvStore = kvStore;
317-
int oldKvStoreInsertionPos = kvStoreInsertionPos;
312+
Object[] oldKvStore = this.kvStore;
313+
int oldKvStoreInsertionPos = this.kvStoreInsertionPos;
318314

319315
this.kvStore = new Object[oldKvStore.length];
320316
this.kvStoreInsertionPos = 0;
321-
this.index = new int[index.length];
317+
this.index = new int[this.index.length];
322318
hash.size = 0;
323319

324320
int i = 0;
@@ -401,7 +397,7 @@ private int firstNonNullKeyPosFromBeginning(
401397
}
402398

403399
private CompactHashStore copy() {
404-
return new CompactHashStore(index.clone(), kvStore.clone(), kvStoreInsertionPos, numSlotsForIndexRebuild);
400+
return new CompactHashStore(index.clone(), kvStore.clone(), kvStoreInsertionPos, indexGrowthThreshold);
405401
}
406402

407403
/** @param hash a hash code
@@ -567,7 +563,7 @@ static boolean keyDoesntExist(
567563
@Cached @Exclusive InlinedLoopConditionProfile indexSlotUnavailable,
568564
@Bind("$node") Node node) {
569565
if (indexResizingIsNeeded.profile(node,
570-
hash.size >= store.numSlotsForIndexRebuild)) {
566+
hash.size >= store.indexGrowthThreshold)) {
571567
resizeIndex(store, node);
572568
}
573569
if (kvResizingIsNeeded.profile(node,
@@ -612,7 +608,7 @@ private static void resizeIndex(CompactHashStore store, Node node) {
612608

613609
insertIntoIndex(hash, kvPos, store.index, InlinedLoopConditionProfile.getUncached(), node);
614610
}
615-
store.numSlotsForIndexRebuild = (int) (oldIndex.length * THRESHOLD_LOAD_FACTOR_FOR_INDEX_REBUILD);
611+
store.indexGrowthThreshold = (int) (oldIndex.length * THRESHOLD_LOAD_FACTOR_FOR_INDEX_REBUILD);
616612
}
617613

618614
private static void resizeKvStore(CompactHashStore store) {

‎src/main/java/org/truffleruby/core/hash/library/PackedHashStoreLibrary.java

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
import org.truffleruby.RubyContext;
1313
import org.truffleruby.RubyLanguage;
14-
import org.truffleruby.annotations.SuppressFBWarnings;
1514
import org.truffleruby.collections.PEBiFunction;
1615
import org.truffleruby.core.array.ArrayHelpers;
1716
import org.truffleruby.core.array.RubyArray;
@@ -39,7 +38,6 @@
3938
import com.oracle.truffle.api.dsl.Cached;
4039
import com.oracle.truffle.api.dsl.Cached.Exclusive;
4140
import com.oracle.truffle.api.dsl.Cached.Shared;
42-
import com.oracle.truffle.api.dsl.GenerateInline;
4341
import com.oracle.truffle.api.dsl.GenerateUncached;
4442
import com.oracle.truffle.api.dsl.ImportStatic;
4543
import com.oracle.truffle.api.dsl.Specialization;
@@ -127,7 +125,6 @@ private static boolean verifyIntegerHashes(Object[] store) {
127125
return true;
128126
}
129127

130-
@SuppressFBWarnings("UPM_UNCALLED_PRIVATE_METHOD")
131128
@TruffleBoundary
132129
private static void promoteToBuckets(RubyHash hash, Object[] store, int size) {
133130
final Entry[] buckets = new Entry[BucketsHashStore.growthCapacityGreaterThan(size)];
@@ -158,14 +155,13 @@ private static void promoteToBuckets(RubyHash hash, Object[] store, int size) {
158155
hash.size = size;
159156
}
160157

161-
@TruffleBoundary
162-
private static void promoteToCompact(RubyHash hash, Object[] store, int size) {
163-
CompactHashStore newStore = new CompactHashStore(size);
164-
for (int n = 0; n < size; n++) {
158+
private static void promoteToCompact(RubyHash hash, Object[] store) {
159+
CompactHashStore newStore = new CompactHashStore(MAX_ENTRIES);
160+
for (int n = 0; n < MAX_ENTRIES; n++) {
165161
newStore.putHashKeyValue(getHashed(store, n), getKey(store, n), getValue(store, n));
166162
}
167163
hash.store = newStore;
168-
hash.size = size;
164+
hash.size = MAX_ENTRIES;
169165
}
170166

171167
// endregion
@@ -209,7 +205,6 @@ static boolean set(Object[] store, RubyHash hash, Object key, Object value, bool
209205
@Cached @Shared CompareHashKeysNode compareHashKeys,
210206
@CachedLibrary(limit = "hashStrategyLimit()") HashStoreLibrary hashes,
211207
@Cached InlinedConditionProfile withinCapacity,
212-
@Cached(inline = true) PromoteToBigHashNode promoteToBigHash,
213208
@Bind("this") Node node) {
214209

215210
assert verify(store, hash);
@@ -239,7 +234,11 @@ static boolean set(Object[] store, RubyHash hash, Object key, Object value, bool
239234

240235

241236
assert size == MAX_ENTRIES;
242-
promoteToBigHash.execute(node, store, hash, MAX_ENTRIES);
237+
if (RubyLanguage.get(node).options.BIG_HASH_STRATEGY_IS_BUCKETS) {
238+
promoteToBuckets(hash, store, MAX_ENTRIES);
239+
} else {
240+
promoteToCompact(hash, store);
241+
}
243242

244243
hashes.set(hash.store, hash, key2, value, byIdentity);
245244
return true;
@@ -412,21 +411,6 @@ static boolean verify(Object[] store, RubyHash hash) {
412411
// endregion
413412
// region Nodes
414413

415-
@GenerateUncached
416-
@GenerateInline
417-
abstract static class PromoteToBigHashNode extends RubyBaseNode {
418-
public abstract void execute(Node node, Object[] store, RubyHash hash, int size);
419-
420-
@Specialization
421-
void promoteToBigHash(Object[] store, RubyHash hash, int size) {
422-
if (getLanguage().options.BIG_HASH_STRATEGY_IS_BUCKETS) {
423-
promoteToBuckets(hash, store, size);
424-
} else {
425-
promoteToCompact(hash, store, size);
426-
}
427-
}
428-
}
429-
430414
@GenerateUncached
431415
@ImportStatic(HashGuards.class)
432416
public abstract static class LookupPackedEntryNode extends RubyBaseNode {

0 commit comments

Comments
 (0)