Skip to content

Commit

Permalink
Make UnsignedBytes.lexicographicalComparator() use `Arrays.compareU…
Browse files Browse the repository at this point in the history
…nsigned` 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: 713020393
  • Loading branch information
cpovirk authored and Google Java Core Libraries committed Jan 10, 2025
1 parent 77cf121 commit abfb31a
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 112 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.common.primitives;

import static com.google.common.base.StandardSystemProperty.JAVA_SPECIFICATION_VERSION;
import static com.google.common.primitives.UnsignedBytes.max;
import static com.google.common.primitives.UnsignedBytes.min;
import static com.google.common.truth.Truth.assertThat;
Expand Down Expand Up @@ -242,12 +243,14 @@ public void testLexicographicalComparatorChoice() throws Exception {
Comparator<byte[]> defaultComparator = UnsignedBytes.lexicographicalComparator();
assertThat(defaultComparator).isNotNull();
assertThat(UnsignedBytes.lexicographicalComparator()).isSameInstanceAs(defaultComparator);
if (unsafeComparatorAvailable()) {
assertThat(Class.forName(unsafeComparatorClassName()))
.isSameInstanceAs(defaultComparator.getClass());
if (!isJava8()) {
assertThat(defaultComparator.getClass())
.isEqualTo(UnsignedBytes.ArraysCompareUnsignedComparator.class);
} else if (unsafeComparatorAvailable()) {
assertThat(defaultComparator.getClass())
.isEqualTo(Class.forName(unsafeComparatorClassName()));
} else {
assertThat(UnsignedBytes.lexicographicalComparatorJavaImpl())
.isSameInstanceAs(defaultComparator);
assertThat(defaultComparator).isEqualTo(UnsignedBytes.lexicographicalComparatorJavaImpl());
}
}

Expand Down Expand Up @@ -360,4 +363,8 @@ public void testSortDescendingIndexed() {
public void testNulls() {
new NullPointerTester().testAllPublicStaticMethods(UnsignedBytes.class);
}

private static boolean isJava8() {
return JAVA_SPECIFICATION_VERSION.value().equals("1.8");
}
}
46 changes: 46 additions & 0 deletions guava/src/com/google/common/primitives/UnsignedBytes.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@
import com.google.common.annotations.J2ktIncompatible;
import com.google.common.annotations.VisibleForTesting;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.j2objc.annotations.J2ObjCIncompatible;
import java.lang.reflect.Field;
import java.nio.ByteOrder;
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.Arrays;
import java.util.Comparator;
import org.jspecify.annotations.Nullable;
import sun.misc.Unsafe;

/**
Expand Down Expand Up @@ -439,6 +441,12 @@ public String toString() {
* to do so.
*/
static Comparator<byte[]> getBestComparator() {
Comparator<byte[]> arraysCompareUnsignedComparator =
ArraysCompareUnsignedComparatorMaker.INSTANCE.tryMakeArraysCompareUnsignedComparator();
if (arraysCompareUnsignedComparator != null) {
return arraysCompareUnsignedComparator;
}

try {
Class<?> theClass = Class.forName(UNSAFE_COMPARATOR_NAME);

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

private enum ArraysCompareUnsignedComparatorMaker {
INSTANCE {
/** Implementation used by non-J2ObjC environments. */
// We use Arrays.compareUnsigned only after confirming that it's available at runtime.
@SuppressWarnings("Java8ApiChecker")
@IgnoreJRERequirement
@Override
@J2ObjCIncompatible
@Nullable Comparator<byte[]> tryMakeArraysCompareUnsignedComparator() {
try {
// Compare AbstractFuture.VarHandleAtomicHelperMaker.
Arrays.class.getMethod("compareUnsigned", byte[].class, byte[].class);
} catch (NoSuchMethodException beforeJava9) {
return null;
}
return ArraysCompareUnsignedComparator.INSTANCE;
}
};

/** Implementation used by J2ObjC environments, overridden for other environments. */
@Nullable Comparator<byte[]> tryMakeArraysCompareUnsignedComparator() {
return null;
}
}

@J2ObjCIncompatible
enum ArraysCompareUnsignedComparator implements Comparator<byte[]> {
INSTANCE;

@Override
// We use the class only after confirming that Arrays.compareUnsigned is available at runtime.
@SuppressWarnings("Java8ApiChecker")
@IgnoreJRERequirement
public int compare(byte[] left, byte[] right) {
return Arrays.compareUnsigned(left, right);
}
}

private static byte flip(byte b) {
return (byte) (b ^ 0x80);
}
Expand Down

0 comments on commit abfb31a

Please sign in to comment.