Skip to content

Commit b3bb29a

Browse files
cpovirkGoogle Java Core Libraries
authored and
Google Java Core Libraries
committed
Make UnsignedBytes.lexicographicalComparator() use Arrays.compareUnsigned when it's available.
This provides another [alternative to using `Unsafe`](#6806). And port the benchmark to JMH, which for now means making it Google-internal. (Not that we have our _Caliper_ benchmarks actually _running_ externally, either, IIRC.) The benchmarks suggest that the old, `Unsafe`-based implementation is faster up to 64 elements or so but that it's a matter of only about a nanosecond. The new implementation pulls ahead for larger arrays, including an advantage of about 5-10 ns for 1024 elements. For now, I've included this implementation only in the JRE flavor of Guava. We could include it in the Android flavor, too, to see if it helps [under API Level 33+](https://developer.android.com/reference/java/util/Arrays#compareUnsigned(byte[],%20byte[])). But we really would want to do yet more benchmarking for that. RELNOTES=n/a PiperOrigin-RevId: 714130759
1 parent 77cf121 commit b3bb29a

File tree

3 files changed

+58
-112
lines changed

3 files changed

+58
-112
lines changed

guava-tests/benchmark/com/google/common/primitives/UnsignedBytesBenchmark.java

-107
This file was deleted.

guava-tests/test/com/google/common/primitives/UnsignedBytesTest.java

+12-5
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.google.common.primitives;
1818

19+
import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION;
1920
import static com.google.common.primitives.UnsignedBytes.max;
2021
import static com.google.common.primitives.UnsignedBytes.min;
2122
import static com.google.common.truth.Truth.assertThat;
@@ -242,12 +243,14 @@ public void testLexicographicalComparatorChoice() throws Exception {
242243
Comparator<byte[]> defaultComparator = UnsignedBytes.lexicographicalComparator();
243244
assertThat(defaultComparator).isNotNull();
244245
assertThat(UnsignedBytes.lexicographicalComparator()).isSameInstanceAs(defaultComparator);
245-
if (unsafeComparatorAvailable()) {
246-
assertThat(Class.forName(unsafeComparatorClassName()))
247-
.isSameInstanceAs(defaultComparator.getClass());
246+
if (!isJava8()) {
247+
assertThat(defaultComparator.getClass())
248+
.isEqualTo(UnsignedBytes.ArraysCompareUnsignedComparator.class);
249+
} else if (unsafeComparatorAvailable()) {
250+
assertThat(defaultComparator.getClass())
251+
.isEqualTo(Class.forName(unsafeComparatorClassName()));
248252
} else {
249-
assertThat(UnsignedBytes.lexicographicalComparatorJavaImpl())
250-
.isSameInstanceAs(defaultComparator);
253+
assertThat(defaultComparator).isEqualTo(UnsignedBytes.lexicographicalComparatorJavaImpl());
251254
}
252255
}
253256

@@ -360,4 +363,8 @@ public void testSortDescendingIndexed() {
360363
public void testNulls() {
361364
new NullPointerTester().testAllPublicStaticMethods(UnsignedBytes.class);
362365
}
366+
367+
private static boolean isJava8() {
368+
return JAVA_SPECIFICATION_VERSION.value().equals("1.8");
369+
}
363370
}

guava/src/com/google/common/primitives/UnsignedBytes.java

+46
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,15 @@
2323
import com.google.common.annotations.J2ktIncompatible;
2424
import com.google.common.annotations.VisibleForTesting;
2525
import com.google.errorprone.annotations.CanIgnoreReturnValue;
26+
import com.google.j2objc.annotations.J2ObjCIncompatible;
2627
import java.lang.reflect.Field;
2728
import java.nio.ByteOrder;
2829
import java.security.AccessController;
2930
import java.security.PrivilegedActionException;
3031
import java.security.PrivilegedExceptionAction;
3132
import java.util.Arrays;
3233
import java.util.Comparator;
34+
import org.jspecify.annotations.Nullable;
3335
import sun.misc.Unsafe;
3436

3537
/**
@@ -439,6 +441,12 @@ public String toString() {
439441
* to do so.
440442
*/
441443
static Comparator<byte[]> getBestComparator() {
444+
Comparator<byte[]> arraysCompareUnsignedComparator =
445+
ArraysCompareUnsignedComparatorMaker.INSTANCE.tryMakeArraysCompareUnsignedComparator();
446+
if (arraysCompareUnsignedComparator != null) {
447+
return arraysCompareUnsignedComparator;
448+
}
449+
442450
try {
443451
Class<?> theClass = Class.forName(UNSAFE_COMPARATOR_NAME);
444452

@@ -455,6 +463,44 @@ static Comparator<byte[]> getBestComparator() {
455463
}
456464
}
457465

466+
private enum ArraysCompareUnsignedComparatorMaker {
467+
INSTANCE {
468+
/** Implementation used by non-J2ObjC environments. */
469+
// We use Arrays.compareUnsigned only after confirming that it's available at runtime.
470+
@SuppressWarnings("Java8ApiChecker")
471+
@IgnoreJRERequirement
472+
@Override
473+
@J2ObjCIncompatible
474+
@Nullable Comparator<byte[]> tryMakeArraysCompareUnsignedComparator() {
475+
try {
476+
// Compare AbstractFuture.VarHandleAtomicHelperMaker.
477+
Arrays.class.getMethod("compareUnsigned", byte[].class, byte[].class);
478+
} catch (NoSuchMethodException beforeJava9) {
479+
return null;
480+
}
481+
return ArraysCompareUnsignedComparator.INSTANCE;
482+
}
483+
};
484+
485+
/** Implementation used by J2ObjC environments, overridden for other environments. */
486+
@Nullable Comparator<byte[]> tryMakeArraysCompareUnsignedComparator() {
487+
return null;
488+
}
489+
}
490+
491+
@J2ObjCIncompatible
492+
enum ArraysCompareUnsignedComparator implements Comparator<byte[]> {
493+
INSTANCE;
494+
495+
@Override
496+
// We use the class only after confirming that Arrays.compareUnsigned is available at runtime.
497+
@SuppressWarnings("Java8ApiChecker")
498+
@IgnoreJRERequirement
499+
public int compare(byte[] left, byte[] right) {
500+
return Arrays.compareUnsigned(left, right);
501+
}
502+
}
503+
458504
private static byte flip(byte b) {
459505
return (byte) (b ^ 0x80);
460506
}

0 commit comments

Comments
 (0)