Skip to content

Various improvements to Bloom Filter work #4986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
import java.util.Map;
import java.util.concurrent.Semaphore;
import java.util.concurrent.atomic.AtomicReference;

import org.junit.After;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -1092,11 +1091,11 @@ public void resumingAQueryShouldUseBloomFilterToAvoidFullRequery() throws Except
// bloom filter, and it was used to avert a full requery.
AtomicReference<QuerySnapshot> snapshot2Ref = new AtomicReference<>();
ArrayList<ExistenceFilterMismatchInfo> existenceFilterMismatches =
captureExistenceFilterMismatches(
() -> {
QuerySnapshot querySnapshot = waitFor(collection.get());
snapshot2Ref.set(querySnapshot);
});
captureExistenceFilterMismatches(
() -> {
QuerySnapshot querySnapshot = waitFor(collection.get());
snapshot2Ref.set(querySnapshot);
});
QuerySnapshot snapshot2 = snapshot2Ref.get();

// Verify that the snapshot from the resumed query contains the expected documents; that is,
Expand Down Expand Up @@ -1132,8 +1131,8 @@ public void resumingAQueryShouldUseBloomFilterToAvoidFullRequery() throws Except
// Verify that Watch sent an existence filter with the correct counts when the query was
// resumed.
assertWithMessage("Watch should have sent exactly 1 existence filter")
.that(existenceFilterMismatches)
.hasSize(1);
.that(existenceFilterMismatches)
.hasSize(1);
ExistenceFilterMismatchInfo existenceFilterMismatchInfo = existenceFilterMismatches.get(0);
assertWithMessage("localCacheCount")
.that(existenceFilterMismatchInfo.localCacheCount())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import com.google.firebase.firestore.ListenerRegistration;
import java.util.ArrayList;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,42 +17,82 @@
import android.util.Base64;
import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;
import com.google.protobuf.ByteString;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;

public class BloomFilter {
public final class BloomFilter {
private final int bitCount;
private final byte[] bitmap;
private final ByteString bitmap;
private final int hashCount;
private final MessageDigest md5HashMessageDigest;

public BloomFilter(@NonNull byte[] bitmap, int padding, int hashCount) {
if (bitmap == null) {
throw new NullPointerException("Bitmap cannot be null.");
}
/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding the docs.

* Creates a new {@link BloomFilter} with the given parameters.
*
* @param bitmap the bitmap of the bloom filter; must not be null.
* @param padding the padding, in bits, of the last byte of the bloom filter; must be between 0
* (zero) and 7, inclusive; must be 0 (zero) if {@code bitmap.length==0}.
* @param hashCount The number of hash functions to use; must be strictly greater than zero; may
* be 0 (zero) if and only if {@code bitmap.length==0}.
*/
public BloomFilter(@NonNull ByteString bitmap, int padding, int hashCount) {
if (padding < 0 || padding >= 8) {
throw new BloomFilterException("Invalid padding: " + padding);
throw new IllegalArgumentException("Invalid padding: " + padding);
}
if (hashCount < 0) {
throw new BloomFilterException("Invalid hash count: " + hashCount);
throw new IllegalArgumentException("Invalid hash count: " + hashCount);
}
if (bitmap.length > 0 && hashCount == 0) {
if (bitmap.size() > 0 && hashCount == 0) {
// Only empty bloom filter can have 0 hash count.
throw new BloomFilterException("Invalid hash count: " + hashCount);
throw new IllegalArgumentException("Invalid hash count: " + hashCount);
}
if (bitmap.length == 0 && padding != 0) {
if (bitmap.size() == 0 && padding != 0) {
// Empty bloom filter should have 0 padding.
throw new BloomFilterException(
throw new IllegalArgumentException(
"Expected padding of 0 when bitmap length is 0, but got " + padding);
}

this.bitmap = bitmap;
this.hashCount = hashCount;
this.bitCount = bitmap.length * 8 - padding;
this.bitCount = bitmap.size() * 8 - padding;
this.md5HashMessageDigest = createMd5HashMessageDigest();
}

/**
* Creates an instance of {@link BloomFilter} with the given arguments, throwing a well-defined
* exception if the given arguments do not satisfy the requirements documented in the {@link
* BloomFilter} constructor.
*/
public static BloomFilter create(@NonNull ByteString bitmap, int padding, int hashCount)
throws BloomFilterCreateException {
if (padding < 0 || padding >= 8) {
throw new BloomFilterCreateException("Invalid padding: " + padding);
}
if (hashCount < 0) {
throw new BloomFilterCreateException("Invalid hash count: " + hashCount);
}
if (bitmap.size() > 0 && hashCount == 0) {
// Only empty bloom filter can have 0 hash count.
throw new BloomFilterCreateException("Invalid hash count: " + hashCount);
}
if (bitmap.size() == 0 && padding != 0) {
// Empty bloom filter should have 0 padding.
throw new BloomFilterCreateException(
"Expected padding of 0 when bitmap length is 0, but got " + padding);
}

return new BloomFilter(bitmap, padding, hashCount);
}

/** Exception thrown by {@link #create} if the given arguments are not valid. */
public static final class BloomFilterCreateException extends Exception {
public BloomFilterCreateException(String message) {
super(message);
}
}

@VisibleForTesting
int getBitCount() {
return this.bitCount;
Expand Down Expand Up @@ -146,7 +186,7 @@ private static long unsignedRemainder(long dividend, long divisor) {
/** Return whether the bit at the given index in the bitmap is set to 1. */
private boolean isBitSet(int index) {
// To retrieve bit n, calculate: (bitmap[n / 8] & (0x01 << (n % 8))).
byte byteAtIndex = this.bitmap[index / 8];
byte byteAtIndex = this.bitmap.byteAt(index / 8);
int offset = index % 8;
return (byteAtIndex & (0x01 << offset)) != 0;
}
Expand All @@ -159,7 +199,7 @@ public String toString() {
+ ", size="
+ bitCount
+ ", bitmap=\""
+ Base64.encodeToString(bitmap, Base64.NO_WRAP)
+ Base64.encodeToString(bitmap.toByteArray(), Base64.NO_WRAP)
+ "\"}";
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.google.firebase.firestore.remote.WatchChange.ExistenceFilterWatchChange;
import com.google.firebase.firestore.remote.WatchChange.WatchTargetChange;
import com.google.firebase.firestore.util.Logger;
import com.google.protobuf.ByteString;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -252,14 +253,14 @@ private BloomFilterApplicationStatus applyBloomFilter(
return BloomFilterApplicationStatus.SKIPPED;
}

byte[] bitmap = unchangedNames.getBits().getBitmap().toByteArray();
ByteString bitmap = unchangedNames.getBits().getBitmap();
BloomFilter bloomFilter;

try {
bloomFilter =
new BloomFilter(
BloomFilter.create(
bitmap, unchangedNames.getBits().getPadding(), unchangedNames.getHashCount());
} catch (BloomFilterException e) {
} catch (BloomFilter.BloomFilterCreateException e) {
Logger.warn(
LOG_TAG,
"Applying bloom filter failed: ("
Expand Down
Loading