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

Add JNI methods for detecting and purging non-empty nulls from LIST and STRUCT #12742

Merged
merged 19 commits into from
Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
37 changes: 37 additions & 0 deletions java/src/main/java/ai/rapids/cudf/ColumnView.java
Original file line number Diff line number Diff line change
Expand Up @@ -4639,6 +4639,10 @@ static native long makeCudfColumnView(int type, int scale, long data, long dataS

static native long applyBooleanMask(long arrayColumnView, long booleanMaskHandle) throws CudfException;

static native boolean hasNonEmptyNulls(long handle) throws CudfException;

static native long purgeNonEmptyNulls(long handle) throws CudfException;

/**
* A utility class to create column vector like objects without refcounts and other APIs when
* creating the device side vector from host side nested vectors. Eventually this can go away or
Expand Down Expand Up @@ -4997,4 +5001,37 @@ public HostColumnVector copyToHost() {
}
}
}

/**
* Exact check if a column or its descendants have non-empty null rows
*
* @return Whether the column or its descendants have non-empty null rows
*/
public boolean hasNonEmptyNulls() {
return hasNonEmptyNulls(viewHandle);
}

/**
* Copies this column into output while purging any non-empty null rows in the column or its
* descendants.
*
* If this column is not of compound type (LIST/STRING/STRUCT/DICTIONARY), the output will be
* the same as input.
*
* The purge operation only applies directly to LIST and STRING columns, but it applies indirectly
* to STRUCT/DICTIONARY columns as well, since these columns may have child columns that
* are LIST or STRING.
*
* Examples:
* lists = data: [{{0,1}, {2,3}, {4,5}} validity: {true, false, true}]
* lists[1] is null, but the list's child column still stores `{2,3}`.
*
* After purging the contents of the list's null rows, the column's contents will be:
* lists = [data: {{0,1}, {4,5}} validity: {true, false, true}]
*
* @return A new column with equivalent contents to `input`, but with null rows purged
*/
public ColumnVector purgeNonEmptyNulls() {
return new ColumnVector(purgeNonEmptyNulls(viewHandle));
}
}
22 changes: 22 additions & 0 deletions java/src/main/native/src/ColumnViewJni.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2457,4 +2457,26 @@ JNIEXPORT jlong JNICALL Java_ai_rapids_cudf_ColumnView_applyBooleanMask(
CATCH_STD(env, 0);
}

JNIEXPORT jboolean JNICALL
Java_ai_rapids_cudf_ColumnView_hasNonEmptyNulls(JNIEnv *env, jclass, jlong column_view_handle) {
JNI_NULL_CHECK(env, column_view_handle, "column_view handle is null", 0);
try {
cudf::jni::auto_set_device(env);
auto const *cv = reinterpret_cast<cudf::column_view const *>(column_view_handle);
return cudf::has_nonempty_nulls(*cv);
}
CATCH_STD(env, 0);
}

JNIEXPORT jlong JNICALL
Java_ai_rapids_cudf_ColumnView_purgeNonEmptyNulls(JNIEnv *env, jclass, jlong column_view_handle) {
JNI_NULL_CHECK(env, column_view_handle, "column_view handle is null", 0);
try {
cudf::jni::auto_set_device(env);
auto const *cv = reinterpret_cast<cudf::column_view const *>(column_view_handle);
return release_as_jlong(cudf::purge_nonempty_nulls(*cv));
}
CATCH_STD(env, 0);
}

} // extern "C"
53 changes: 53 additions & 0 deletions java/src/test/java/ai/rapids/cudf/ColumnVectorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
Expand Down Expand Up @@ -6691,4 +6692,56 @@ void testApplyBooleanMaskFromListOfStructure() {
assertColumnsAreEqual(expectedCv, actualCv);
}
}

private ColumnView getColumnViewWithNonEmptyNulls() {
List<Integer> list0 = Arrays.asList(1, 2, 3);
List<Integer> list1 = Arrays.asList(4, 5, null);
List<Integer> list2 = Arrays.asList(7, 8, 9);
List<Integer> list3 = null;
ColumnVector input = makeListsColumn(DType.INT32, list0, list1, list2, list3);
Copy link
Contributor

Choose a reason for hiding this comment

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

CI failed because there is memory leak in Java tests. Sorry I overlooked the Java tests. Please fix them.

Copy link
Contributor

@ttnghia ttnghia Feb 24, 2023

Choose a reason for hiding this comment

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

ColumnVector as well as HostColumnVector must be wrapped in try blocks. Probably the buffer vars need to be wrapped too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm... makes me want to have this failure on my local so we can avoid this. It will still be good to get to the bottom of this discrepancy.

In the meanwhile, I will push a fix. Thanks for pointing us to this.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, the discrepancy isn't ideal.

Could it be caused by the types of GPUs that are used in CI vs. on our local machines?

I'm wondering if that or the amount of memory on a given GPU would contribute to this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe there is a variable that controls the test behavior, and that var is set by the local environment. @revans2 Do you know about this?

Copy link
Contributor

Choose a reason for hiding this comment

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

If a memory leak happens and java's GC is fast, then the leak is cleared up before RMM is shut down and there is no error. If GC is slow, then the leak is not cleared and you get the error. We have a flag to print out messages to help debug leaks, but it is off by default because it makes the tests take a lot longer. Just set -Dai.rapids.refcount.debug=true on the maven commend line when running the tests. Even if they pass you can then look at the logs to see if any leaks were detected and use the stack traces to track down what is leaked and why.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But in this case aren't the vectors off the heap? Did you mean the MemoryCleaner instead of GC?


// Modify the validity buffer
BaseDeviceMemoryBuffer dmb = input.getDeviceBufferFor(BufferType.VALIDITY);
HostMemoryBuffer newValidity = HostMemoryBuffer.allocate(64);
newValidity.copyFromDeviceBuffer(dmb);
BitVectorHelper.setNullAt(newValidity, 1);
dmb.copyFromHostBuffer(newValidity);

HostColumnVector hostColumnVector = input.copyToHost();
assert(hostColumnVector.isNull(1));
assert(hostColumnVector.isNull(3));

ColumnVector expectedOffsetsBeforePurge = ColumnVector.fromInts(0, 3, 6, 9, 9);
ColumnView offsetsCvBeforePurge = input.getListOffsetsView();
assertColumnsAreEqual(expectedOffsetsBeforePurge, offsetsCvBeforePurge);
ColumnView colWithNonEmptyNulls = new ColumnView(input.type, input.rows, Optional.of(2L), dmb,
input.getDeviceBufferFor(BufferType.OFFSET), input.getChildColumnViews());
assertEquals(2, colWithNonEmptyNulls.nullCount);
return colWithNonEmptyNulls;
}

@Test
void testPurgeNonEmptyNullsList() {
ColumnView colWithNonEmptyNulls = getColumnViewWithNonEmptyNulls();
// purge non-empty nulls
assertTrue(colWithNonEmptyNulls.hasNonEmptyNulls());
ColumnView colWithEmptyNulls = colWithNonEmptyNulls.purgeNonEmptyNulls();
ColumnVector expectedOffsetsAfterPurge = ColumnVector.fromInts(0, 3, 3, 6, 6);
ColumnView offsetsCvAfterPurge = colWithEmptyNulls.getListOffsetsView();
assertColumnsAreEqual(expectedOffsetsAfterPurge, offsetsCvAfterPurge);
assertFalse(colWithEmptyNulls.hasNonEmptyNulls());
}

@Test
void testPurgeNonEmptyNullsStruct() {
ColumnView listCol = getColumnViewWithNonEmptyNulls();
ColumnView stringsCol = ColumnVector.fromStrings("A", "col", "of", "Strings");
ColumnView structView = ColumnView.makeStructView(stringsCol, listCol);
ColumnView structWithEmptyNulls = structView.purgeNonEmptyNulls();
ColumnView newListChild = structWithEmptyNulls.getChildColumnView(1);
ColumnVector expectedOffsetsAfterPurge = ColumnVector.fromInts(0, 3, 3, 6, 6);
ColumnView offsetsCvAfterPurge = newListChild.getListOffsetsView();
assertColumnsAreEqual(expectedOffsetsAfterPurge, offsetsCvAfterPurge);
assertFalse(newListChild.hasNonEmptyNulls());
}
}