From 080380b18a9f8b8c8772a2d665eb09e0632eab2f Mon Sep 17 00:00:00 2001 From: Tim Hennekey Date: Wed, 15 Jan 2020 18:13:20 -0500 Subject: [PATCH 1/3] Use Guice instead of custom implementation This uses the avaialable code in Guice rather than a custom implementation. It also provides a performance increase (as demonstrated by the unit tests) --- .../org/archive/util/BloomFilter64bit.java | 257 +++--------------- 1 file changed, 44 insertions(+), 213 deletions(-) diff --git a/commons/src/main/java/org/archive/util/BloomFilter64bit.java b/commons/src/main/java/org/archive/util/BloomFilter64bit.java index eaf57874..b2ba81b6 100644 --- a/commons/src/main/java/org/archive/util/BloomFilter64bit.java +++ b/commons/src/main/java/org/archive/util/BloomFilter64bit.java @@ -27,91 +27,30 @@ package org.archive.util; import java.io.Serializable; +import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.security.SecureRandom; import java.util.Random; -/** A Bloom filter. - * - * ADAPTED/IMPROVED VERSION OF MG4J it.unimi.dsi.mg4j.util.BloomFilter - * - *

KEY CHANGES: - * - *

- * - *
- * - *

Instances of this class represent a set of character sequences (with - * false positives) using a Bloom filter. Because of the way Bloom filters work, - * you cannot remove elements. - * - *

Bloom filters have an expected error rate, depending on the number - * of hash functions used, on the filter size and on the number of elements in - * the filter. This implementation uses a variable optimal number of hash - * functions, depending on the expected number of elements. More precisely, a - * Bloom filter for n character sequences with d hash - * functions will use ln 2 dn ≈ - * 1.44 dn bits; false positives will happen with - * probability 2-d. - * - *

Hash functions are generated at creation time using universal hashing. - * Each hash function uses {@link #NUMBER_OF_WEIGHTS} random integers, which - * are cyclically multiplied by the character codes in a character sequence. - * The resulting integers are XOR-ed together. - * - *

This class exports access methods that are very similar to those of - * {@link java.util.Set}, but it does not implement that interface, as too - * many non-optional methods would be unimplementable (e.g., iterators). - * - * @author Sebastiano Vigna - * @author Gordon Mohr - */ +import com.google.common.annotations.VisibleForTesting; +import com.google.common.hash.Funnels; +import com.google.common.primitives.Ints; + public class BloomFilter64bit implements Serializable, BloomFilter { - private static final long serialVersionUID = 2L; + private static final long serialVersionUID = 3L; - /** The number of weights used to create hash functions. */ - protected final static int NUMBER_OF_WEIGHTS = 2083; // CHANGED FROM 16 - /** The number of bits in this filter. */ - final protected long m; - /** if bitfield is an exact power of 2 in length, it is this power */ - protected int power = -1; /** The expected number of inserts; determines calculated size */ - final protected long expectedInserts; - /** The number of hash functions used by this filter. */ - final protected int d; - /** The underlying bit vector */ - final protected long[][] bits; - /** The random integers used to generate the hash functions. */ - final protected long[][] weight; + private final long expectedInserts; /** The number of elements currently in the filter. It may be * smaller than the actual number of additions of distinct character * sequences because of false positives. */ - protected int size; + private int size; - /** The natural logarithm of 2, used in the computation of the number of bits. */ - protected final static double NATURAL_LOG_OF_2 = Math.log( 2 ); - - /** power-of-two to use as maximum size of bitfield subarrays */ - protected final static int SUBARRAY_POWER_OF_TWO = 26; // 512MiB of longs - /** number of longs in one subarray */ - protected final static int SUBARRAY_LENGTH_IN_LONGS = 1 << SUBARRAY_POWER_OF_TWO; - /** mask for lowest SUBARRAY_POWER_OF_TWO bits */ - protected final static int SUBARRAY_MASK = SUBARRAY_LENGTH_IN_LONGS - 1; //0x0FFFFFFF - - protected final static boolean DEBUG = false; + private final com.google.common.hash.BloomFilter delegate; + private final long bitSize; + private final int numHashFunctions; /** Creates a new Bloom filter with given number of hash functions and * expected number of elements. @@ -141,45 +80,18 @@ public class BloomFilter64bit implements Serializable, BloomFilter { * @param roundUp if true, round bit size up to next-nearest-power-of-2 */ public BloomFilter64bit(final long n, final int d, Random weightsGenerator, boolean roundUp ) { + delegate = com.google.common.hash.BloomFilter.create(Funnels.unencodedCharsFunnel(), Ints.saturatedCast(n), 0.0000003); this.expectedInserts = n; - this.d = d; - long lenInLongs = (long)Math.ceil( ( (long)n * (long)d / NATURAL_LOG_OF_2 ) / 64L ); - if ( lenInLongs > (1L<<48) ) { - throw new IllegalArgumentException( - "This filter would require " + lenInLongs + " longs, " + - "greater than this classes maximum of 2^48 longs (2PiB)." ); - } - long lenInBits = lenInLongs * 64L; - - if(roundUp) { - int pow = 0; - while((1L<s. - * @param k a hash function index (smaller than {@link #d}). - * @return the position in the filter corresponding to s for the hash function k. - */ - protected long hash( final CharSequence s, final int l, final int k ) { - final long[] w = weight[ k ]; - long h = 0; - int i = l; - while( i-- != 0 ) h ^= s.charAt( i ) * w[ i % NUMBER_OF_WEIGHTS ]; - long retVal; - if(power>0) { - retVal = h >>> (64-power); - } else { - // ####----####---- - retVal = ( h & 0x7FFFFFFFFFFFFFFFL ) % m; - } - return retVal; - } - - public long[] bitIndexesFor(CharSequence s) { - long[] ret = new long[d]; - for(int i = 0; i < d; i++) { - ret[i] = hash(s,s.length(),i); - } - return ret; - } - /** Checks whether the given character sequence is in this filter. * *

Note that this method may return true on a character sequence that is has @@ -237,9 +119,7 @@ public class BloomFilter64bit implements Serializable, BloomFilter { */ public boolean contains( final CharSequence s ) { - int i = d, l = s.length(); - while( i-- != 0 ) if ( ! getBit( hash( s, l, i ) ) ) return false; - return true; + return delegate.mightContain(s); } /** Adds a character sequence to the filter. @@ -249,79 +129,16 @@ public class BloomFilter64bit implements Serializable, BloomFilter { */ public boolean add( final CharSequence s ) { - boolean result = false; - int i = d, l = s.length(); - long h; - while( i-- != 0 ) { - h = hash( s, l, i ); - if ( ! setGetBit( h ) ) { - result = true; - } - } - if ( result ) size++; - return result; - } - - protected final static long ADDRESS_BITS_PER_UNIT = 6; // 64=2^6 - protected final static long BIT_INDEX_MASK = (1<<6)-1; // = 63 = 2^BITS_PER_UNIT - 1; - - /** - * Returns from the local bitvector the value of the bit with - * the specified index. The value is true if the bit - * with the index bitIndex is currently set; otherwise, - * returns false. - * - * (adapted from cern.colt.bitvector.QuickBitVector) - * - * @param bitIndex the bit index. - * @return the value of the bit with the specified index. - */ - public boolean getBit(long bitIndex) { - long longIndex = bitIndex >>> ADDRESS_BITS_PER_UNIT; - int arrayIndex = (int) (longIndex >>> SUBARRAY_POWER_OF_TWO); - int subarrayIndex = (int) (longIndex & SUBARRAY_MASK); - return ((bits[arrayIndex][subarrayIndex] & (1L << (bitIndex & BIT_INDEX_MASK))) != 0); + size++; + return delegate.put(s); } - /** - * Changes the bit with index bitIndex in local bitvector. - * - * (adapted from cern.colt.bitvector.QuickBitVector) - * - * @param bitIndex the index of the bit to be set. + /* (non-Javadoc) + * @see org.archive.util.BloomFilter#getSizeBytes() */ - protected void setBit( long bitIndex) { - long longIndex = bitIndex >>> ADDRESS_BITS_PER_UNIT; - int arrayIndex = (int) (longIndex >>> SUBARRAY_POWER_OF_TWO); - int subarrayIndex = (int) (longIndex & SUBARRAY_MASK); - bits[arrayIndex][subarrayIndex] |= (1L << (bitIndex & BIT_INDEX_MASK)); + public long getSizeBytes() { + return bitSize / 8; } - - /** - * Sets the bit with index bitIndex in local bitvector -- - * returning the old value. - * - * (adapted from cern.colt.bitvector.QuickBitVector) - * - * @param bitIndex the index of the bit to be set. - */ - protected boolean setGetBit( long bitIndex) { - long longIndex = bitIndex >>> ADDRESS_BITS_PER_UNIT; - int arrayIndex = (int) (longIndex >>> SUBARRAY_POWER_OF_TWO); - int subarrayIndex = (int) (longIndex & SUBARRAY_MASK); - long mask = 1L << (bitIndex & BIT_INDEX_MASK); - boolean ret = (bits[arrayIndex][subarrayIndex] & mask)!=0; - bits[arrayIndex][subarrayIndex] |= mask; - return ret; - } - - /* (non-Javadoc) - * @see org.archive.util.BloomFilter#getSizeBytes() - */ - public long getSizeBytes() { - // account for ragged-sized last array - return 8*(((bits.length-1)*bits[0].length)+bits[bits.length-1].length); - } @Override public long getExpectedInserts() { @@ -330,6 +147,20 @@ public class BloomFilter64bit implements Serializable, BloomFilter { @Override public long getHashCount() { - return d; + return numHashFunctions; + } + + @VisibleForTesting + public boolean getBit(long bitIndex) { + try { + Field bitsField = delegate.getClass().getDeclaredField("bits"); + bitsField.setAccessible(true); + Object bitarray = bitsField.get(delegate); + Method getBitMethod = bitarray.getClass().getDeclaredMethod("get", long.class); + getBitMethod.setAccessible(true); + return (boolean) getBitMethod.invoke(bitarray, bitIndex); + } catch (Exception e) { + throw new RuntimeException(e); + } } } From 33458f1518843a359b10e9731b456be40969c425 Mon Sep 17 00:00:00 2001 From: Tim Hennekey Date: Wed, 22 Jan 2020 17:13:58 -0500 Subject: [PATCH 2/3] Fix assertions By using assertEquals and seting the expected and actual values, the failure messages become a bit more useful. --- .../org/archive/crawler/util/BloomUriUniqFilterTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/engine/src/test/java/org/archive/crawler/util/BloomUriUniqFilterTest.java b/engine/src/test/java/org/archive/crawler/util/BloomUriUniqFilterTest.java index 28390704..81e3c1a5 100644 --- a/engine/src/test/java/org/archive/crawler/util/BloomUriUniqFilterTest.java +++ b/engine/src/test/java/org/archive/crawler/util/BloomUriUniqFilterTest.java @@ -67,7 +67,7 @@ implements UriUniqFilter.CrawlUriReceiver { this.filter.addForce(this.getUri(), new CrawlURI(UURIFactory.getInstance(this.getUri()))); // Should only have add 'this' once. - assertTrue("Count is off", this.filter.count() == 1); + assertEquals("Count is off", 1, this.filter.count()); } /** @@ -104,8 +104,7 @@ implements UriUniqFilter.CrawlUriReceiver { logger.fine("Readded subset " + list.size() + " in " + (System.currentTimeMillis() - start)); - assertTrue("Count is off: " + filter.count(), - filter.count() == MAX_COUNT); + assertEquals("Count is off", MAX_COUNT, filter.count()); } public void testNote() { From 9cb9563da3add0ef18fa0811554b936e0a4dde66 Mon Sep 17 00:00:00 2001 From: Tim Hennekey Date: Wed, 22 Jan 2020 17:15:24 -0500 Subject: [PATCH 3/3] Increment the count only when the filter notes it Otherwise this is a count of how many times this add method is called, not how many times an element was noted as being actually added. --- .../src/main/java/org/archive/util/BloomFilter64bit.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/commons/src/main/java/org/archive/util/BloomFilter64bit.java b/commons/src/main/java/org/archive/util/BloomFilter64bit.java index b2ba81b6..9c048a59 100644 --- a/commons/src/main/java/org/archive/util/BloomFilter64bit.java +++ b/commons/src/main/java/org/archive/util/BloomFilter64bit.java @@ -129,8 +129,11 @@ public class BloomFilter64bit implements Serializable, BloomFilter { */ public boolean add( final CharSequence s ) { - size++; - return delegate.put(s); + boolean added = delegate.put(s); + if (added) { + size++; + } + return added; } /* (non-Javadoc)