Skip to content
Permalink
Browse files
[Truffle] Make the bucket chain singly linked.
  • Loading branch information
chrisseaton committed Dec 16, 2014
1 parent 18593e6 commit 35c13507781fe8d91b2603085235f99bee19ecc2
@@ -467,14 +467,10 @@ public Object delete(VirtualFrame frame, RubyHash hash, Object key) {

// Remove from the lookup chain

if (bucket.getPreviousInLookup() == null) {
if (bucketSearchResult.getPreviousBucket() == null) {
((Bucket[]) hash.getStore())[bucketSearchResult.getIndex()] = bucket.getNextInLookup();
} else {
bucket.getPreviousInLookup().setNextInLookup(bucket.getNextInLookup());
}

if (bucket.getNextInLookup() != null) {
bucket.getNextInLookup().setPreviousInLookup(bucket.getPreviousInLookup());
bucketSearchResult.getPreviousBucket().setNextInLookup(bucket.getNextInLookup());
}

hash.setSize(hash.getSize() - 1);
@@ -48,18 +48,18 @@ public BucketSearchResult search(VirtualFrame frame, RubyHash hash, Object key)
final int bucketIndex = (hashed & HashOperations.SIGN_BIT_MASK) % buckets.length;
Bucket bucket = buckets[bucketIndex];

Bucket endOfLookupChain = null;
Bucket previousBucket = null;

while (bucket != null) {
if (eqlNode.call(frame, key, "eql?", null, bucket.getKey())) {
return new BucketSearchResult(bucketIndex, bucket, bucket);
return new BucketSearchResult(bucketIndex, previousBucket, bucket);
}

endOfLookupChain = bucket;
previousBucket = bucket;
bucket = bucket.getNextInLookup();
}

return new BucketSearchResult(bucketIndex, endOfLookupChain, null);
return new BucketSearchResult(bucketIndex, previousBucket, null);
}

@Override
@@ -19,7 +19,6 @@ public class Bucket {
private Object key;
private Object value;

private Bucket previousInLookup;
private Bucket nextInLookup;

private Bucket previousInSequence;
@@ -46,14 +45,6 @@ public void setValue(Object value) {
this.value = value;
}

public Bucket getPreviousInLookup() {
return previousInLookup;
}

public void setPreviousInLookup(Bucket previousInLookup) {
this.previousInLookup = previousInLookup;
}

public Bucket getNextInLookup() {
return nextInLookup;
}
@@ -10,36 +10,37 @@
package org.jruby.truffle.runtime.hash;

/**
* The result of looking for a bucket (a {@link Bucket}) in a Ruby hash. We get the last bucket in the lookup chain for
* this index, the bucket that was found, and the index that was used. There are three possible outcomes for a search.
* The result of looking for a bucket (a {@link Bucket}) in a Ruby hash. We get the previous bucket in the lookup chain
* for this index until the bucket was found, the bucket that was found, and the index that was used. There are three
* possible outcomes for a search.
* <ul>
* <li>There is nothing at that index, in which case the bucket and last bucket in the chain will be
* {@code null}</li>
* <li>There were buckets at that index, but none for our key, in which case the bucket will be null, but the last
* bucket will the last bucket in the chain at that index, presumably where we will want to insert your new
* bucket</li>
* <li>A bucket was found for our key, in which case the bucket and the last bucket in the chain will be the
* same</li>
* <li>There were buckets at that index, but none for our key, in which case the bucket will be null, but the
* previous bucket will be the last bucket in the chain at that index, presumably where we will want to insert our
* new bucket</li>
* <li>A bucket was found for our key, in which case the bucket will be the one correspond to the key, and the
* previous bucket will be the one in the bucket chain before that one</li>
* </ul>
*/
public class BucketSearchResult {

private final Bucket endOfLookupChain;
private final Bucket previousBucket;
private final Bucket bucket;
private final int index;

public BucketSearchResult(int index, Bucket endOfLookupChain, Bucket bucket) {
public BucketSearchResult(int index, Bucket previousBucket, Bucket bucket) {
this.index = index;
this.endOfLookupChain = endOfLookupChain;
this.previousBucket = previousBucket;
this.bucket = bucket;
}

public int getIndex() {
return index;
}

public Bucket getEndOfLookupChain() {
return endOfLookupChain;
public Bucket getPreviousBucket() {
return previousBucket;
}

public Bucket getBucket() {
@@ -139,26 +139,32 @@ public static BucketSearchResult verySlowFindBucket(RubyHash hash, Object key) {
final int bucketIndex = (hashed & SIGN_BIT_MASK) % buckets.length;
Bucket bucket = buckets[bucketIndex];

Bucket endOfLookupChain = null;
Bucket previousBucket = null;

while (bucket != null) {
// TODO: cast

if ((boolean) DebugOperations.send(hash.getContext(), key, "eql?", null, bucket.getKey())) {
return new BucketSearchResult(bucketIndex, bucket, bucket);
return new BucketSearchResult(bucketIndex, previousBucket, bucket);
}

endOfLookupChain = bucket;
previousBucket = bucket;
bucket = bucket.getNextInLookup();
}

return new BucketSearchResult(bucketIndex, endOfLookupChain, null);
return new BucketSearchResult(bucketIndex, previousBucket, null);
}

public static void setAtBucket(RubyHash hash, BucketSearchResult bucketSearchResult, Object key, Object value) {
if (bucketSearchResult.getBucket() == null) {
final Bucket bucket = new Bucket(key, value);

if (bucketSearchResult.getPreviousBucket() == null) {
((Bucket[]) hash.getStore())[bucketSearchResult.getIndex()] = bucket;
} else {
bucketSearchResult.getPreviousBucket().setNextInLookup(bucket);
}

if (hash.getFirstInSequence() == null) {
hash.setFirstInSequence(bucket);
hash.setLastInSequence(bucket);
@@ -167,13 +173,6 @@ public static void setAtBucket(RubyHash hash, BucketSearchResult bucketSearchRes
bucket.setPreviousInSequence(hash.getLastInSequence());
hash.setLastInSequence(bucket);
}

if (bucketSearchResult.getEndOfLookupChain() == null) {
((Bucket[]) hash.getStore())[bucketSearchResult.getIndex()] = bucket;
} else {
bucketSearchResult.getEndOfLookupChain().setNextInLookup(bucket);
bucket.setPreviousInLookup(bucketSearchResult.getEndOfLookupChain());
}
} else {
final Bucket bucket = bucketSearchResult.getBucket();

1 comment on commit 35c1350

@eregon
Copy link
Member

@eregon eregon commented on 35c1350 Dec 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!
previousBucket might not be needed for many operations, but this is future optimization.
It can also be re-computed cheaply if we know the chain at an index does not get too long.

Please sign in to comment.