Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
3 changes: 3 additions & 0 deletions firebase-firestore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
* [fixed] Further improved performance of UTF-8 string ordering logic,
which had degraded in v25.1.2 and received some improvements in v25.1.3.
[#7053](//github.com/firebase/firebase-android-sdk/issues/7053)
* [changed] Use `Integer.compare()` and `Long.compare()` (which were added in Android API 19)
instead of Firestore's bespoke implementations, since minSdkVersion is greater than 19.
[#7109](//github.com/firebase/firebase-android-sdk/pull/7109)


# 25.1.4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static int firestoreCompareDoubleWithLong(double doubleValue, long longVa
}

long doubleAsLong = (long) doubleValue;
int cmp = compareLongs(doubleAsLong, longValue);
int cmp = Long.compare(doubleAsLong, longValue);
if (cmp != 0) {
return cmp;
}
Expand All @@ -60,11 +60,6 @@ public static int firestoreCompareDoubleWithLong(double doubleValue, long longVa
return firestoreCompareDoubles(doubleValue, longAsDouble);
}

/** Compares longs. */
public static int compareLongs(long leftLong, long rightLong) {
return Long.compare(leftLong, rightLong);
}

/**
* Compares doubles with Firestore query semantics: NaN precedes all other numbers and equals
* itself, all zeroes are equal.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static com.google.firebase.firestore.core.Query.LimitType.LIMIT_TO_FIRST;
import static com.google.firebase.firestore.core.Query.LimitType.LIMIT_TO_LAST;
import static com.google.firebase.firestore.util.Assert.hardAssert;
import static com.google.firebase.firestore.util.Util.compareIntegers;

import androidx.annotation.Nullable;
import com.google.firebase.database.collection.ImmutableSortedMap;
Expand Down Expand Up @@ -301,7 +300,7 @@ public ViewChange applyChanges(
Collections.sort(
viewChanges,
(DocumentViewChange o1, DocumentViewChange o2) -> {
int typeComp = compareIntegers(View.changeTypeOrder(o1), View.changeTypeOrder(o2));
int typeComp = Integer.compare(View.changeTypeOrder(o1), View.changeTypeOrder(o2));
if (typeComp != 0) {
return typeComp;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

package com.google.firebase.firestore.local;

import static com.google.firebase.firestore.util.Util.compareIntegers;

import com.google.firebase.firestore.model.DocumentKey;
import java.util.Comparator;

Expand Down Expand Up @@ -60,12 +58,12 @@ int getId() {
return keyComp;
}

return compareIntegers(o1.targetOrBatchId, o2.targetOrBatchId);
return Integer.compare(o1.targetOrBatchId, o2.targetOrBatchId);
};

static final Comparator<DocumentReference> BY_TARGET =
(o1, o2) -> {
int targetComp = compareIntegers(o1.targetOrBatchId, o2.targetOrBatchId);
int targetComp = Integer.compare(o1.targetOrBatchId, o2.targetOrBatchId);

if (targetComp != 0) {
return targetComp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import com.google.firebase.firestore.model.mutation.MutationBatch;
import com.google.firebase.firestore.remote.WriteStream;
import com.google.firebase.firestore.util.Consumer;
import com.google.firebase.firestore.util.Util;
import com.google.protobuf.ByteString;
import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.MessageLite;
Expand Down Expand Up @@ -324,7 +323,7 @@ public List<MutationBatch> getAllMutationBatchesAffectingDocumentKeys(
Collections.sort(
result,
(MutationBatch lhs, MutationBatch rhs) ->
Util.compareIntegers(lhs.getBatchId(), rhs.getBatchId()));
Integer.compare(lhs.getBatchId(), rhs.getBatchId()));
}
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public int compareTo(@NonNull B o) {
}
i++;
}
return Util.compareIntegers(myLength, theirLength);
return Integer.compare(myLength, theirLength);
}

private static int compareSegments(String lhs, String rhs) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ public static int compare(Value left, Value right) {
int rightType = typeOrder(right);

if (leftType != rightType) {
return Util.compareIntegers(leftType, rightType);
return Integer.compare(leftType, rightType);
}

switch (leftType) {
Expand Down Expand Up @@ -291,7 +291,7 @@ private static int compareNumbers(Value left, Value right) {
} else if (left.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) {
long leftLong = left.getIntegerValue();
if (right.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE) {
return Util.compareLongs(leftLong, right.getIntegerValue());
return Long.compare(leftLong, right.getIntegerValue());
} else if (right.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) {
return -1 * Util.compareMixed(right.getDoubleValue(), leftLong);
}
Expand All @@ -301,11 +301,11 @@ private static int compareNumbers(Value left, Value right) {
}

private static int compareTimestamps(Timestamp left, Timestamp right) {
int cmp = Util.compareLongs(left.getSeconds(), right.getSeconds());
int cmp = Long.compare(left.getSeconds(), right.getSeconds());
if (cmp != 0) {
return cmp;
}
return Util.compareIntegers(left.getNanos(), right.getNanos());
return Integer.compare(left.getNanos(), right.getNanos());
}

private static int compareReferences(String leftPath, String rightPath) {
Expand All @@ -319,7 +319,7 @@ private static int compareReferences(String leftPath, String rightPath) {
return cmp;
}
}
return Util.compareIntegers(leftSegments.length, rightSegments.length);
return Integer.compare(leftSegments.length, rightSegments.length);
}

private static int compareGeoPoints(LatLng left, LatLng right) {
Expand All @@ -338,7 +338,7 @@ private static int compareArrays(ArrayValue left, ArrayValue right) {
return cmp;
}
}
return Util.compareIntegers(left.getValuesCount(), right.getValuesCount());
return Integer.compare(left.getValuesCount(), right.getValuesCount());
}

private static int compareMaps(MapValue left, MapValue right) {
Expand Down Expand Up @@ -372,7 +372,7 @@ private static int compareVectors(MapValue left, MapValue right) {
ArrayValue rightArrayValue = rightMap.get(Values.VECTOR_MAP_VECTORS_KEY).getArrayValue();

int lengthCompare =
Util.compareIntegers(leftArrayValue.getValuesCount(), rightArrayValue.getValuesCount());
Integer.compare(leftArrayValue.getValuesCount(), rightArrayValue.getValuesCount());
if (lengthCompare != 0) {
return lengthCompare;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,6 @@ public static int compareBooleans(boolean b1, boolean b2) {
}
}

/**
* Utility function to compare integers. Note that we can't use Integer.compare because it's only
* available after Android 19.
*/
public static int compareIntegers(int i1, int i2) {
if (i1 < i2) {
return -1;
} else if (i1 > i2) {
return 1;
} else {
return 0;
}
}

/** Compare strings in UTF-8 encoded byte order */
public static int compareUtf8Strings(String left, String right) {
// noinspection StringEquality
Expand Down Expand Up @@ -119,7 +105,7 @@ public static int compareUtf8Strings(String left, String right) {
final char rightChar = right.charAt(i);
if (leftChar != rightChar) {
return (isSurrogate(leftChar) == isSurrogate(rightChar))
? Util.compareIntegers(leftChar, rightChar)
? Character.compare(leftChar, rightChar)
: isSurrogate(leftChar) ? 1 : -1;
}
}
Expand All @@ -129,14 +115,6 @@ public static int compareUtf8Strings(String left, String right) {
return Integer.compare(left.length(), right.length());
}

/**
* Utility function to compare longs. Note that we can't use Long.compare because it's only
* available after Android 19.
*/
public static int compareLongs(long i1, long i2) {
return NumberComparisonHelper.compareLongs(i1, i2);
}

/** Utility function to compare doubles (using Firestore semantics for NaN). */
public static int compareDoubles(double i1, double i2) {
return NumberComparisonHelper.firestoreCompareDoubles(i1, i2);
Expand Down Expand Up @@ -274,7 +252,7 @@ public static int compareByteArrays(byte[] left, byte[] right) {
}
// Byte values are equal, continue with comparison
}
return Util.compareIntegers(left.length, right.length);
return Integer.compare(left.length, right.length);
}

public static int compareByteStrings(ByteString left, ByteString right) {
Expand All @@ -290,7 +268,7 @@ public static int compareByteStrings(ByteString left, ByteString right) {
}
// Byte values are equal, continue with comparison
}
return Util.compareIntegers(left.size(), right.size());
return Integer.compare(left.size(), right.size());
}

public static StringBuilder repeatSequence(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void compareUtf8StringsShouldReturnCorrectValue() {
ByteString b2 = ByteString.copyFromUtf8(s2);
int expected = Util.compareByteStrings(b1, b2);

if (actual == expected) {
if (Integer.signum(actual) == Integer.signum(expected)) {
passCount++;
} else {
errors.add(
Expand All @@ -117,9 +117,12 @@ public void compareUtf8StringsShouldReturnCorrectValue() {
+ "\", s2=\""
+ s2
+ "\") returned "
+ signName(actual)
+ " ("
+ actual
+ ")"
+ ", but expected "
+ expected
+ signName(expected)
+ " (i="
+ i
+ ", s1.length="
Expand All @@ -142,6 +145,16 @@ public void compareUtf8StringsShouldReturnCorrectValue() {
}
}

private static String signName(int value) {
if (value < 0) {
return "a negative value";
} else if (value > 0) {
return "a positive value";
} else {
return "0";
}
}

private static class StringPairGenerator {

private final StringGenerator stringGenerator;
Expand Down
Loading