Skip to content

Commit

Permalink
[SPARK-21967][CORE] org.apache.spark.unsafe.types.UTF8String#compareT…
Browse files Browse the repository at this point in the history
…o Should Compare 8 Bytes at a Time for Better Performance

## What changes were proposed in this pull request?

* Using 64 bit unsigned long comparison instead of unsigned int comparison in `org.apache.spark.unsafe.types.UTF8String#compareTo` for better performance.
* Making `IS_LITTLE_ENDIAN` a constant for correctness reasons (shouldn't use a non-constant in `compareTo` implementations and it def. is a constant per JVM)

## How was this patch tested?

Build passes and the functionality is widely covered by existing tests as far as I can see.

Author: Armin <me@obrown.io>

Closes #19180 from original-brownbear/SPARK-21967.
  • Loading branch information
original-brownbear authored and srowen committed Sep 16, 2017
1 parent 0bad10d commit 73d9067
Showing 1 changed file with 19 additions and 5 deletions.
Expand Up @@ -64,7 +64,8 @@ public final class UTF8String implements Comparable<UTF8String>, Externalizable,
5, 5, 5, 5,
6, 6};

private static boolean isLittleEndian = ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN;
private static final boolean IS_LITTLE_ENDIAN =
ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN;

private static final UTF8String COMMA_UTF8 = UTF8String.fromString(",");
public static final UTF8String EMPTY_UTF8 = UTF8String.fromString("");
Expand Down Expand Up @@ -220,7 +221,7 @@ public long getPrefix() {
// After getting the data, we use a mask to mask out data that is not part of the string.
long p;
long mask = 0;
if (isLittleEndian) {
if (IS_LITTLE_ENDIAN) {
if (numBytes >= 8) {
p = Platform.getLong(base, offset);
} else if (numBytes > 4) {
Expand Down Expand Up @@ -1097,10 +1098,23 @@ public UTF8String copy() {
@Override
public int compareTo(@Nonnull final UTF8String other) {
int len = Math.min(numBytes, other.numBytes);
// TODO: compare 8 bytes as unsigned long
for (int i = 0; i < len; i ++) {
int wordMax = (len / 8) * 8;
long roffset = other.offset;
Object rbase = other.base;
for (int i = 0; i < wordMax; i += 8) {
long left = getLong(base, offset + i);
long right = getLong(rbase, roffset + i);
if (left != right) {
if (IS_LITTLE_ENDIAN) {
return Long.compareUnsigned(Long.reverseBytes(left), Long.reverseBytes(right));
} else {
return Long.compareUnsigned(left, right);
}
}
}
for (int i = wordMax; i < len; i++) {
// In UTF-8, the byte should be unsigned, so we should compare them as unsigned int.
int res = (getByte(i) & 0xFF) - (other.getByte(i) & 0xFF);
int res = (getByte(i) & 0xFF) - (Platform.getByte(rbase, roffset + i) & 0xFF);
if (res != 0) {
return res;
}
Expand Down

0 comments on commit 73d9067

Please sign in to comment.