diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LruDelegate.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LruDelegate.java index b42d984ce38..569ef9415ea 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LruDelegate.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LruDelegate.java @@ -14,8 +14,8 @@ package com.google.firebase.firestore.local; +import android.util.SparseArray; import com.google.firebase.firestore.util.Consumer; -import java.util.Set; /** * Persistence layers intending to use LRU Garbage collection should implement this interface. This @@ -37,7 +37,7 @@ interface LruDelegate { * * @return the number of targets removed. */ - int removeTargets(long upperBound, Set activeTargetIds); + int removeTargets(long upperBound, SparseArray activeTargetIds); /** * Removes all unreferenced documents from the cache that have a sequence number less than or diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LruGarbageCollector.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LruGarbageCollector.java index 18c675d38c3..91a61731338 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LruGarbageCollector.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LruGarbageCollector.java @@ -14,10 +14,10 @@ package com.google.firebase.firestore.local; +import android.util.SparseArray; import com.google.firebase.firestore.core.ListenSequence; import java.util.Comparator; import java.util.PriorityQueue; -import java.util.Set; /** Implements the steps for LRU garbage collection. */ class LruGarbageCollector { @@ -80,7 +80,7 @@ long nthSequenceNumber(int count) { * Removes targets with a sequence number equal to or less than the given upper bound, and removes * document associations with those targets. */ - int removeTargets(long upperBound, Set activeTargetIds) { + int removeTargets(long upperBound, SparseArray activeTargetIds) { return delegate.removeTargets(upperBound, activeTargetIds); } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryLruReferenceDelegate.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryLruReferenceDelegate.java index 09bec4032c3..5376b5f9378 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryLruReferenceDelegate.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryLruReferenceDelegate.java @@ -16,13 +16,13 @@ import static com.google.firebase.firestore.util.Assert.hardAssert; +import android.util.SparseArray; import com.google.firebase.firestore.core.ListenSequence; import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.MaybeDocument; import com.google.firebase.firestore.util.Consumer; import java.util.HashMap; import java.util.Map; -import java.util.Set; /** Provides LRU garbage collection functionality for MemoryPersistence. */ class MemoryLruReferenceDelegate implements ReferenceDelegate, LruDelegate { @@ -94,7 +94,7 @@ public void setInMemoryPins(ReferenceSet inMemoryPins) { } @Override - public int removeTargets(long upperBound, Set activeTargetIds) { + public int removeTargets(long upperBound, SparseArray activeTargetIds) { return persistence.getQueryCache().removeQueries(upperBound, activeTargetIds); } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryQueryCache.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryQueryCache.java index 3d428e382d3..e37a451810d 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryQueryCache.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryQueryCache.java @@ -14,6 +14,7 @@ package com.google.firebase.firestore.local; +import android.util.SparseArray; import com.google.firebase.database.collection.ImmutableSortedSet; import com.google.firebase.firestore.core.Query; import com.google.firebase.firestore.model.DocumentKey; @@ -22,7 +23,6 @@ import java.util.HashMap; import java.util.Iterator; import java.util.Map; -import java.util.Set; import javax.annotation.Nullable; /** @@ -115,13 +115,13 @@ public void removeQueryData(QueryData queryData) { * * @return the number of targets removed */ - int removeQueries(long upperBound, Set activeTargetIds) { + int removeQueries(long upperBound, SparseArray activeTargetIds) { int removed = 0; for (Iterator> it = queries.entrySet().iterator(); it.hasNext(); ) { Map.Entry entry = it.next(); int targetId = entry.getValue().getTargetId(); long sequenceNumber = entry.getValue().getSequenceNumber(); - if (sequenceNumber <= upperBound && !activeTargetIds.contains(targetId)) { + if (sequenceNumber <= upperBound && activeTargetIds.get(targetId) == null) { it.remove(); removeMatchingKeysForTargetId(targetId); removed++; diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteLruReferenceDelegate.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteLruReferenceDelegate.java index f27c929588b..cee90c741cc 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteLruReferenceDelegate.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteLruReferenceDelegate.java @@ -16,11 +16,11 @@ import static com.google.firebase.firestore.util.Assert.hardAssert; +import android.util.SparseArray; import com.google.firebase.firestore.core.ListenSequence; import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.ResourcePath; import com.google.firebase.firestore.util.Consumer; -import java.util.Set; /** Provides LRU functionality for SQLite persistence. */ class SQLiteLruReferenceDelegate implements ReferenceDelegate, LruDelegate { @@ -103,7 +103,7 @@ public void removeReference(DocumentKey key) { } @Override - public int removeTargets(long upperBound, Set activeTargetIds) { + public int removeTargets(long upperBound, SparseArray activeTargetIds) { return persistence.getQueryCache().removeQueries(upperBound, activeTargetIds); } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteQueryCache.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteQueryCache.java index e3f43da0bc4..571b302136a 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteQueryCache.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteQueryCache.java @@ -18,6 +18,7 @@ import static com.google.firebase.firestore.util.Assert.hardAssert; import android.database.sqlite.SQLiteStatement; +import android.util.SparseArray; import com.google.firebase.Timestamp; import com.google.firebase.database.collection.ImmutableSortedSet; import com.google.firebase.firestore.core.Query; @@ -26,7 +27,6 @@ import com.google.firebase.firestore.proto.Target; import com.google.firebase.firestore.util.Consumer; import com.google.protobuf.InvalidProtocolBufferException; -import java.util.Set; import javax.annotation.Nullable; /** Cached Queries backed by SQLite. */ @@ -186,7 +186,7 @@ public void removeQueryData(QueryData queryData) { * present in `activeTargetIds`. Document associations for the removed targets are also removed. * Returns the number of targets removed. */ - int removeQueries(long upperBound, Set activeTargetIds) { + int removeQueries(long upperBound, SparseArray activeTargetIds) { int[] count = new int[1]; // SQLite has a max sql statement size, so there is technically a possibility that including a // an IN clause in this query to filter `activeTargetIds` could overflow. Rather than deal with @@ -196,7 +196,7 @@ int removeQueries(long upperBound, Set activeTargetIds) { .forEach( row -> { int targetId = row.getInt(0); - if (!activeTargetIds.contains(targetId)) { + if (activeTargetIds.get(targetId) == null) { removeTarget(targetId); count[0]++; } diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LruGarbageCollectorTestCase.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LruGarbageCollectorTestCase.java index 5247a1e5a4b..11cbc6a3493 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LruGarbageCollectorTestCase.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LruGarbageCollectorTestCase.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import android.util.SparseArray; import com.google.firebase.Timestamp; import com.google.firebase.firestore.auth.User; import com.google.firebase.firestore.core.ListenSequence; @@ -162,7 +163,7 @@ private void removeDocumentFromTarget(DocumentKey key, int targetId) { queryCache.removeMatchingKeys(keySet(key), targetId); } - private int removeTargets(long upperBound, Set activeTargetIds) { + private int removeTargets(long upperBound, SparseArray activeTargetIds) { return persistence.runTransaction( "Remove queries", () -> garbageCollector.removeTargets(upperBound, activeTargetIds)); } @@ -283,7 +284,7 @@ public void testSequenceNumbersWithMutationsInQueries() { @Test public void testRemoveQueriesUpThroughSequenceNumber() { - Map activeTargetIds = new HashMap<>(); + SparseArray activeTargetIds = new SparseArray<>(); for (int i = 0; i < 100; i++) { QueryData queryData = addNextQuery(); // Mark odd queries as live so we can test filtering out live queries. @@ -295,7 +296,7 @@ public void testRemoveQueriesUpThroughSequenceNumber() { // GC up through 20th query, which is 20%. // Expect to have GC'd 10 targets, since every other target is live long upperBound = 20 + initialSequenceNumber; - int removed = removeTargets(upperBound, activeTargetIds.keySet()); + int removed = removeTargets(upperBound, activeTargetIds); assertEquals(10, removed); // Make sure we removed the even targets with targetID <= 20. persistence.runTransaction( @@ -567,8 +568,8 @@ public void testRemoveTargetsThenGC() { }); // Finally, do the garbage collection, up to but not including the removal of middleTarget - Set activeTargetIds = new HashSet<>(); - activeTargetIds.add(oldestTarget.getTargetId()); + SparseArray activeTargetIds = new SparseArray<>(); + activeTargetIds.put(oldestTarget.getTargetId(), oldestTarget); int targetsRemoved = garbageCollector.removeTargets(upperBound, activeTargetIds); // Expect to remove newest target assertEquals(1, targetsRemoved); diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MemoryLruGarbageCollectorTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MemoryLruGarbageCollectorTest.java index 2b9490dcb1e..98074286836 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MemoryLruGarbageCollectorTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MemoryLruGarbageCollectorTest.java @@ -14,6 +14,12 @@ package com.google.firebase.firestore.local; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +@RunWith(RobolectricTestRunner.class) +@Config(manifest = Config.NONE) public class MemoryLruGarbageCollectorTest extends LruGarbageCollectorTestCase { @Override Persistence createPersistence() {