Skip to content

Commit

Permalink
[brettwooldridge#15] previousSetBit resets word length after miss
Browse files Browse the repository at this point in the history
Currently it sets the length to the position of the needle in the first
word it looks up and only searches indices below that. This is incorrect
after the initial word where we need to search from the top.
  • Loading branch information
BrentDouglas committed Nov 10, 2018
1 parent e8d1c80 commit f5a8413
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 26 deletions.
61 changes: 35 additions & 26 deletions src/main/java/com/zaxxer/sparsebits/SparseBitSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,21 @@ when choosing to increase the size (see resize()). The only place where
*/
protected static final int SHIFT1 = LEVEL2 + LEVEL3;

/**
* LENGTH2_SIZE is maximum index of a LEVEL2 page.
*/
protected static final int LENGTH2_SIZE = LENGTH2 - 1;

/**
* LENGTH3_SIZE is maximum index of a LEVEL3 page.
*/
protected static final int LENGTH3_SIZE = LENGTH3 - 1;

/**
* LENGTH4_SIZE is maximum index of a bit in a LEVEL4 word.
*/
protected static final int LENGTH4_SIZE = LENGTH4 - 1;

/**
* Holds reference to the cache of statistics values computed by the
* UpdateStrategy
Expand Down Expand Up @@ -1044,41 +1059,34 @@ public int previousClearBit(int i)
if (w1 > aLength - 1)
return i;
w1 = Math.min(w1, aLength - 1);
final int w4 = i % LENGTH4;
int w4 = i % LENGTH4;

long word;
long[][] a2;
long[] a3;

final int f3 = w3;
final int f2 = w2;
final int f1 = w1;

for (; w1 >= 0; --w1)
{
if ((a2 = bits[w1]) == null)
return (((w1 << SHIFT1) + (w2 << SHIFT2) + w3) << SHIFT3)
+ (f1 == w1 ? w4 : LENGTH4 - 1);
return (((w1 << SHIFT1) + (w2 << SHIFT2) + w3) << SHIFT3) + w4;
for (; w2 >= 0; --w2)
{
if ((a3 = a2[w2]) == null)
return (((w1 << SHIFT1) + (w2 << SHIFT2) + w3) << SHIFT3)
+ (f2 == w2 ? w4 : LENGTH4 - 1);
return (((w1 << SHIFT1) + (w2 << SHIFT2) + w3) << SHIFT3) + w4;
for (; w3 >= 0; --w3)
{
if ((word = a3[w3]) == 0)
return (((w1 << SHIFT1) + (w2 << SHIFT2) + w3) << SHIFT3)
+ (f3 == w3 ? w4 : LENGTH4 - 1);
return (((w1 << SHIFT1) + (w2 << SHIFT2) + w3) << SHIFT3) + w4;
for (int bitIdx = w4; bitIdx >= 0; --bitIdx)
{
if ((word & (1L << bitIdx)) == 0)
return (((w1 << SHIFT1) + (w2 << SHIFT2) + w3) << SHIFT3) + bitIdx;
}
w4 = LENGTH4_SIZE;
}
w3 = LENGTH3 - 1;
w3 = LENGTH3_SIZE;
}
w2 = LENGTH2 - 1;
w3 = LENGTH3 - 1;
w2 = LENGTH2_SIZE;
}
return -1;
}
Expand Down Expand Up @@ -1117,40 +1125,41 @@ public int previousSetBit(int i)
if (w1 > aLength - 1)
{
w1 = aLength - 1;
w2 = LENGTH2 - 1;
w3 = LENGTH3 - 1;
w4 = LENGTH4 - 1;
w2 = LENGTH2_SIZE;
w3 = LENGTH3_SIZE;
w4 = LENGTH4_SIZE;
}
else
{
w2 = (w >> SHIFT2) & MASK2;
w3 = w & MASK3;
w4 = i % LENGTH4;
}
boolean initialWord = true;

long word;
long[][] a2;
long[] a3;
for (; w1 >= 0; --w1, initialWord = false)
for (; w1 >= 0; --w1)
{
if ((a2 = bits[w1]) != null)
for (; w2 >= 0; --w2, initialWord = false)
for (; w2 >= 0; --w2)
{
if ((a3 = a2[w2]) != null)
for (; w3 >= 0; --w3, initialWord = false)
for (; w3 >= 0; --w3)
{
if ((word = a3[w3]) != 0)
for (int bitIdx = (initialWord ? w4 : LENGTH4 - 1); bitIdx >= 0; --bitIdx)
for (int bitIdx = w4; bitIdx >= 0; --bitIdx)
{
if ((word & (1L << bitIdx)) != 0)
return (((w1 << SHIFT1) + (w2 << SHIFT2) + w3) << SHIFT3) + bitIdx;
}
w4 = LENGTH4_SIZE;
}
w3 = LENGTH3 - 1;
w3 = LENGTH3_SIZE;
w4 = LENGTH4_SIZE;
}
w2 = LENGTH2 - 1;
w3 = LENGTH3 - 1;
w2 = LENGTH2_SIZE;
w3 = LENGTH3_SIZE;
w4 = LENGTH4_SIZE;
}
return -1;
}
Expand Down
10 changes: 10 additions & 0 deletions src/test/java/com/zaxxer/sparsebits/PreviousClearBitTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -206,4 +206,14 @@ public void randomMultiEntry() throws Exception {
values.clear();
}
}

@Test
public void bug15() throws Exception {
set.set(1);
set.set(64);
assertEquals(63, set.previousClearBit(64));
set.clear(0);
set.set(1);
assertEquals(63, set.previousClearBit(64));
}
}

0 comments on commit f5a8413

Please sign in to comment.