Skip to content
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
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 the `compare()` methods defined in standard `Integer`, `Long`, and `Character`
classes instead of Firestore's bespoke implementations.
[#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