Skip to content

Commit

Permalink
Fix #3570 by resetting expandTableThreshold and
Browse files Browse the repository at this point in the history
maxRunBeforeFallback after resizing the hashTable.

Fixes #3571

[]

RELNOTES=Fixed a bug in `ImmutableSet.Builder` that could lead to infinite loops when building multiple sets from the same builder.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=264648412
  • Loading branch information
tomasisonfire authored and cpovirk committed Aug 21, 2019
1 parent 7ab65d0 commit 0007cb2
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 0 deletions.
Expand Up @@ -366,4 +366,12 @@ public void testReusedBuilder() {
builder.add("baz");
assertTrue(set.elements != builder.contents);
}

public void testReuseBuilderReducingHashTableSizeWithPowerOfTwoTotalElements() {
ImmutableSet.Builder<Object> builder = ImmutableSet.builderWithExpectedSize(6);
builder.add(0);
ImmutableSet<Object> unused = builder.build();
ImmutableSet<Object> subject = builder.add(1).add(2).add(3).build();
assertFalse(subject.contains(4));
}
}
Expand Up @@ -233,6 +233,11 @@ public void testEquals_sameType() throws Exception {
testCase.testEquals_sameType();
}

public void testReuseBuilderReducingHashTableSizeWithPowerOfTwoTotalElements() throws Exception {
com.google.common.collect.ImmutableSetTest testCase = new com.google.common.collect.ImmutableSetTest();
testCase.testReuseBuilderReducingHashTableSizeWithPowerOfTwoTotalElements();
}

public void testReuseBuilderWithDuplicateElements() throws Exception {
com.google.common.collect.ImmutableSetTest testCase = new com.google.common.collect.ImmutableSetTest();
testCase.testReuseBuilderWithDuplicateElements();
Expand Down
Expand Up @@ -582,4 +582,12 @@ private static long worstCaseQueryOperations(Set<?> set, CallsCounter counter) {
}
return worstCalls;
}

public void testReuseBuilderReducingHashTableSizeWithPowerOfTwoTotalElements() {
ImmutableSet.Builder<Object> builder = ImmutableSet.builderWithExpectedSize(6);
builder.add(0);
ImmutableSet<Object> unused = builder.build();
ImmutableSet<Object> subject = builder.add(1).add(2).add(3).build();
assertFalse(subject.contains(4));
}
}
2 changes: 2 additions & 0 deletions guava/src/com/google/common/collect/ImmutableSet.java
Expand Up @@ -813,6 +813,8 @@ SetBuilderImpl<E> review() {
int targetTableSize = chooseTableSize(distinct);
if (targetTableSize * 2 < hashTable.length) {
hashTable = rebuildHashTable(targetTableSize, dedupedElements, distinct);
maxRunBeforeFallback = maxRunBeforeFallback(targetTableSize);
expandTableThreshold = (int) (DESIRED_LOAD_FACTOR * targetTableSize);
}
return hashFloodingDetected(hashTable) ? new JdkBackedSetBuilderImpl<E>(this) : this;
}
Expand Down

0 comments on commit 0007cb2

Please sign in to comment.