refactor: extract get_server_context helper to eliminate JNI boilerplate#75
Merged
bernardladenthin merged 4 commits intomasterfrom Apr 5, 2026
Merged
Conversation
Move jllama_context struct to a new jni_helpers.hpp header and introduce get_server_context_impl() — a dependency-injectable helper that validates the Java-side model handle and returns the native server_context pointer. A thin get_server_context() wrapper in jllama.cpp binds the module-level globals so all call sites become a clean two-liner. Replaces 19 repetitive 2-5 line blocks (reinterpret_cast + optional null check + ThrowNew) with the helper across all JNIEXPORT functions. As a side-effect, nine functions that previously had no null-check guard now throw "Model is not loaded" instead of crashing on a stale handle. Add src/test/cpp/test_jni_helpers.cpp: four unit tests exercising get_server_context_impl() via a mock JNINativeInterface_ table — no JVM required. Wire the new test file and the JNI include path into the BUILD_TESTING CMake target. https://claude.ai/code/session_01UzZRdmP6koZtJpHgYns6rR
The wrapper referenced f_model_pointer and c_llama_error which are declared later in the anonymous namespace, causing a build error. Moved the wrapper to after all globals, just before parse_jstring(). https://claude.ai/code/session_01UzZRdmP6koZtJpHgYns6rR
AVX-512 is only available on newer Intel CPUs (e.g. Intel Xeon Platinum 8370C). The AMD EPYC 7763 used by GitHub Actions runners does not support it. When a libjllama.so compiled with AVX-512 instructions is loaded on such a machine the JVM crashes immediately with SIGILL. Set GGML_AVX512=OFF unconditionally so the distributed library runs on any x86-64 CPU with at least AVX2 (available since ~2013). https://claude.ai/code/session_01UzZRdmP6koZtJpHgYns6rR
Document four concrete reasons why AVX-512 is disabled for all builds: 1. CPUs without AVX-512 support crash with SIGILL (AMD EPYC 7763 vs Intel Xeon Platinum 8370C is the exact CI scenario that triggered this) 2. Intel disabled AVX-512 on all Alder Lake / Raptor Lake desktop CPUs due to the P-core / E-core scheduler incompatibility 3. Clock throttling on Skylake-X server chips and AMD Ryzen 9000 (Zen 5) makes AVX-512 actively harmful for throughput-sensitive workloads 4. High power draw can destabilize systems near voltage limits https://claude.ai/code/session_01UzZRdmP6koZtJpHgYns6rR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move jllama_context struct to a new jni_helpers.hpp header and introduce get_server_context_impl() — a dependency-injectable helper that validates the Java-side model handle and returns the native server_context pointer. A thin get_server_context() wrapper in jllama.cpp binds the module-level globals so all call sites become a clean two-liner.
Replaces 19 repetitive 2-5 line blocks (reinterpret_cast + optional null check + ThrowNew) with the helper across all JNIEXPORT functions. As a side-effect, nine functions that previously had no null-check guard now throw "Model is not loaded" instead of crashing on a stale handle.
Add src/test/cpp/test_jni_helpers.cpp: four unit tests exercising get_server_context_impl() via a mock JNINativeInterface_ table — no JVM required. Wire the new test file and the JNI include path into the BUILD_TESTING CMake target.
https://claude.ai/code/session_01UzZRdmP6koZtJpHgYns6rR