Skip to content

Commit

Permalink
Backport PrimitiveHashSet#iterator()#remove() to not rehash. Fixes go…
Browse files Browse the repository at this point in the history
…ldmansachs#35.

Signed-off-by: Nikhil Nanivadekar <nikhil.nanivadekar@gs.com>
  • Loading branch information
nikhilnanivadekar committed Mar 25, 2018
1 parent fa8bf3e commit e222fac
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -289,14 +289,7 @@ public final class <name>HashSet extends Abstract<name>Set implements Mutable<na
{
if (isBetweenZeroAndThirtyOne(value))
{
int initial = this.zeroToThirtyOne;
this.zeroToThirtyOne &= ~(1 \<\< <(castRealTypeToInt.(type))("value")>);
if (this.zeroToThirtyOne == initial)
{
return false;
}
this.zeroToThirtyOneOccupied--;
return true;
return this.removeZeroToThirtyOne(value);
}
int index = this.probe(value);
if (<(equals.(type))("this.table[index]", "value")>)
Expand All @@ -318,6 +311,18 @@ public final class <name>HashSet extends Abstract<name>Set implements Mutable<na
return false;
}

private boolean removeZeroToThirtyOne(<type> value)
{
int initial = this.zeroToThirtyOne;
this.zeroToThirtyOne &= ~(1 \<\< <(castRealTypeToInt.(type))("value")>);
if (this.zeroToThirtyOne == initial)
{
return false;
}
this.zeroToThirtyOneOccupied--;
return true;
}

public boolean removeAll(<name>Iterable source)
{
if (source.isEmpty())
Expand Down Expand Up @@ -1606,32 +1611,46 @@ public final class <name>HashSet extends Abstract<name>Set implements Mutable<na
return result;
}

public void remove()
{
if (this.count == 0)
{
throw new IllegalStateException();
}
<type> removeValue;
if (this.zeroToThirtyOne \<= <(literal.(type))("32")> && this.position == 0)
{
if (<name>HashSet.this.zeroToThirtyOne != (<name>HashSet.this.zeroToThirtyOne | 1 \<\< (<(castRealTypeToInt.(type))("this.zeroToThirtyOne")> - 1)))
{
throw new IllegalStateException();
}
removeValue = <(castIntToNarrowTypeWithParens.(type))("this.zeroToThirtyOne - 1")>;
}
else if (<(equals.(type))({<name>HashSet.this.table[this.position - 1]}, "REMOVED")>)
{
throw new IllegalStateException();
}
else
{
removeValue = <name>HashSet.this.table[this.position - 1];
}
<name>HashSet.this.remove(removeValue);
this.count--;
}
public void remove()
{
if (this.count == 0)
{
throw new IllegalStateException();
}
<type> removeValue;
if (this.zeroToThirtyOne \<= <(literal.(type))("32")> && this.position == 0)
{
if (<name>HashSet.this.zeroToThirtyOne != (<name>HashSet.this.zeroToThirtyOne | 1 \<\< (<(castRealTypeToInt.(type))("this.zeroToThirtyOne")> - 1)))
{
throw new IllegalStateException();
}
removeValue = <(castIntToNarrowTypeWithParens.(type))("this.zeroToThirtyOne - 1")>;
}
else if (<(equals.(type))({<name>HashSet.this.table[this.position - 1]}, "REMOVED")>)
{
throw new IllegalStateException();
}
else
{
removeValue = <name>HashSet.this.table[this.position - 1];
}
if (<name>HashSet.isBetweenZeroAndThirtyOne(removeValue))
{
<name>HashSet.this.removeZeroToThirtyOne(removeValue);
}
else if (<(equals.(type))({<name>HashSet.this.table[this.position - 1]}, "removeValue")>)
{
if (<name>HashSet.this.copyOnWrite)
{
<name>HashSet.this.copyTable();
}
<name>HashSet.this.table[position -1] = REMOVED;
<name>HashSet.this.occupiedWithData--;
<name>HashSet.this.occupiedWithSentinels++;
}

this.count--;
}
}
}

Expand Down Expand Up @@ -1716,4 +1735,4 @@ public <wideType.(type)> sum()
return result;
}

>>
>>
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,18 @@ targetPath() ::= "com/gs/collections/impl/set/mutable/primitive"
fileName(primitive) ::= "<primitive.name>HashSetTest"

class(primitive) ::= <<
<body(primitive.type, primitive.wrapperName, primitive.name)>
<body(primitive.type, primitive.wrapperName, primitive.name, primitive.charPrimitive, primitive.shortPrimitive)>
>>

body(type, wrapperName, name) ::= <<
body(type, wrapperName, name, charPrimitive, shortPrimitive) ::= <<
<copyright()>

package com.gs.collections.impl.set.mutable.primitive;

import java.lang.reflect.Field;

import com.gs.collections.api.iterator.Mutable<name>Iterator;
import com.gs.collections.api.set.primitive.Mutable<name>Set;
import com.gs.collections.impl.factory.primitive.<name>Sets;
import com.gs.collections.impl.list.mutable.primitive.<name>ArrayList;
import com.gs.collections.impl.test.Verify;
Expand Down Expand Up @@ -177,6 +179,39 @@ public class <name>HashSetTest extends Abstract<name>SetTestCase
Assert.assertEquals(0, occupiedWithSentinels.get(hashSet));
}

@Test
public void iterator_remove()
{
Mutable<name>Set set1 = <name>Sets.mutable.empty();

int <if(shortPrimitive)>max = 65_536<elseif(charPrimitive)>max = 10<else>max = 100_000<endif>;
for (Integer i = 0; i \< max; i++)
{
<if(charPrimitive)>set1.add(i.toString().charAt(0));<else>set1.add(i.<type>Value());<endif>
}

// set2 to verify copyTable()
Mutable<name>Set set2 = <name>Sets.mutable.withAll(set1);
set2.freeze();

this.assertIteratorRemove(set1, max);
this.assertIteratorRemove(set2, max);
}

private void assertIteratorRemove(Mutable<name>Set set, int max)
{
Verify.assertSize(max, set);
Mutable<name>Iterator iterator = set.<type>Iterator();
Verify.assertThrows(IllegalStateException.class, () -> iterator.remove());

while (iterator.hasNext())
{
iterator.next();
iterator.remove();
Verify.assertSize(--max, set);
}
}

@Test
public void addEverySlot()
{
Expand Down

0 comments on commit e222fac

Please sign in to comment.