Skip to content

[GLUTEN-12535][CORE] Support backend-specific JNI input adapters - #12535

Merged
taiyang-li merged 5 commits into
apache:mainfrom
wangxinshuo-bolt:pr/core-jni-backend-adapters
Jul 24, 2026
Merged

[GLUTEN-12535][CORE] Support backend-specific JNI input adapters#12535
taiyang-li merged 5 commits into
apache:mainfrom
wangxinshuo-bolt:pr/core-jni-backend-adapters

Conversation

@wangxinshuo-bolt

@wangxinshuo-bolt wangxinshuo-bolt commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

What changes are proposed in this pull request?

This PR provides backend-extensible JNI input adapters in Core as part of #12456.

The implementation:

  • Adds a JniInputIteratorFactory registry keyed by runtime kind.
  • Provides a common entry point for creating backend-specific JNI input iterators.
  • Uses JniColumnarBatchIterator as the default when no backend-specific factory is registered.
  • Provides a reusable ShuffleStreamReader and Java input-stream adapter in JniCommon.
  • Manages the related Java class global references and method IDs through JniCommonState.
  • Rejects empty factories and duplicate registrations.

Backends can register their own JNI input iterator factory while continuing to use the common plan-evaluation and shuffle-stream interfaces.

How was this patch tested?

  • Ran C++ formatting checks.
  • Built and linked libgluten.so.

Was this patch authored or co-authored using generative AI tooling?

Yes.

Generated-by: Codex GPT-5

Related issue: #12535

@github-actions github-actions Bot added the VELOX label Jul 16, 2026
Comment thread cpp/core/tests/JniInputIteratorTest.cc Outdated
Comment thread cpp/core/jni/JniWrapper.cc Outdated
Comment thread cpp/core/jni/JniWrapper.cc Outdated
@wangxinshuo-bolt
wangxinshuo-bolt force-pushed the pr/core-jni-backend-adapters branch from bee9bdd to c436dce Compare July 16, 2026 09:30
@wangxinshuo-bolt
wangxinshuo-bolt marked this pull request as ready for review July 16, 2026 12:43
Comment thread cpp/core/jni/JniWrapper.cc Outdated
@wangxinshuo-bolt

Copy link
Copy Markdown
Contributor Author

@jackylee-ch Can you review this PR?

@jackylee-ch

Copy link
Copy Markdown
Contributor

cc @zhztheplayer @zhouyuan

@zhouyuan
zhouyuan requested a review from Copilot July 22, 2026 10:28

This comment was marked as off-topic.

@taiyang-li taiyang-li left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@zhouyuan

Copy link
Copy Markdown
Member

hi @wangxinshuo-bolt is this also part of refactor work in #12456?
Cc: @marin-ma

Comment thread cpp/core/jni/JniCommon.cc Outdated
Comment on lines +70 to +74
std::unique_ptr<gluten::ColumnarBatchIterator> gluten::Runtime::createJniInputIterator(
const JniInputIteratorContext& context) {
return std::make_unique<JniColumnarBatchIterator>(
context.env, context.jColumnarBatchIterator, this, context.iteratorIndex);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be defined in Runtime.cc?

Comment thread cpp/core/compute/Runtime.h Outdated
return dumper_.get();
}

virtual std::unique_ptr<ColumnarBatchIterator> createJniInputIterator(const JniInputIteratorContext& context);

@zhztheplayer zhztheplayer Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

JniInputIteratorContext::env is of type JNIEnv* which requires jni.h, while the Runtime API is currently isolated from the JNI layer. Does it look like a design issue in the PR?

Comment thread cpp/core/jni/JniWrapper.cc Outdated

namespace gluten {

std::shared_ptr<StreamReader> makeShuffleStreamReader(JNIEnv* env, jobject jShuffleStreamReader) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't need to keep ShuffleStreamReader private. Would it make sense to move ShuffleStreamReader into JniCommon.h?

@wangxinshuo-bolt

Copy link
Copy Markdown
Contributor Author

hi @wangxinshuo-bolt is this also part of refactor work in #12456? Cc: @marin-ma

Yes

Comment thread cpp/core/jni/JniWrapper.cc Outdated
}
std::shared_ptr<arrow::io::InputStream> in = std::make_shared<JavaInputStreamAdaptor>(env, pool, jniIn);
return in;
ShuffleStreamReader::ShuffleStreamReader(JNIEnv* env, jobject reader) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The implementation should be placed in JniCommon.cc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The implementation should be placed in JniCommon.cc

ok. I will fix it.

@wangxinshuo-bolt wangxinshuo-bolt changed the title [CORE] Add reusable JNI input adapters for backend runtimes [CORE] Decouple JNI input adapters from Runtime Jul 23, 2026
@wangxinshuo-bolt wangxinshuo-bolt changed the title [CORE] Decouple JNI input adapters from Runtime [GLUTEN-12456][CORE] Support backend-specific JNI input adapters Jul 23, 2026
@wangxinshuo-bolt wangxinshuo-bolt changed the title [GLUTEN-12456][CORE] Support backend-specific JNI input adapters [GLUTEN-12535][CORE] Support backend-specific JNI input adapters Jul 23, 2026
@taiyang-li
taiyang-li merged commit 26f04d8 into apache:main Jul 24, 2026
99 of 101 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants