Markduckworth/cache query speedups

firebase-firestore/CHANGELOG.md

@@ -3,6 +3,8 @@
 - [changed] Bumped internal dependencies.
 - [changed] Improve the performance of queries in collections that contain many deleted documents.
   [#7295](//github.com/firebase/firebase-android-sdk/issues/7295)
+- [changed] Improve the performance of queries against SDK cache through internal memoization of
+  document data.
 
 # 26.0.0
 

firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCache.java

@@ -32,8 +32,6 @@
 import com.google.firebase.firestore.model.MutableDocument;
 import com.google.firebase.firestore.model.ResourcePath;
 import com.google.firebase.firestore.model.SnapshotVersion;
-import com.google.firebase.firestore.util.BackgroundQueue;
-import com.google.firebase.firestore.util.Executors;
 import com.google.firebase.firestore.util.Function;
 import com.google.protobuf.InvalidProtocolBufferException;
 import com.google.protobuf.MessageLite;
@@ -47,7 +45,6 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.Executor;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
@@ -170,13 +167,9 @@ public Map<DocumentKey, MutableDocument> getAll(Iterable<DocumentKey> documentKe
             bindVars,
             ") ORDER BY path");
 
-    BackgroundQueue backgroundQueue = new BackgroundQueue();
     while (longQuery.hasMoreSubqueries()) {
-      longQuery
-          .performNextSubquery()
-          .forEach(row -> processRowInBackground(backgroundQueue, results, row, /*filter*/ null));
+      longQuery.performNextSubquery().forEach(row -> processRow(results, row, /*filter*/ null));
     }
-    backgroundQueue.drain();
 
     // Backfill any rows with null "document_type" discovered by processRowInBackground().
     documentTypeBackfiller.backfill(db);
@@ -266,18 +259,16 @@ private Map<DocumentKey, MutableDocument> getAll(
     }
     bindVars[i] = count;
 
-    BackgroundQueue backgroundQueue = new BackgroundQueue();
     Map<DocumentKey, MutableDocument> results = new HashMap<>();
     db.query(sql.toString())
         .binding(bindVars)
         .forEach(
             row -> {
-              processRowInBackground(backgroundQueue, results, row, filter);
+              processRow(results, row, filter);
               if (context != null) {
                 context.incrementDocumentReadCount();
               }
             });
-    backgroundQueue.drain();
 
     // Backfill any null "document_type" columns discovered by processRowInBackground().
     documentTypeBackfiller.backfill(db);
@@ -297,8 +288,7 @@ private Map<DocumentKey, MutableDocument> getAll(
         collections, offset, count, /*tryFilterDocumentType*/ null, filter, /*context*/ null);
   }
 
-  private void processRowInBackground(
-      BackgroundQueue backgroundQueue,
+  private void processRow(
       Map<DocumentKey, MutableDocument> results,
       Cursor row,
       @Nullable Function<MutableDocument, Boolean> filter) {
@@ -308,22 +298,15 @@ private void processRowInBackground(
     boolean documentTypeIsNull = row.isNull(3);
     String path = row.getString(4);
 
-    // Since scheduling background tasks incurs overhead, we only dispatch to a
-    // background thread if there are still some documents remaining.
-    Executor executor = row.isLast() ? Executors.DIRECT_EXECUTOR : backgroundQueue;
-    executor.execute(
-        () -> {
-          MutableDocument document =
-              decodeMaybeDocument(rawDocument, readTimeSeconds, readTimeNanos);
-          if (documentTypeIsNull) {
-            documentTypeBackfiller.enqueue(path, readTimeSeconds, readTimeNanos, document);
-          }
-          if (filter == null || filter.apply(document)) {
-            synchronized (results) {
-              results.put(document.getKey(), document);
-            }
-          }
-        });
+    MutableDocument document = decodeMaybeDocument(rawDocument, readTimeSeconds, readTimeNanos);
+    if (documentTypeIsNull) {
+      documentTypeBackfiller.enqueue(path, readTimeSeconds, readTimeNanos, document);
+    }
+    if (filter == null || filter.apply(document)) {
+      synchronized (results) {
+        results.put(document.getKey(), document);
+      }
+    }
   }
 
   @Override

firebase-firestore/src/main/java/com/google/firebase/firestore/model/BasePath.java

@@ -18,6 +18,7 @@
 
 import androidx.annotation.NonNull;
 import com.google.firebase.firestore.util.Util;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -27,6 +28,7 @@
  */
 public abstract class BasePath<B extends BasePath<B>> implements Comparable<B> {
   final List<String> segments;
+  List<byte[]> utf8Segments = null;
 
   BasePath(List<String> segments) {
     this.segments = segments;
@@ -93,8 +95,12 @@ public int compareTo(@NonNull B o) {
     int i = 0;
     int myLength = length();
     int theirLength = o.length();
+    List<byte[]> myUtf8Segments = ensureUtf8Segments();
+    List<byte[]> theirUtf8Segments = o.ensureUtf8Segments();
     while (i < myLength && i < theirLength) {
-      int localCompare = compareSegments(getSegment(i), o.getSegment(i));
+      int localCompare =
+          compareSegments(
+              myUtf8Segments.get(i), theirUtf8Segments.get(i), getSegment(i), o.getSegment(i));
       if (localCompare != 0) {
         return localCompare;
       }
@@ -103,21 +109,39 @@ public int compareTo(@NonNull B o) {
     return Integer.compare(myLength, theirLength);
   }
 
-  private static int compareSegments(String lhs, String rhs) {
-    boolean isLhsNumeric = isNumericId(lhs);
-    boolean isRhsNumeric = isNumericId(rhs);
+  private static int compareSegments(byte[] lhs, byte[] rhs, String lhsString, String rhsString) {
+    boolean isLhsNumeric = lhs == null;
+    boolean isRhsNumeric = rhs == null;
 
     if (isLhsNumeric && !isRhsNumeric) { // Only lhs is numeric
       return -1;
     } else if (!isLhsNumeric && isRhsNumeric) { // Only rhs is numeric
       return 1;
     } else if (isLhsNumeric && isRhsNumeric) { // both numeric
-      return Long.compare(extractNumericId(lhs), extractNumericId(rhs));
+      return Long.compare(extractNumericId(lhsString), extractNumericId(rhsString));
     } else { // both string
-      return Util.compareUtf8Strings(lhs, rhs);
+      return Util.compareByteArrays(lhs, rhs);
     }
   }
 
+  public List<byte[]> ensureUtf8Segments() {
+    if (this.utf8Segments == null) {
+      this.utf8Segments = new ArrayList<>(this.segments.size());
+      for (int i = 0; i < this.segments.size(); i++) {
+        String segment = this.segments.get(i);
+        boolean isNumeric = isNumericId(segment);
+        if (!isNumeric) {
+          byte[] utf8Bytes = segment.getBytes(StandardCharsets.UTF_8);
+          this.utf8Segments.add(utf8Bytes);
+        } else {
+          this.utf8Segments.add(null);
+        }
+      }
+    }
+
+    return this.utf8Segments;
+  }
+
   /** Checks if a segment is a numeric ID (starts with "__id" and ends with "__"). */
   private static boolean isNumericId(String segment) {
     return segment.startsWith("__id") && segment.endsWith("__");

firebase-firestore/src/main/java/com/google/firebase/firestore/model/ObjectValue.java

@@ -28,6 +28,8 @@
 
 /** A structured object value stored in Firestore. */
 public final class ObjectValue implements Cloneable {
+  private final Object lock = new Object(); // Monitor object
+
   /**
    * The immutable Value proto for this object. Local mutations are stored in `overlayMap` and only
    * applied when {@link #buildProto()} is invoked.
@@ -124,10 +126,12 @@ private Value extractNestedValue(Value value, FieldPath fieldPath) {
    * invocations are based on this memoized result.
    */
   private Value buildProto() {
-    synchronized (overlayMap) {
-      MapValue mergedResult = applyOverlay(FieldPath.EMPTY_PATH, overlayMap);
-      if (mergedResult != null) {
-        partialValue = Value.newBuilder().setMapValue(mergedResult).build();
+    synchronized (lock) {
+      if (!overlayMap.isEmpty()) {
+        MapValue mergedResult = applyOverlayLocked(FieldPath.EMPTY_PATH, overlayMap);
+        if (mergedResult != null) {
+          partialValue = Value.newBuilder().setMapValue(mergedResult).build();
+        }
         overlayMap.clear();
       }
     }
@@ -171,31 +175,33 @@ public void setAll(Map<FieldPath, Value> data) {
    * Adds {@code value} to the overlay map at {@code path}. Creates nested map entries if needed.
    */
   private void setOverlay(FieldPath path, @Nullable Value value) {
-    Map<String, Object> currentLevel = overlayMap;
+    synchronized (lock) {
+      Map<String, Object> currentLevel = overlayMap;
 
-    for (int i = 0; i < path.length() - 1; ++i) {
-      String currentSegment = path.getSegment(i);
-      Object currentValue = currentLevel.get(currentSegment);
+      for (int i = 0; i < path.length() - 1; ++i) {
+        String currentSegment = path.getSegment(i);
+        Object currentValue = currentLevel.get(currentSegment);
 
-      if (currentValue instanceof Map) {
-        // Re-use a previously created map
-        currentLevel = (Map<String, Object>) currentValue;
-      } else if (currentValue instanceof Value
-          && ((Value) currentValue).getValueTypeCase() == Value.ValueTypeCase.MAP_VALUE) {
-        // Convert the existing Protobuf MapValue into a Java map
-        Map<String, Object> nextLevel =
-            new HashMap<>(((Value) currentValue).getMapValue().getFieldsMap());
-        currentLevel.put(currentSegment, nextLevel);
-        currentLevel = nextLevel;
-      } else {
-        // Create an empty hash map to represent the current nesting level
-        Map<String, Object> nextLevel = new HashMap<>();
-        currentLevel.put(currentSegment, nextLevel);
-        currentLevel = nextLevel;
+        if (currentValue instanceof Map) {
+          // Re-use a previously created map
+          currentLevel = (Map<String, Object>) currentValue;
+        } else if (currentValue instanceof Value
+            && ((Value) currentValue).getValueTypeCase() == Value.ValueTypeCase.MAP_VALUE) {
+          // Convert the existing Protobuf MapValue into a Java map
+          Map<String, Object> nextLevel =
+              new HashMap<>(((Value) currentValue).getMapValue().getFieldsMap());
+          currentLevel.put(currentSegment, nextLevel);
+          currentLevel = nextLevel;
+        } else {
+          // Create an empty hash map to represent the current nesting level
+          Map<String, Object> nextLevel = new HashMap<>();
+          currentLevel.put(currentSegment, nextLevel);
+          currentLevel = nextLevel;
+        }
       }
-    }
 
-    currentLevel.put(path.getLastSegment(), value);
+      currentLevel.put(path.getLastSegment(), value);
+    }
   }
 
   /**
@@ -208,7 +214,7 @@ private void setOverlay(FieldPath path, @Nullable Value value) {
    *     overlayMap}.
    * @return The merged data at `currentPath` or null if no modifications were applied.
    */
-  private @Nullable MapValue applyOverlay(
+  private @Nullable MapValue applyOverlayLocked(
       FieldPath currentPath, Map<String, Object> currentOverlays) {
     boolean modified = false;
 
@@ -227,7 +233,7 @@ private void setOverlay(FieldPath path, @Nullable Value value) {
       if (value instanceof Map) {
         @Nullable
         MapValue nested =
-            applyOverlay(currentPath.append(pathSegment), (Map<String, Object>) value);
+            applyOverlayLocked(currentPath.append(pathSegment), (Map<String, Object>) value);
         if (nested != null) {
           resultAtPath.putFields(pathSegment, Value.newBuilder().setMapValue(nested).build());
           modified = true;