Apache
/
commons-collections
mirror of https://github.com/apache/commons-collections.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #315 from Claudenw/remove_merge_in_place 

replaced merge with old mergeInPlace
This commit is contained in:
Claude Warren 2022-07-28 12:17:28 +01:00 committed by GitHub
parent ef728ec96b c104fd4924
commit ebc66db426
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 68 additions and 178 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/main/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilter.java
View File

@ -120,28 +120,7 @@ public final class ArrayCountingBloomFilter implements CountingBloomFilter {
}
@Override
public CountingBloomFilter merge(BloomFilter other) {
Objects.requireNonNull(other, "other");
CountingBloomFilter filter = copy();
filter.add(BitCountProducer.from(other));
return filter;
}
@Override
public CountingBloomFilter merge(Hasher hasher) {
Objects.requireNonNull(hasher, "hasher");
ArrayCountingBloomFilter filter = copy();
try {
filter.add(BitCountProducer.from(hasher.uniqueIndices(shape)));
} catch (IndexOutOfBoundsException e) {
throw new IllegalArgumentException(
String.format("Filter only accepts values in the [0,%d) range", shape.getNumberOfBits()));
}
return filter;
}
@Override
public boolean mergeInPlace(final BloomFilter other) {
public boolean merge(final BloomFilter other) {
Objects.requireNonNull(other, "other");
try {
return add(BitCountProducer.from(other));
@ -151,7 +130,7 @@ public final class ArrayCountingBloomFilter implements CountingBloomFilter {
}
@Override
public boolean mergeInPlace(final Hasher hasher) {
public boolean merge(final Hasher hasher) {
Objects.requireNonNull(hasher, "hasher");
try {
return add(BitCountProducer.from(hasher.uniqueIndices(shape)));

42
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java
View File

@ -112,38 +112,6 @@ public interface BloomFilter extends IndexProducer, BitMapProducer {
// update operations
/**
* Merges the specified Bloom filter with this Bloom filter creating a new Bloom filter.
*
* <p>Specifically all bit indexes that are enabled in the {@code other} and in @code this} filter will be
* enabled in the resulting filter.</p>
*
* @param other the other Bloom filter
* @return The new Bloom filter.
*/
default BloomFilter merge(BloomFilter other) {
Objects.requireNonNull(other, "other");
BloomFilter result = copy();
result.mergeInPlace(other);
return result;
}
/**
* Merges the specified Hasher with this Bloom filter and returns a new Bloom filter.
*
* <p>Specifically all bit indexes that are identified by the {@code hasher} and in {@code this} Bloom filter
* be enabled in the resulting filter.</p>
*
* @param hasher the hasher to provide the indices
* @return the new Bloom filter.
*/
default BloomFilter merge(Hasher hasher) {
Objects.requireNonNull(hasher, "hasher");
BloomFilter result = copy();
result.mergeInPlace(hasher);
return result;
}
/**
* Merges the specified Bloom filter into this Bloom filter.
*
@ -158,7 +126,7 @@ public interface BloomFilter extends IndexProducer, BitMapProducer {
* @param other The bloom filter to merge into this one.
* @return true if the merge was successful
*/
boolean mergeInPlace(BloomFilter other);
boolean merge(BloomFilter other);
/**
* Merges the specified hasher into this Bloom filter. Specifically all
@ -172,12 +140,12 @@ public interface BloomFilter extends IndexProducer, BitMapProducer {
* @param hasher The hasher to merge.
* @return true if the merge was successful
*/
default boolean mergeInPlace(Hasher hasher) {
default boolean merge(Hasher hasher) {
Objects.requireNonNull(hasher, "hasher");
Shape shape = getShape();
// create the bloomfilter that is most likely to merge quickly with this one
BloomFilter result = isSparse() ? new SparseBloomFilter(shape, hasher) : new SimpleBloomFilter(shape, hasher);
return mergeInPlace(result);
return merge(result);
}
// Counting Operations
@ -231,7 +199,9 @@ public interface BloomFilter extends IndexProducer, BitMapProducer {
*/
default int estimateUnion(BloomFilter other) {
Objects.requireNonNull(other, "other");
return this.merge(other).estimateN();
BloomFilter cpy = this.copy();
cpy.merge(other);
return cpy.estimateN();
}
/**

33
src/main/java/org/apache/commons/collections4/bloomfilter/CountingBloomFilter.java
View File

@ -144,38 +144,11 @@ public interface CountingBloomFilter extends BloomFilter, BitCountProducer {
*/
boolean subtract(BitCountProducer other);
/**
* Merges the specified Bloom filter into this Bloom filter to produce a new CountingBloomFilter.
*
* <p>Specifically the new Bloom filter will contain all the counts of this filter and in addition
* all bit indexes that are enabled in the {@code other} filter will be incremented
* by one in the new filter.</p>
*
* <p>Note: the validity of the resulting filter is not guaranteed. When in doubt {@code isValid()}
* should be called on the new filter.</p>
*
* @param other the other Bloom filter
* @return A new CountingBloomFilter instance.
*/
@Override
CountingBloomFilter merge(BloomFilter other);
/**
* Merges the specified hasher with this Bloom filter to create a new CountingBloomFilter.
*
* <p>Specifically the new Bloom filter will contain all the counts of this filter and in addition
* all bit indexes specified by the {@code hasher} will be incremented
* by one in the new filter.</p>
*
* <p>For HasherCollections each enclosed Hasher will be considered a single item and increment
* the counts separately.</p>
*
* <p>Note: the validity of the resulting filter is not guaranteed. When in doubt {@code isValid()}
* should be called on the new filter.</p>
*
* @param hasher the hasher to provide the indexes
* @return A new CountingBloomFilter instance.
* Creates a new instance of the CountingBloomFilter with the same properties as the current one.
* @return a copy of this CountingBloomFilter
*/
@Override
CountingBloomFilter merge(Hasher hasher);
CountingBloomFilter copy();
}

28
src/main/java/org/apache/commons/collections4/bloomfilter/SimpleBloomFilter.java
View File

@ -66,9 +66,9 @@ public final class SimpleBloomFilter implements BloomFilter {
this.bitMap = new long[BitMap.numberOfBitMaps(shape.getNumberOfBits())];
this.cardinality = 0;
if (other.isSparse()) {
mergeInPlace((IndexProducer) other);
merge((IndexProducer) other);
} else {
mergeInPlace((BitMapProducer) other);
merge((BitMapProducer) other);
}
}
@ -80,7 +80,7 @@ public final class SimpleBloomFilter implements BloomFilter {
public SimpleBloomFilter(final Shape shape, Hasher hasher) {
this(shape);
Objects.requireNonNull(hasher, "hasher");
mergeInPlace(hasher);
merge(hasher);
}
/**
@ -92,7 +92,7 @@ public final class SimpleBloomFilter implements BloomFilter {
public SimpleBloomFilter(final Shape shape, IndexProducer indices) {
this(shape);
Objects.requireNonNull(indices, "indices");
mergeInPlace(indices);
merge(indices);
}
/**
@ -104,7 +104,7 @@ public final class SimpleBloomFilter implements BloomFilter {
public SimpleBloomFilter(final Shape shape, BitMapProducer bitMaps) {
this(shape);
Objects.requireNonNull(bitMaps, "bitMaps");
mergeInPlace(bitMaps);
merge(bitMaps);
}
/**
@ -134,11 +134,11 @@ public final class SimpleBloomFilter implements BloomFilter {
}
/**
* Performs a merge in place using an IndexProducer.
* Performs a merge using an IndexProducer.
* @param indexProducer the IndexProducer to merge from.
* @throws IllegalArgumentException if producer sends illegal value.
*/
private void mergeInPlace(IndexProducer indexProducer) {
private void merge(IndexProducer indexProducer) {
indexProducer.forEachIndex(idx -> {
if (idx < 0 || idx >= shape.getNumberOfBits()) {
throw new IllegalArgumentException(String.format(
@ -151,11 +151,11 @@ public final class SimpleBloomFilter implements BloomFilter {
}
/**
* Performs a merge in place using an BitMapProducer.
* Performs a merge using an BitMapProducer.
* @param bitMapProducer the BitMapProducer to merge from.
* @throws IllegalArgumentException if producer sends illegal value.
*/
private void mergeInPlace(BitMapProducer bitMapProducer) {
private void merge(BitMapProducer bitMapProducer) {
try {
int[] idx = new int[1];
bitMapProducer.forEachBitMap(value -> {
@ -185,19 +185,19 @@ public final class SimpleBloomFilter implements BloomFilter {
}
@Override
public boolean mergeInPlace(Hasher hasher) {
public boolean merge(Hasher hasher) {
Objects.requireNonNull(hasher, "hasher");
mergeInPlace(hasher.indices(shape));
merge(hasher.indices(shape));
return true;
}
@Override
public boolean mergeInPlace(BloomFilter other) {
public boolean merge(BloomFilter other) {
Objects.requireNonNull(other, "other");
if (other.isSparse()) {
mergeInPlace((IndexProducer) other);
merge((IndexProducer) other);
} else {
mergeInPlace((BitMapProducer) other);
merge((BitMapProducer) other);
}
return true;
}

18
src/main/java/org/apache/commons/collections4/bloomfilter/SparseBloomFilter.java
View File

@ -59,9 +59,9 @@ public final class SparseBloomFilter implements BloomFilter {
this.shape = other.getShape();
this.indices = new TreeSet<>();
if (other.isSparse()) {
mergeInPlace((IndexProducer) other);
merge((IndexProducer) other);
} else {
mergeInPlace(IndexProducer.fromBitMapProducer(other));
merge(IndexProducer.fromBitMapProducer(other));
}
}
@ -108,7 +108,7 @@ public final class SparseBloomFilter implements BloomFilter {
public SparseBloomFilter(Shape shape, BitMapProducer bitMaps) {
this(shape);
Objects.requireNonNull(bitMaps, "bitMaps");
mergeInPlace(IndexProducer.fromBitMapProducer(bitMaps));
merge(IndexProducer.fromBitMapProducer(bitMaps));
}
private SparseBloomFilter(SparseBloomFilter source) {
@ -141,11 +141,11 @@ public final class SparseBloomFilter implements BloomFilter {
}
/**
* Performs a merge in place using an IndexProducer.
* Performs a merge using an IndexProducer.
* @param indexProducer the IndexProducer to merge from.
* @throws IllegalArgumentException if producer sends illegal value.
*/
private void mergeInPlace(IndexProducer indexProducer) {
private void merge(IndexProducer indexProducer) {
indexProducer.forEachIndex(this::add);
if (!this.indices.isEmpty()) {
if (this.indices.last() >= shape.getNumberOfBits()) {
@ -160,17 +160,17 @@ public final class SparseBloomFilter implements BloomFilter {
}
@Override
public boolean mergeInPlace(Hasher hasher) {
public boolean merge(Hasher hasher) {
Objects.requireNonNull(hasher, "hasher");
mergeInPlace(hasher.indices(shape));
merge(hasher.indices(shape));
return true;
}
@Override
public boolean mergeInPlace(BloomFilter other) {
public boolean merge(BloomFilter other) {
Objects.requireNonNull(other, "other");
IndexProducer producer = other.isSparse() ? (IndexProducer) other : IndexProducer.fromBitMapProducer(other);
mergeInPlace(producer);
merge(producer);
return true;
}

51
src/test/java/org/apache/commons/collections4/bloomfilter/AbstractBloomFilterTest.java
View File

@ -228,11 +228,11 @@ public abstract class AbstractBloomFilterTest<T extends BloomFilter> {
// the data provided above do not generate an estimate that is equivalent to the
// actual.
filter1.mergeInPlace(new SimpleHasher(4, 1));
filter1.merge(new SimpleHasher(4, 1));
assertEquals(1, filter1.estimateN());
filter1.mergeInPlace(new SimpleHasher(17, 1));
filter1.merge(new SimpleHasher(17, 1));
assertEquals(3, filter1.estimateN());
}
@ -275,41 +275,10 @@ public abstract class AbstractBloomFilterTest<T extends BloomFilter> {
@Test
public final void testMerge() {
// test with BloomFilter
final BloomFilter bf1 = createFilter(getTestShape(), from1);
final BloomFilter bf2 = createFilter(getTestShape(), from11);
final BloomFilter bf3 = bf1.merge(bf2);
assertTrue(bf3.contains(bf1), "Should contain bf1");
assertTrue(bf3.contains(bf2), "Should contain bf2");
final BloomFilter bf4 = bf2.merge(bf1);
assertTrue(bf4.contains(bf1), "Should contain bf1");
assertTrue(bf4.contains(bf2), "Should contain bf2");
assertTrue(bf4.contains(bf3), "Should contain bf3");
assertTrue(bf3.contains(bf4), "Should contain bf4");
// test with Hasher
final BloomFilter bf5 = bf1.merge(from11);
assertTrue(bf5.contains(bf1), "Should contain bf1");
assertTrue(bf5.contains(bf2), "Should contain bf2");
// test with hasher returning numbers out of range
assertThrows(IllegalArgumentException.class, () -> bf1.merge(new BadHasher(bf1.getShape().getNumberOfBits())));
assertThrows(IllegalArgumentException.class, () -> bf1.merge(new BadHasher(-1)));
}
/**
* Tests that merging in place works as expected.
*/
@Test
public final void testMergeInPlace() {
final BloomFilter bf1 = createFilter(getTestShape(), from1);
final BloomFilter bf2 = createFilter(getTestShape(), from11);
final BloomFilter bf3 = bf1.merge(bf2);
final BloomFilter bf3 = bf1.copy();
bf3.merge(bf2);
// test with BloomFilter
@ -318,7 +287,7 @@ public abstract class AbstractBloomFilterTest<T extends BloomFilter> {
for (int i = 0; i < bf1Val.length; i++) {
bf1Val[i] |= bf2Val[i];
}
bf1.mergeInPlace(bf2);
bf1.merge(bf2);
long[] bf1New = bf1.asBitMapArray();
for (int i = 0; i < bf1Val.length; i++) {
@ -331,26 +300,26 @@ public abstract class AbstractBloomFilterTest<T extends BloomFilter> {
// test with hasher
BloomFilter bf4 = createFilter(getTestShape(), from1);
bf4.mergeInPlace(from11);
bf4.merge(from11);
assertTrue(bf4.contains(bf2), "Should contain Bf2");
assertTrue(bf4.contains(bf3), "Should contain Bf3");
// test with hasher returning numbers out of range
assertThrows(IllegalArgumentException.class,
() -> bf1.mergeInPlace(new BadHasher(bf1.getShape().getNumberOfBits())));
assertThrows(IllegalArgumentException.class, () -> bf1.mergeInPlace(new BadHasher(-1)));
() -> bf1.merge(new BadHasher(bf1.getShape().getNumberOfBits())));
assertThrows(IllegalArgumentException.class, () -> bf1.merge(new BadHasher(-1)));
// test error when bloom filter returns values out of range
final BloomFilter bf5 = new SimpleBloomFilter(
Shape.fromKM(getTestShape().getNumberOfHashFunctions(), 3 * Long.SIZE),
new SimpleHasher(Long.SIZE * 2, 1));
assertThrows(IllegalArgumentException.class, () -> bf1.mergeInPlace(bf5));
assertThrows(IllegalArgumentException.class, () -> bf1.merge(bf5));
final BloomFilter bf6 = new SparseBloomFilter(
Shape.fromKM(getTestShape().getNumberOfHashFunctions(), 3 * Long.SIZE),
new SimpleHasher(Long.SIZE * 2, 1));
assertThrows(IllegalArgumentException.class, () -> bf1.mergeInPlace(bf6));
assertThrows(IllegalArgumentException.class, () -> bf1.merge(bf6));
}
private void assertIndexProducerConstructor(Shape shape, int[] values, int[] expected) {

14
src/test/java/org/apache/commons/collections4/bloomfilter/AbstractCountingBloomFilterTest.java
View File

@ -114,11 +114,13 @@ public abstract class AbstractCountingBloomFilterTest<T extends CountingBloomFil
final BloomFilter bf2 = new SimpleBloomFilter(getTestShape(), from11);
final BloomFilter bf3 = bf1.merge(bf2);
final BloomFilter bf3 = bf1.copy();
bf3.merge(bf2);
assertTrue(bf3.contains(bf1), "Should contain");
assertTrue(bf3.contains(bf2), "Should contain");
final BloomFilter bf4 = bf2.merge(bf1);
final BloomFilter bf4 = bf2.copy();
bf4.merge(bf1);
assertTrue(bf4.contains(bf1), "Should contain");
assertTrue(bf4.contains(bf2), "Should contain");
assertTrue(bf4.contains(bf3), "Should contain");
@ -130,7 +132,8 @@ public abstract class AbstractCountingBloomFilterTest<T extends CountingBloomFil
assertTrue(bf5.add(maximumValueProducer), "Should add to empty");
assertTrue(bf5.isValid(), "Should be valid");
CountingBloomFilter bf6 = bf5.merge(new SimpleBloomFilter(getTestShape(), from1));
CountingBloomFilter bf6 = bf5.copy();
bf6.merge(new SimpleBloomFilter(getTestShape(), from1));
assertFalse(bf6.isValid(), "Should not be valid");
}
@ -241,7 +244,7 @@ public abstract class AbstractCountingBloomFilterTest<T extends CountingBloomFil
});
bf1 = createEmptyFilter(shape);
bf1.mergeInPlace(hasher);
bf1.merge(hasher);
assertEquals(6, bf1.cardinality());
bf1.forEachCount((x, y) -> {
assertEquals(1, y, "Hasher in mergeInPlace results in value not equal to 1");
@ -249,7 +252,8 @@ public abstract class AbstractCountingBloomFilterTest<T extends CountingBloomFil
});
bf1 = createEmptyFilter(shape);
CountingBloomFilter bf2 = bf1.merge(hasher);
CountingBloomFilter bf2 = bf1.copy();
bf2.merge(hasher);
assertEquals(6, bf2.cardinality());
bf2.forEachCount((x, y) -> {
assertEquals(1, y, "Hasher in merge results in value not equal to 1");

3
src/test/java/org/apache/commons/collections4/bloomfilter/BitCountProducerFromArrayCountingBloomFilterTest.java
View File

@ -24,7 +24,8 @@ public class BitCountProducerFromArrayCountingBloomFilterTest extends AbstractBi
protected BitCountProducer createProducer() {
ArrayCountingBloomFilter filter = new ArrayCountingBloomFilter(shape);
Hasher hasher = new SimpleHasher(0, 1);
return filter.merge(hasher);
filter.merge(hasher);
return filter;
}
@Override

3
src/test/java/org/apache/commons/collections4/bloomfilter/BitMapProducerFromArrayCountingBloomFilterTest.java
View File

@ -24,7 +24,8 @@ public class BitMapProducerFromArrayCountingBloomFilterTest extends AbstractBitM
protected BitMapProducer createProducer() {
ArrayCountingBloomFilter filter = new ArrayCountingBloomFilter(shape);
Hasher hasher = new SimpleHasher(0, 1);
return filter.merge(hasher);
filter.merge(hasher);
return filter;
}
@Override

20
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java
View File

@ -50,27 +50,21 @@ public class DefaultBloomFilterTest extends AbstractBloomFilterTest<DefaultBloom
}
@Test
public void testDefaultBloomFilterSimpleSpecificMergeInPlace() {
public void testDefaultBloomFilterSimpleSpecificMerge() {
AbstractDefaultBloomFilter filter = new SparseDefaultBloomFilter(Shape.fromKM(3, 150));
Hasher hasher = new SimpleHasher(0, 1);
assertTrue(filter.mergeInPlace(hasher));
assertTrue(filter.merge(hasher));
assertEquals(3, filter.cardinality());
}
@Test
public void testDefaultBloomFilterSparseSpecificMergeInPlace() {
AbstractDefaultBloomFilter filter = new SparseDefaultBloomFilter(Shape.fromKM(3, 150));
Hasher hasher = new SimpleHasher(0, 1);
BloomFilter newFilter = filter.merge(hasher);
assertEquals(3, newFilter.cardinality());
}
@Test
public void testDefaultBloomFilterSparseSpecificMerge() {
Shape shape = Shape.fromKM(3, 150);
AbstractDefaultBloomFilter filter = new SparseDefaultBloomFilter(shape);
AbstractDefaultBloomFilter filter2 = new SparseDefaultBloomFilter(shape, new SimpleHasher(0, 1));
BloomFilter newFilter = filter.merge(filter2);
BloomFilter newFilter = filter.copy();
newFilter.merge(filter2);
assertEquals(3, newFilter.cardinality());
}
@ -79,12 +73,12 @@ public class DefaultBloomFilterTest extends AbstractBloomFilterTest<DefaultBloom
Hasher hasher = new SimpleHasher(1, 1);
BloomFilter bf1 = new NonSparseDefaultBloomFilter(getTestShape());
bf1.mergeInPlace(hasher);
bf1.merge(hasher);
assertTrue(BitMapProducer.fromIndexProducer(hasher.indices(getTestShape()), getTestShape().getNumberOfBits())
.forEachBitMapPair(bf1, (x, y) -> x == y));
bf1 = new SparseDefaultBloomFilter(getTestShape());
bf1.mergeInPlace(hasher);
bf1.merge(hasher);
assertTrue(BitMapProducer.fromIndexProducer(hasher.indices(getTestShape()), getTestShape().getNumberOfBits())
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
@ -149,7 +143,7 @@ public class DefaultBloomFilterTest extends AbstractBloomFilterTest<DefaultBloom
}
@Override
public boolean mergeInPlace(BloomFilter other) {
public boolean merge(BloomFilter other) {
other.forEachIndex((i) -> {
indices.add(i);
return true;

2
src/test/java/org/apache/commons/collections4/bloomfilter/HasherCollectionTest.java
View File

@ -126,7 +126,7 @@ public class HasherCollectionTest extends AbstractHasherTest {
ArrayCountingBloomFilter bf = new ArrayCountingBloomFilter(Shape.fromKM(5, 10000));
// Should add h1, h1, h2, h3
Assertions.assertTrue(bf.mergeInPlace(hc3));
Assertions.assertTrue(bf.merge(hc3));
Assertions.assertTrue(bf.remove(h1));
Assertions.assertTrue(bf.remove(h1));
Assertions.assertNotEquals(0, bf.cardinality());

3
src/test/java/org/apache/commons/collections4/bloomfilter/IndexProducerFromArrayCountingBloomFilterTest.java
View File

@ -24,7 +24,8 @@ public class IndexProducerFromArrayCountingBloomFilterTest extends AbstractIndex
protected IndexProducer createProducer() {
ArrayCountingBloomFilter filter = new ArrayCountingBloomFilter(shape);
Hasher hasher = new SimpleHasher(0, 1);
return filter.merge(hasher);
filter.merge(hasher);
return filter;
}
@Override

1
src/test/java/org/apache/commons/collections4/bloomfilter/SimpleBloomFilterTest.java
View File

@ -50,7 +50,6 @@ public class SimpleBloomFilterTest extends AbstractBloomFilterTest<SimpleBloomFi
nestedTest.testEstimateUnion();
nestedTest.testIsFull();
nestedTest.testMerge();
nestedTest.testMergeInPlace();
}
@Test

3
src/test/java/org/apache/commons/collections4/bloomfilter/SparseBloomFilterTest.java
View File

@ -53,7 +53,6 @@ public class SparseBloomFilterTest extends AbstractBloomFilterTest<SparseBloomFi
nestedTest.testEstimateUnion();
nestedTest.testIsFull();
nestedTest.testMerge();
nestedTest.testMergeInPlace();
}
@Test
@ -144,7 +143,7 @@ public class SparseBloomFilterTest extends AbstractBloomFilterTest<SparseBloomFi
public void testBloomFilterBasedMergeInPlaceEdgeCases() {
BloomFilter bf1 = createEmptyFilter(getTestShape());
BloomFilter bf2 = new SimpleBloomFilter(getTestShape(), from1);
bf1.mergeInPlace(bf2);
bf1.merge(bf2);
assertTrue(bf2.forEachBitMapPair(bf1, (x, y) -> x == y));
}
}