Skip to content

Commit 7d04207

Browse files
committed
[GR-32640] Read entries from the Hash only once
* To ensure it is the same array used for getBucketIndex() and accesses inside a given operation.
1 parent 177f786 commit 7d04207

File tree

1 file changed

+23
-15
lines changed

1 file changed

+23
-15
lines changed

src/main/java/org/truffleruby/core/hash/library/BucketsHashStore.java

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
@GenerateUncached
5555
public class BucketsHashStore {
5656

57-
private Entry[] entries;
57+
private final Entry[] entries;
5858

5959
public BucketsHashStore(Entry[] entries) {
6060
this.entries = entries;
@@ -178,9 +178,9 @@ private static void removeFromSequenceChain(RubyHash hash, Entry entry) {
178178
}
179179
}
180180

181-
private static void removeFromLookupChain(RubyHash hash, int index, Entry entry, Entry previousEntry) {
181+
private static void removeFromLookupChain(Entry[] entries, int index, Entry entry, Entry previousEntry) {
182182
if (previousEntry == null) {
183-
((BucketsHashStore) hash.store).entries[index] = entry.getNextInLookup();
183+
entries[index] = entry.getNextInLookup();
184184
} else {
185185
previousEntry.setNextInLookup(entry.getNextInLookup());
186186
}
@@ -194,7 +194,8 @@ protected Object lookupOrDefault(Frame frame, RubyHash hash, Object key, PEBiFun
194194
@Cached @Shared("lookup") LookupEntryNode lookup,
195195
@Cached @Exclusive ConditionProfile found) {
196196

197-
final HashLookupResult hashLookupResult = lookup.execute(hash, key);
197+
final Entry[] entries = this.entries;
198+
final HashLookupResult hashLookupResult = lookup.execute(hash, entries, key);
198199

199200
if (found.profile(hashLookupResult.getEntry() != null)) {
200201
return hashLookupResult.getEntry().getValue();
@@ -213,14 +214,15 @@ protected boolean set(RubyHash hash, Object key, Object value, boolean byIdentit
213214
@Cached @Exclusive ConditionProfile bucketCollision,
214215
@Cached @Exclusive ConditionProfile appending,
215216
@Cached @Exclusive ConditionProfile resize) {
216-
217217
assert verify(hash);
218+
218219
final Object key2 = freezeHashKeyIfNeeded.executeFreezeIfNeeded(key, byIdentity);
219220

220221
propagateSharingKey.executePropagate(hash, key2);
221222
propagateSharingValue.executePropagate(hash, value);
222223

223-
final HashLookupResult result = lookup.execute(hash, key2);
224+
final Entry[] entries = this.entries;
225+
final HashLookupResult result = lookup.execute(hash, entries, key2);
224226
final Entry entry = result.getEntry();
225227

226228
if (missing.profile(entry == null)) {
@@ -260,17 +262,18 @@ protected boolean set(RubyHash hash, Object key, Object value, boolean byIdentit
260262
protected Object delete(RubyHash hash, Object key,
261263
@Cached @Shared("lookup") LookupEntryNode lookup,
262264
@Cached @Exclusive ConditionProfile missing) {
263-
264265
assert verify(hash);
265-
final HashLookupResult lookupResult = lookup.execute(hash, key);
266+
267+
final Entry[] entries = this.entries;
268+
final HashLookupResult lookupResult = lookup.execute(hash, entries, key);
266269
final Entry entry = lookupResult.getEntry();
267270

268271
if (missing.profile(entry == null)) {
269272
return null;
270273
}
271274

272275
removeFromSequenceChain(hash, entry);
273-
removeFromLookupChain(hash, lookupResult.getIndex(), entry, lookupResult.getPreviousEntry());
276+
removeFromLookupChain(entries, lookupResult.getIndex(), entry, lookupResult.getPreviousEntry());
274277
hash.size -= 1;
275278
assert verify(hash);
276279
return entry.getValue();
@@ -280,6 +283,8 @@ protected Object delete(RubyHash hash, Object key,
280283
protected Object deleteLast(RubyHash hash, Object key,
281284
@Cached @Exclusive ConditionProfile singleEntry) {
282285
assert verify(hash);
286+
287+
final Entry[] entries = this.entries;
283288
final Entry lastEntry = hash.lastInSequence;
284289
if (key != lastEntry.getKey()) {
285290
CompilerDirectives.transferToInterpreterAndInvalidate();
@@ -308,7 +313,7 @@ protected Object deleteLast(RubyHash hash, Object key,
308313
hash.lastInSequence = previousInSequence;
309314
}
310315

311-
removeFromLookupChain(hash, index, entry, previousEntry);
316+
removeFromLookupChain(entries, index, entry, previousEntry);
312317
hash.size -= 1;
313318
assert verify(hash);
314319
return entry.getValue();
@@ -351,7 +356,8 @@ protected void replace(RubyHash hash, RubyHash dest,
351356
propagateSharing.executePropagate(dest, hash);
352357
assert verify(hash);
353358

354-
final Entry[] newEntries = new Entry[((BucketsHashStore) hash.store).entries.length];
359+
final Entry[] entries = ((BucketsHashStore) hash.store).entries;
360+
final Entry[] newEntries = new Entry[entries.length];
355361

356362
Entry firstInSequence = null;
357363
Entry lastInSequence = null;
@@ -390,6 +396,7 @@ protected RubyArray shift(RubyHash hash,
390396

391397
assert verify(hash);
392398

399+
final Entry[] entries = this.entries;
393400
final Entry first = hash.firstInSequence;
394401
assert first.getPreviousInSequence() == null;
395402

@@ -407,7 +414,7 @@ protected RubyArray shift(RubyHash hash,
407414
hash.lastInSequence = null;
408415
}
409416

410-
final int index = getBucketIndex(first.getHashed(), this.entries.length);
417+
final int index = getBucketIndex(first.getHashed(), entries.length);
411418

412419
Entry previous = null;
413420
Entry entry = entries[index];
@@ -437,6 +444,7 @@ protected void rehash(RubyHash hash,
437444
@Cached HashingNodes.ToHash hashNode) {
438445

439446
assert verify(hash);
447+
final Entry[] entries = this.entries;
440448
Arrays.fill(entries, null);
441449

442450
Entry entry = hash.firstInSequence;
@@ -483,6 +491,7 @@ protected void rehash(RubyHash hash,
483491
public boolean verify(RubyHash hash) {
484492
assert hash.store == this;
485493

494+
final Entry[] entries = this.entries;
486495
final int size = hash.size;
487496
final Entry firstInSequence = hash.firstInSequence;
488497
final Entry lastInSequence = hash.lastInSequence;
@@ -538,17 +547,16 @@ public boolean verify(RubyHash hash) {
538547
@GenerateUncached
539548
abstract static class LookupEntryNode extends RubyBaseNode {
540549

541-
public abstract HashLookupResult execute(RubyHash hash, Object key);
550+
public abstract HashLookupResult execute(RubyHash hash, Entry[] entries, Object key);
542551

543552
@Specialization
544-
protected HashLookupResult lookup(RubyHash hash, Object key,
553+
protected HashLookupResult lookup(RubyHash hash, Entry[] entries, Object key,
545554
@Cached HashingNodes.ToHash hashNode,
546555
@Cached CompareHashKeysNode compareHashKeysNode,
547556
@Cached ConditionProfile byIdentityProfile) {
548557
final boolean compareByIdentity = byIdentityProfile.profile(hash.compareByIdentity);
549558
int hashed = hashNode.execute(key, compareByIdentity);
550559

551-
final Entry[] entries = ((BucketsHashStore) hash.store).entries;
552560
final int index = getBucketIndex(hashed, entries.length);
553561
Entry entry = entries[index];
554562

0 commit comments

Comments
 (0)