From 7cf2059ba4de9b0aebf11e5d0042f115f8fbea7c Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 18 Oct 2018 11:23:01 -0700 Subject: [PATCH 1/2] Use orphaned documents in sequence number calculation --- .../com/google/firebase/firestore/local/LruDelegate.java | 2 +- .../firebase/firestore/local/LruGarbageCollector.java | 6 ++---- .../firestore/local/MemoryLruReferenceDelegate.java | 9 +++++++-- .../firestore/local/SQLiteLruReferenceDelegate.java | 8 ++++++-- 4 files changed, 16 insertions(+), 9 deletions(-) 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 62db1e6fc69..2dd128c285e 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 @@ -26,7 +26,7 @@ public interface LruDelegate { /** Enumerates all the targets in the QueryCache. */ void forEachTarget(Consumer consumer); - long getTargetCount(); + long getSequenceNumberCount(); /** Enumerates sequence numbers for documents not associated with a target. */ void forEachOrphanedDocumentSequenceNumber(Consumer consumer); 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 722fc3df75a..b327bce0f11 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 @@ -109,7 +109,7 @@ public int getDocumentsRemoved() { /** Given a percentile of target to collect, returns the number of targets to collect. */ int calculateQueryCount(int percentile) { - long targetCount = delegate.getTargetCount(); + long targetCount = delegate.getSequenceNumberCount(); return (int) ((percentile / 100.0f) * targetCount); } @@ -216,15 +216,13 @@ private Results runGarbageCollection(SparseArray liveTargetIds) { int numDocumentsRemoved = removeOrphanedDocuments(upperBound); long removedDocumentsTs = System.currentTimeMillis(); - // TODO(gsoltis): post-compaction? - if (Logger.isDebugEnabled()) { String desc = "LRU Garbage Collection:\n"; desc += "\tCounted targets in " + (countedTargetsTs - startTs) + "ms\n"; desc += String.format( Locale.ROOT, - "\tDetermined least recently used %d sequence numbers in %dms", + "\tDetermined least recently used %d sequence numbers in %dms\n", sequenceNumbers, (foundUpperBoundTs - countedTargetsTs)); desc += 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 56fc1df0d9d..5312c5d4b68 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 @@ -82,8 +82,13 @@ public void forEachTarget(Consumer consumer) { } @Override - public long getTargetCount() { - return persistence.getQueryCache().getTargetCount(); + public long getSequenceNumberCount() { + long targetCount = persistence.getQueryCache().getTargetCount(); + long orphanedCount[] = new long[1]; + forEachOrphanedDocumentSequenceNumber(sequenceNumber -> { + orphanedCount[0]++; + }); + return targetCount + orphanedCount[0]; } @Override 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 d846c08636a..679fe46d982 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 @@ -70,8 +70,12 @@ public LruGarbageCollector getGarbageCollector() { } @Override - public long getTargetCount() { - return persistence.getQueryCache().getTargetCount(); + public long getSequenceNumberCount() { + long targetCount = persistence.getQueryCache().getTargetCount(); + long orphanedDocumentCount = persistence.query( + "SELECT COUNT(*) FROM (SELECT sequence_number FROM target_documents GROUP BY path HAVING COUNT(*) = 1 AND target_id = 0)" + ).firstValue(row -> row.getLong(0)); + return targetCount + orphanedDocumentCount; } @Override From 6edc3f6408eb34e006017610bfdcd557d0c81332 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Thu, 18 Oct 2018 12:12:39 -0700 Subject: [PATCH 2/2] Formatting --- .../firebase/firestore/FirebaseFirestoreSettings.java | 8 ++++---- .../firestore/local/MemoryLruReferenceDelegate.java | 7 ++++--- .../firestore/local/SQLiteLruReferenceDelegate.java | 8 +++++--- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestoreSettings.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestoreSettings.java index afadfca0560..486e59f0511 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestoreSettings.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestoreSettings.java @@ -25,8 +25,8 @@ @PublicApi public final class FirebaseFirestoreSettings { /** - * Constant to use with {@link - * FirebaseFirestoreSettings.Builder#setCacheSizeBytes(long)} to disable garbage collection. + * Constant to use with {@link FirebaseFirestoreSettings.Builder#setCacheSizeBytes(long)} to + * disable garbage collection. */ @PublicApi public static final long CACHE_SIZE_UNLIMITED = -1; @@ -140,8 +140,8 @@ public Builder setTimestampsInSnapshotsEnabled(boolean value) { * guarantee that the cache will stay below that size, only that if the cache exceeds the given * size, cleanup will be attempted. * - *

The default value is 100 MB. The threshold must be set to at least 1 MB, and can be set - * to {@link FirebaseFirestoreSettings#CACHE_SIZE_UNLIMITED} to disable garbage collection. + *

The default value is 100 MB. The threshold must be set to at least 1 MB, and can be set to + * {@link FirebaseFirestoreSettings#CACHE_SIZE_UNLIMITED} to disable garbage collection. * * @return A settings object on which the cache size is configured as specified by the given * {@code value}. 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 5312c5d4b68..6ff4cb74dcc 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 @@ -85,9 +85,10 @@ public void forEachTarget(Consumer consumer) { public long getSequenceNumberCount() { long targetCount = persistence.getQueryCache().getTargetCount(); long orphanedCount[] = new long[1]; - forEachOrphanedDocumentSequenceNumber(sequenceNumber -> { - orphanedCount[0]++; - }); + forEachOrphanedDocumentSequenceNumber( + sequenceNumber -> { + orphanedCount[0]++; + }); return targetCount + orphanedCount[0]; } 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 679fe46d982..d2823ac3990 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 @@ -72,9 +72,11 @@ public LruGarbageCollector getGarbageCollector() { @Override public long getSequenceNumberCount() { long targetCount = persistence.getQueryCache().getTargetCount(); - long orphanedDocumentCount = persistence.query( - "SELECT COUNT(*) FROM (SELECT sequence_number FROM target_documents GROUP BY path HAVING COUNT(*) = 1 AND target_id = 0)" - ).firstValue(row -> row.getLong(0)); + long orphanedDocumentCount = + persistence + .query( + "SELECT COUNT(*) FROM (SELECT sequence_number FROM target_documents GROUP BY path HAVING COUNT(*) = 1 AND target_id = 0)") + .firstValue(row -> row.getLong(0)); return targetCount + orphanedDocumentCount; }