-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[FEA] Add Java bindings for multi-output and reusable AST JIT execution #23828
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
base: main
Are you sure you want to change the base?
Changes from all commits
aa0963c
e37a829
41a0eb5
6ac19c8
983fc16
d77f4cc
1fb6066
a705ccb
a10b302
afe7db3
c2473d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -110,6 +110,52 @@ TEST_F(TransformProgramTest, ReusesAstWithCompatibleTable) | |||||||||||||||||||||||||||||||||||||||||||||||
| CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected, result->view(), verbosity); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| TEST_F(TransformProgramTest, OwnsScalarColumnViewLiterals) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to also add a test that combines a literal built from |
||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||
| std::unique_ptr<cudf::transform_program> program; | ||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto construction_input = column_wrapper<int32_t>{3, 20, 1, 50}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto construction_table = cudf::table_view{{construction_input}}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto literal_column = column_wrapper<int32_t>{2}; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
igorpeshansky marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||||||||
| auto column_ref = cudf::ast::column_reference{0}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto literal = cudf::ast::literal{cudf::scalar_column_view{literal_column}}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto expression = cudf::ast::operation{cudf::ast::ast_operator::ADD, column_ref, literal}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| std::reference_wrapper<cudf::ast::expression const> expressions[] = {expression}; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| program = std::make_unique<cudf::transform_program>(construction_table, expressions); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+115
to
+126
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Optional] An IIFE lets you do it in one assignment:
Suggested change
Also in |
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| auto input = column_wrapper<int32_t>{10, 20, 30}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto table = cudf::table_view{{input}}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto expected = column_wrapper<int32_t>{12, 22, 32}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto result = std::move(program->run(table)->release().front()); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected, result->view(), verbosity); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| TEST_F(TransformProgramTest, OwnsStringScalarColumnViewLiterals) | ||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||
| std::unique_ptr<cudf::transform_program> program; | ||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto construction_input = cudf::test::strings_column_wrapper{"a", "ccc"}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto construction_table = cudf::table_view{{construction_input}}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto literal_column = cudf::test::strings_column_wrapper{"ccc"}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto column_ref = cudf::ast::column_reference{0}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto literal = cudf::ast::literal{cudf::scalar_column_view{literal_column}}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto expression = cudf::ast::operation{cudf::ast::ast_operator::LESS, column_ref, literal}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| std::reference_wrapper<cudf::ast::expression const> expressions[] = {expression}; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| program = std::make_unique<cudf::transform_program>(construction_table, expressions); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| auto input = cudf::test::strings_column_wrapper{"a", "ccc", "dddd"}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto table = cudf::table_view{{input}}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto expected = column_wrapper<bool>{true, false, false}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto result = std::move(program->run(table)->release().front()); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected, result->view(), verbosity); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| TEST_F(TransformProgramTest, RejectsIncompatibleTable) | ||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||
| auto construction_input = column_wrapper<int32_t>{3, 20, 1, 50}; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,14 +41,39 @@ void serialize(ByteBuffer bb) { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Compile this expression for execution with the process-level backend selection. | ||
| * | ||
| * @return expression compatible with default AST consumers | ||
|
igorpeshansky marked this conversation as resolved.
|
||
| * @throws IllegalArgumentException if a root literal requires JIT compilation | ||
| * @throws ai.rapids.cudf.CudfException if compilation fails | ||
| */ | ||
| public CompiledExpression compile() { | ||
| return compile(CompiledExpression.CompilationMode.DEFAULT); | ||
| } | ||
|
|
||
| /** | ||
| * Compile this expression for explicit execution with the libcudf JIT backend. | ||
| * The returned expression cannot be used as a join or scan predicate. | ||
| * | ||
| * @return expression specialized for JIT execution | ||
| * @throws ai.rapids.cudf.CudfException if compilation fails | ||
| */ | ||
| public CompiledExpression compileJit() { | ||
| return compile(CompiledExpression.CompilationMode.JIT); | ||
| } | ||
|
|
||
| private CompiledExpression compile(CompiledExpression.CompilationMode mode) { | ||
| validateCompilationMode(mode); | ||
| int size = getSerializedSize(); | ||
| ByteBuffer bb = ByteBuffer.allocate(size); | ||
| bb.order(ByteOrder.nativeOrder()); | ||
| serialize(bb); | ||
| return new CompiledExpression(bb.array()); | ||
| return new CompiledExpression(bb.array(), mode); | ||
| } | ||
|
|
||
| void validateCompilationMode(CompiledExpression.CompilationMode mode) {} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (a) This needs a Javadoc. |
||
|
|
||
| /** Get the size in bytes of the serialized form of this node and all child nodes */ | ||
| abstract int getSerializedSize(); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package ai.rapids.cudf.ast; | ||
|
|
||
| import ai.rapids.cudf.MemoryCleaner; | ||
| import ai.rapids.cudf.NativeDepsLoader; | ||
| import ai.rapids.cudf.Table; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import java.util.Objects; | ||
|
|
||
| /** | ||
| * A reusable AST JIT program specialized to an input schema. | ||
| * Construction lowers the expressions and retrieves their JIT kernel. Subsequent calls reuse that | ||
| * kernel with tables whose referenced columns have compatible types and nullability. | ||
| * Callers must ensure that {@link #close()} does not overlap with {@link #computeTable(Table)}. | ||
| */ | ||
| public final class AstJitProgram implements AutoCloseable { | ||
| static { | ||
| NativeDepsLoader.loadNativeDeps(); | ||
| } | ||
|
|
||
| private static final Logger log = LoggerFactory.getLogger(AstJitProgram.class); | ||
|
|
||
| private static final class AstJitProgramCleaner extends MemoryCleaner.Cleaner { | ||
| private long nativeHandle; | ||
|
|
||
| AstJitProgramCleaner(long nativeHandle) { | ||
| this.nativeHandle = nativeHandle; | ||
| } | ||
|
|
||
| @Override | ||
| protected synchronized boolean cleanImpl(boolean logErrorIfNotClean) { | ||
| boolean alreadyClean = nativeHandle == 0; | ||
| if (alreadyClean) { | ||
| return false; | ||
| } | ||
| long origAddress = nativeHandle; | ||
| try { | ||
| destroy(nativeHandle); | ||
| } finally { | ||
| nativeHandle = 0; | ||
| } | ||
| if (logErrorIfNotClean) { | ||
| log.error("AN AST JIT PROGRAM WAS LEAKED (ID: {} {})", id, | ||
| Long.toHexString(origAddress)); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isClean() { | ||
| return nativeHandle == 0; | ||
| } | ||
| } | ||
|
|
||
| private final AstJitProgramCleaner cleaner; | ||
| private boolean isClosed = false; | ||
|
|
||
| private AstJitProgram(long nativeHandle) { | ||
| AstJitProgramCleaner newCleaner = null; | ||
| try { | ||
| newCleaner = new AstJitProgramCleaner(nativeHandle); | ||
| cleaner = newCleaner; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably a good idea to assign it as the last line of the AstJitProgramCleaner newCleaner = null;
try {
newCleaner = new AstJitProgramCleaner(nativeHandle);
MemoryCleaner.register(this, newCleaner);
newCleaner.addRef();
cleaner = newCleaner;
} catch (Throwable t) {Also in the [Really optional] Sigh, now we have two classes with an identical complex constructor pattern. It's probably way beyond the scope of this PR (and in scope for #23939), but we could fold a lot of this in by reusing Java's try-with-resources, e.g., get suppressed exception behavior for free: static abstract class CleanerManager<T extends MemoryCleaner.Cleaner> implements AutoCloseable {
private T cleaner = null;
private boolean released = false;
public final T acquire(T c) { cleaner = c; return c; }
public final T release() { released = true; return cleaner; }
@Override
public final void close() {
if (released) return;
if (cleaner != null) { cleaner.clean(false); }
else { fallbackClean(); }
}
protected abstract void fallbackClean();
}
private AstJitProgram(long nativeHandle) {
try (CleanerManager<AstJitProgramCleaner> manager =
new CleanerManager<AstJitProgramCleaner>() {
@Override
protected void fallbackClean() { destroy(nativeHandle); }
}) {
AstJitProgramCleaner newCleaner = manager.acquire(new AstJitProgramCleaner(nativeHandle));
MemoryCleaner.register(this, newCleaner);
newCleaner.addRef();
cleaner = manager.release();
}
} |
||
| MemoryCleaner.register(this, cleaner); | ||
| cleaner.addRef(); | ||
| } catch (Throwable t) { | ||
| try { | ||
| if (newCleaner == null) { | ||
| destroy(nativeHandle); | ||
| } else { | ||
| newCleaner.clean(false); | ||
| } | ||
| } catch (Throwable cleanupFailure) { | ||
| t.addSuppressed(cleanupFailure); | ||
| } | ||
| throw t; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Compile a reusable program from one or more JIT-compiled expressions. | ||
| * The schema table and expressions are inspected during construction but are not retained. The | ||
| * returned program owns any literal values required by later evaluations. | ||
| * | ||
| * @param schemaTable table whose referenced column schema is used to compile the program | ||
| * @param expressions non-empty JIT-compiled expressions in output order | ||
| * @return a reusable AST JIT program | ||
| * @throws NullPointerException if the table, expression array, or an expression is null | ||
| * @throws IllegalArgumentException if no expressions are provided or an expression was not | ||
| * produced by {@link AstExpression#compileJit()} | ||
| * @throws IllegalStateException if the table or an expression is closed | ||
| * @throws ai.rapids.cudf.CudfException if JIT compilation fails | ||
| */ | ||
| public static AstJitProgram compile(Table schemaTable, CompiledExpression... expressions) { | ||
| long tableHandle = Objects.requireNonNull(schemaTable, "schemaTable").getNativeView(); | ||
| CompiledExpression.JitExpressionArgs expressionArgs = | ||
| CompiledExpression.getJitExpressionArgs(expressions); | ||
|
Comment on lines
+101
to
+102
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to do the closed table check before processing all of the expressions? Otherwise it's a behavior change. Also in |
||
| if (tableHandle == 0) { | ||
| throw new IllegalStateException("Table is closed"); | ||
| } | ||
|
|
||
| try { | ||
| return new AstJitProgram(create(expressionArgs.nativeHandles, tableHandle)); | ||
| } finally { | ||
| CompiledExpression.reachabilityFence(schemaTable); | ||
| CompiledExpression.reachabilityFence(expressionArgs.expressionRefs); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Evaluate this program on a table with a compatible referenced-column schema. | ||
| * The row count and unreferenced columns may differ from the schema table used at compilation. | ||
| * Calling {@link #close()} while an evaluation is in progress is unsupported. | ||
| * | ||
| * @param table input table for expression evaluation | ||
| * @return table containing the program outputs in expression order | ||
| * @throws NullPointerException if the table is null | ||
| * @throws IllegalStateException if the program or table is closed | ||
| * @throws ai.rapids.cudf.CudfException if the referenced-column schema is incompatible or | ||
| * evaluation fails | ||
| */ | ||
| public Table computeTable(Table table) { | ||
| long tableHandle = Objects.requireNonNull(table, "table").getNativeView(); | ||
| long programHandle = cleaner.nativeHandle; | ||
| if (programHandle == 0) { | ||
| throw new IllegalStateException("AST JIT program is closed"); | ||
| } | ||
| if (tableHandle == 0) { | ||
| throw new IllegalStateException("Table is closed"); | ||
| } | ||
|
|
||
| try { | ||
| return new Table(computeTableNative(programHandle, tableHandle)); | ||
| } finally { | ||
| CompiledExpression.reachabilityFence(this); | ||
| CompiledExpression.reachabilityFence(table); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized void close() { | ||
| cleaner.delRef(); | ||
| if (isClosed) { | ||
| cleaner.logRefCountDebug("double free " + this); | ||
| throw new IllegalStateException("Close called too many times " + this); | ||
| } | ||
| cleaner.clean(false); | ||
| isClosed = true; | ||
|
Comment on lines
+147
to
+153
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are a bunch of these identical
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, let's keep this comment open for tracking.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we're keeping this open, I'll also add that since |
||
| } | ||
|
|
||
| private static native long create(long[] astHandles, long tableHandle); | ||
| private static native long[] computeTableNative(long programHandle, long tableHandle); | ||
| private static native void destroy(long handle); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raised by Claude verbatim, so take it with a grain of salt.
Dropping
impl_->ast_scalar_columns_ = std::move(args.scalar_columns)leavestransform_args::scalar_columnsorphaned: it's still populated atcpp/src/jit/row_ir.cpp:1030-1034, but no production code reads it anymore — the only remaining reader iscpp/tests/jit/row_ir.cpp:359-360. A populated-but-never-read ownership vector is a trap for the next reader, who will reasonably assume the program still owns those columns.It also costs an extra allocation per
cudf::scalarliteral: the converter materializes a one-row column, we copy it here, and the original is freed whenargsgoes out of scope.mainmoved it instead.Would it make sense to normalize ownership in the converter instead — have the loop at
cpp/src/jit/row_ir.cpp:1030-1034also push a copy for the non-owningscalar_column_viewalternative? Then this constructor keeps its singlestd::move,scalar_columnsstops being dead, and the "one entry pernulloptininput_column_indices" invariant lives in one place rather than being re-derived here. That means touchingrow_ir.*, which is outside the current diff, so could be done in a follow-up.If you'd rather keep the copy here, can we at least assert the correspondence — e.g.
CUDF_EXPECTS(scalar_index == impl_->ast_scalar_columns_.size(), …)after the loop inrun()? The alignment currently rests on several unstated facts inrow_ir.cpp. In particular,add_input(column_view const&)produces acolumn_inputwith anulloptcolumn_index, which would desynchronize the two. Nothing incpp/srccalls it today — the only callers are the directinstance_contexttests incpp/tests/jit/row_ir.cpp, which never go throughast_converter::compute_table— but nothing stops a future caller either.