Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-5909: [Java] Optimize ByteFunctionHelpers equals & compare logic #4852

Closed
wants to merge 4 commits into from
Closed
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.arrow.vector.util;

import java.util.concurrent.TimeUnit;

import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.RootAllocator;
import org.junit.Test;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.TearDown;
import org.openjdk.jmh.runner.Runner;
import org.openjdk.jmh.runner.RunnerException;
import org.openjdk.jmh.runner.options.Options;
import org.openjdk.jmh.runner.options.OptionsBuilder;

import io.netty.buffer.ArrowBuf;

/**
* Benchmarks for {@link ByteFunctionHelpers}.
*/
@State(Scope.Benchmark)
public class ByteFunctionHelpersBenchmarks {

private static final int BUFFER_CAPACITY = 7;

private static final int ALLOCATOR_CAPACITY = 1024 * 1024;

private BufferAllocator allocator;

private ArrowBuf buffer1;

private ArrowBuf buffer2;

/**
* Setup benchmarks.
*/
@Setup
public void prepare() {
allocator = new RootAllocator(ALLOCATOR_CAPACITY);
buffer1 = allocator.buffer(BUFFER_CAPACITY);
buffer2 = allocator.buffer(BUFFER_CAPACITY);

for (int i = 0; i < BUFFER_CAPACITY; i++) {
buffer1.setByte(i, i);
buffer2.setByte(i, i);
}
}

/**
* Tear down benchmarks.
*/
@TearDown
public void tearDown() {
buffer1.close();
buffer2.close();
allocator.close();
}

@Benchmark
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public void equals() {
ByteFunctionHelpers.equal(buffer1, 0, BUFFER_CAPACITY - 1, buffer2, 0, BUFFER_CAPACITY - 1);

}

@Test
public void evaluate() throws RunnerException {
Options opt = new OptionsBuilder()
.include(ByteFunctionHelpersBenchmarks.class.getSimpleName())
.forks(1)
.build();

new Runner(opt).run();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ private ByteFunctionHelpers() {}
* @param right Right ArrowBuf for comparison
* @param rStart start offset in the buffer
* @param rEnd end offset in the buffer
* @return 1 if left input is greater, -1 if left input is smaller, 0 otherwise
* @return 1 if equals, 0 otherwise
*/
public static final int equal(final ArrowBuf left, int lStart, int lEnd, final ArrowBuf right, int rStart, int rEnd) {
if (BoundsChecking.BOUNDS_CHECKING_ENABLED) {
Expand Down Expand Up @@ -67,6 +67,18 @@ private static int memEqual(final long laddr, int lStart, int lEnd, final long r
rPos += 8;
n -= 8;
}

while (n > 3) {
int leftInt = PlatformDependent.getInt(lPos);
int rightInt = PlatformDependent.getInt(rPos);
if (leftInt != rightInt) {
return 0;
}
lPos += 4;
rPos += 4;
n -= 4;
}

while (n-- != 0) {
byte leftByte = PlatformDependent.getByte(lPos);
byte rightByte = PlatformDependent.getByte(rPos);
Expand Down Expand Up @@ -126,13 +138,24 @@ private static int memcmp(
long leftLong = PlatformDependent.getLong(lPos);
long rightLong = PlatformDependent.getLong(rPos);
if (leftLong != rightLong) {
return unsignedLongCompare(Long.reverseBytes(leftLong), Long.reverseBytes(rightLong));
return unsignedLongCompare(leftLong, rightLong);
}
lPos += 8;
rPos += 8;
n -= 8;
}

while (n > 3) {
int leftInt = PlatformDependent.getInt(lPos);
int rightInt = PlatformDependent.getInt(rPos);
if (leftInt != rightInt) {
return unsignedIntCompare(leftInt, rightInt);
}
lPos += 4;
rPos += 4;
n -= 4;
}

while (n-- != 0) {
byte leftByte = PlatformDependent.getByte(lPos);
byte rightByte = PlatformDependent.getByte(rPos);
Expand Down Expand Up @@ -189,6 +212,9 @@ public static int unsignedLongCompare(long a, long b) {
return Long.compare(a ^ Long.MIN_VALUE, b ^ Long.MIN_VALUE);
}

public static int unsignedIntCompare(int a, int b) {
return Integer.compare(a ^ Integer.MIN_VALUE, b ^ Integer.MIN_VALUE);
}

private static int memcmp(
final long laddr,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.arrow.vector.util;

import static org.junit.Assert.assertEquals;

import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.RootAllocator;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

import io.netty.buffer.ArrowBuf;

public class TestByteFunctionHelpers {

private BufferAllocator allocator;

private static final int SIZE = 100;

@Before
public void init() {
allocator = new RootAllocator(Long.MAX_VALUE);

}

@After
public void terminate() throws Exception {
allocator.close();
}

@Test
public void testEquals() {
ArrowBuf buffer1 = allocator.buffer(SIZE);
ArrowBuf buffer2 = allocator.buffer(SIZE);

for (int i = 0; i < SIZE; i++) {
buffer1.setByte(i, i);
buffer2.setByte(i, i);
}

//test three cases, length>8, length>3, length<3

assertEquals(1, ByteFunctionHelpers.equal(buffer1, 0, SIZE - 1,
buffer2, 0, SIZE - 1));
assertEquals(1, ByteFunctionHelpers.equal(buffer1, 0, 6,
buffer2, 0, 6));
assertEquals(1, ByteFunctionHelpers.equal(buffer1, 0, 2,
buffer2, 0, 2));

//change value at index1
buffer1.setByte(1, 10);

assertEquals(0, ByteFunctionHelpers.equal(buffer1, 0, SIZE - 1,
buffer2, 0, SIZE - 1));
assertEquals(0, ByteFunctionHelpers.equal(buffer1, 0, 6,
buffer2, 0, 6));
assertEquals(0, ByteFunctionHelpers.equal(buffer1, 0, 2,
buffer2, 0, 2));

buffer1.close();
buffer2.close();

}

@Test
public void testCompare() {
ArrowBuf buffer1 = allocator.buffer(SIZE);
ArrowBuf buffer2 = allocator.buffer(SIZE);

for (int i = 0; i < SIZE; i++) {
buffer1.setByte(i, i);
buffer2.setByte(i, i);
}

//test three cases, length>8, length>3, length<3

assertEquals(0, ByteFunctionHelpers.compare(buffer1, 0, SIZE - 1,
buffer2, 0, SIZE - 1));
assertEquals(0, ByteFunctionHelpers.compare(buffer1, 0, 6,
buffer2, 0, 6));
assertEquals(0, ByteFunctionHelpers.compare(buffer1, 0, 2,
buffer2, 0, 2));

//change value at index 1
buffer1.setByte(1, 0);

assertEquals(-1, ByteFunctionHelpers.compare(buffer1, 0, SIZE - 1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add another test case for the + case.

In an ideal world we would explicitly test the three cases for different lengths >8, >3 and <=3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, fixed.

buffer2, 0, SIZE - 1));
assertEquals(-1, ByteFunctionHelpers.compare(buffer1, 0, 6,
buffer2, 0, 6));
assertEquals(-1, ByteFunctionHelpers.compare(buffer1, 0, 2,
buffer2, 0, 2));

buffer1.close();
buffer2.close();

}
}