Refactor JNI lifecycle and log formatting into reusable helpers (#176)

* Extract pure log_helpers.hpp with unit tests

Pulls log_level_name() and a deterministic format_log_as_json(level, text, ts)
out of jllama.cpp into a new pure header with zero JNI/llama dependencies.
Adds 13 GoogleTest cases covering level mapping, JSON shape, null text, and
explicit timestamp handling.

https://claude.ai/code/session_01TBELeBieKqvMwp1Ez8HgSk

* Extract erase_reader() helper for readers-map cleanup

Replaces four identical lock_guard + readers.erase() pairs in jllama.cpp
(releaseTask, receiveCompletionJson x2, cancelCompletion) with a single
inline helper in jni_helpers.hpp. Adds 3 unit tests covering present,
missing, and selective-id removal.

https://claude.ai/code/session_01TBELeBieKqvMwp1Ez8HgSk

* Extract require_embedding_support() guard helper

Replaces two identical 'if (!jctx->params.embedding) ThrowNew(...) return'
blocks in embed() and handleEmbeddings() with a single helper in
jni_helpers.hpp. Adds 2 unit tests using the existing mock JNI fixture.

https://claude.ai/code/session_01TBELeBieKqvMwp1Ez8HgSk

* Table-driven JNI global ref lifecycle in JNI_OnLoad/OnUnload

Replaces 14 hand-written NewGlobalRef + 7 GetStaticObjectField + 7
NewGlobalRef + 21 DeleteGlobalRef calls with iteration over three small
tables: g_global_class_refs, g_global_object_refs, g_static_object_bindings.
Adding a new global ref is now a single-line table edit instead of
touching four separate places.

https://claude.ai/code/session_01TBELeBieKqvMwp1Ez8HgSk

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: bernardladenthin

CMakeLists.txt

@@ -297,6 +297,7 @@ if(BUILD_TESTING)
         src/test/cpp/test_server.cpp
         src/test/cpp/test_jni_helpers.cpp
         src/test/cpp/test_json_helpers.cpp
+        src/test/cpp/test_log_helpers.cpp
         ${llama.cpp_SOURCE_DIR}/tools/server/server-common.cpp
         ${llama.cpp_SOURCE_DIR}/tools/server/server-chat.cpp
         ${llama.cpp_SOURCE_DIR}/tools/server/server-context.cpp

src/main/cpp/jllama.cpp

@@ -18,6 +18,7 @@
 #include "server-chat.h"
 #include "utils.hpp"
 #include "jni_helpers.hpp"
+#include "log_helpers.hpp"
 
 #include <atomic>
 #include <chrono>
@@ -102,6 +103,42 @@ jobject o_log_format_json = nullptr;
 jobject o_log_format_text = nullptr;
 jobject o_log_callback = nullptr;
 
+// ---------------------------------------------------------------------------
+// JNI global-ref lifecycle tables
+//
+// Every entry here is promoted to a JNI global ref in JNI_OnLoad and released
+// in JNI_OnUnload. Keep these in sync with the declarations above; ordering
+// within each table only matters for the human reader.
+// ---------------------------------------------------------------------------
+static jclass *const g_global_class_refs[] = {
+    &c_llama_model, &c_string,   &c_hash_map, &c_map,       &c_set,
+    &c_entry,       &c_iterator, &c_integer,  &c_float,     &c_biconsumer,
+    &c_llama_error, &c_log_level, &c_log_format, &c_error_oom,
+};
+
+static jobject *const g_global_object_refs[] = {
+    &o_utf_8,
+    &o_log_level_debug, &o_log_level_info, &o_log_level_warn, &o_log_level_error,
+    &o_log_format_json, &o_log_format_text,
+};
+
+// Maps every object that is fetched from a Java static field on load to the
+// (class, field) pair it should be looked up from.
+struct static_object_binding {
+    jobject  *target;
+    jclass   *cls;
+    jfieldID *field;
+};
+
+static const static_object_binding g_static_object_bindings[] = {
+    {&o_log_level_debug, &c_log_level,  &f_log_level_debug},
+    {&o_log_level_info,  &c_log_level,  &f_log_level_info},
+    {&o_log_level_warn,  &c_log_level,  &f_log_level_warn},
+    {&o_log_level_error, &c_log_level,  &f_log_level_error},
+    {&o_log_format_json, &c_log_format, &f_log_format_json},
+    {&o_log_format_text, &c_log_format, &f_log_format_text},
+};
+
 /**
  * Returns the jllama_context wrapper for the Java LlamaModel object.
  * Used by the delete path and any method that needs jctx directly.
@@ -390,38 +427,14 @@ JNIEnv *get_jni_env() {
 bool log_json;
 std::function<void(ggml_log_level, const char *, void *)> log_callback;
 
-/**
- * Format a log message as JSON.
- */
-std::string format_log_as_json(ggml_log_level level, const char *text) {
-    std::string level_str;
-    switch (level) {
-    case GGML_LOG_LEVEL_ERROR:
-        level_str = "ERROR";
-        break;
-    case GGML_LOG_LEVEL_WARN:
-        level_str = "WARN";
-        break;
-    case GGML_LOG_LEVEL_INFO:
-        level_str = "INFO";
-        break;
-    default:
-    case GGML_LOG_LEVEL_DEBUG:
-        level_str = "DEBUG";
-        break;
-    }
-    nlohmann::json log_obj = {{"timestamp", std::time(nullptr)}, {"level", level_str}, {"message", text}};
-    return log_obj.dump();
-}
-
 /**
  * Invoke the log callback if there is any. When JSON mode is enabled,
  * the message is formatted as a JSON object before forwarding.
  */
 void log_callback_trampoline(ggml_log_level level, const char *text, void *user_data) {
     if (log_callback != nullptr) {
         if (log_json) {
-            std::string json_text = format_log_as_json(level, text);
+            std::string json_text = format_log_as_json(level, text, std::time(nullptr));
             log_callback(level, json_text.c_str(), user_data);
         } else {
             log_callback(level, text, user_data);
@@ -477,21 +490,12 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *reserved) {
         goto error;
     }
 
-    // create references
-    c_llama_model = (jclass)env->NewGlobalRef(c_llama_model);
-    c_string = (jclass)env->NewGlobalRef(c_string);
-    c_hash_map = (jclass)env->NewGlobalRef(c_hash_map);
-    c_map = (jclass)env->NewGlobalRef(c_map);
-    c_set = (jclass)env->NewGlobalRef(c_set);
-    c_entry = (jclass)env->NewGlobalRef(c_entry);
-    c_iterator = (jclass)env->NewGlobalRef(c_iterator);
-    c_integer = (jclass)env->NewGlobalRef(c_integer);
-    c_float = (jclass)env->NewGlobalRef(c_float);
-    c_biconsumer = (jclass)env->NewGlobalRef(c_biconsumer);
-    c_llama_error = (jclass)env->NewGlobalRef(c_llama_error);
-    c_log_level = (jclass)env->NewGlobalRef(c_log_level);
-    c_log_format = (jclass)env->NewGlobalRef(c_log_format);
-    c_error_oom = (jclass)env->NewGlobalRef(c_error_oom);
+    // Promote local class refs (from FindClass) to global refs in one pass.
+    // c_standard_charsets is intentionally omitted: only used to look up
+    // f_utf_8 in this function and never referenced again.
+    for (jclass *p : g_global_class_refs) {
+        *p = (jclass)env->NewGlobalRef(*p);
+    }
 
     // find constructors
     cc_hash_map = env->GetMethodID(c_hash_map, "<init>", "()V");
@@ -536,25 +540,18 @@ JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *reserved) {
     }
 
     o_utf_8 = env->NewStringUTF("UTF-8");
-    o_log_level_debug = env->GetStaticObjectField(c_log_level, f_log_level_debug);
-    o_log_level_info = env->GetStaticObjectField(c_log_level, f_log_level_info);
-    o_log_level_warn = env->GetStaticObjectField(c_log_level, f_log_level_warn);
-    o_log_level_error = env->GetStaticObjectField(c_log_level, f_log_level_error);
-    o_log_format_json = env->GetStaticObjectField(c_log_format, f_log_format_json);
-    o_log_format_text = env->GetStaticObjectField(c_log_format, f_log_format_text);
+    for (const auto &b : g_static_object_bindings) {
+        *b.target = env->GetStaticObjectField(*b.cls, *b.field);
+    }
 
     if (!(o_utf_8 && o_log_level_debug && o_log_level_info && o_log_level_warn && o_log_level_error &&
           o_log_format_json && o_log_format_text)) {
         goto error;
     }
 
-    o_utf_8 = env->NewGlobalRef(o_utf_8);
-    o_log_level_debug = env->NewGlobalRef(o_log_level_debug);
-    o_log_level_info = env->NewGlobalRef(o_log_level_info);
-    o_log_level_warn = env->NewGlobalRef(o_log_level_warn);
-    o_log_level_error = env->NewGlobalRef(o_log_level_error);
-    o_log_format_json = env->NewGlobalRef(o_log_format_json);
-    o_log_format_text = env->NewGlobalRef(o_log_format_text);
+    for (jobject *p : g_global_object_refs) {
+        *p = env->NewGlobalRef(*p);
+    }
 
     if (env->ExceptionCheck()) {
         env->ExceptionDescribe();
@@ -587,28 +584,12 @@ JNIEXPORT void JNICALL JNI_OnUnload(JavaVM *vm, void *reserved) {
         return;
     }
 
-    env->DeleteGlobalRef(c_llama_model);
-    env->DeleteGlobalRef(c_string);
-    env->DeleteGlobalRef(c_hash_map);
-    env->DeleteGlobalRef(c_map);
-    env->DeleteGlobalRef(c_set);
-    env->DeleteGlobalRef(c_entry);
-    env->DeleteGlobalRef(c_iterator);
-    env->DeleteGlobalRef(c_integer);
-    env->DeleteGlobalRef(c_float);
-    env->DeleteGlobalRef(c_biconsumer);
-    env->DeleteGlobalRef(c_llama_error);
-    env->DeleteGlobalRef(c_log_level);
-    env->DeleteGlobalRef(c_log_format);
-    env->DeleteGlobalRef(c_error_oom);
-
-    env->DeleteGlobalRef(o_utf_8);
-    env->DeleteGlobalRef(o_log_level_debug);
-    env->DeleteGlobalRef(o_log_level_info);
-    env->DeleteGlobalRef(o_log_level_warn);
-    env->DeleteGlobalRef(o_log_level_error);
-    env->DeleteGlobalRef(o_log_format_json);
-    env->DeleteGlobalRef(o_log_format_text);
+    for (jclass *p : g_global_class_refs) {
+        env->DeleteGlobalRef(*p);
+    }
+    for (jobject *p : g_global_object_refs) {
+        env->DeleteGlobalRef(*p);
+    }
 
     if (o_log_callback != nullptr) {
         env->DeleteGlobalRef(o_log_callback);
@@ -769,8 +750,7 @@ JNIEXPORT jint JNICALL Java_net_ladenthin_llama_LlamaModel_requestCompletion(JNI
 
 JNIEXPORT void JNICALL Java_net_ladenthin_llama_LlamaModel_releaseTask(JNIEnv *env, jobject obj, jint id_task) {
     REQUIRE_SERVER_CONTEXT();
-    std::lock_guard<std::mutex> lk(jctx->readers_mutex);
-    jctx->readers.erase(id_task);
+    erase_reader(jctx, id_task);
 }
 
 JNIEXPORT jstring JNICALL Java_net_ladenthin_llama_LlamaModel_receiveCompletionJson(JNIEnv *env, jobject obj,
@@ -791,17 +771,15 @@ JNIEXPORT jstring JNICALL Java_net_ladenthin_llama_LlamaModel_receiveCompletionJ
     server_task_result_ptr result = rd->next([] { return false; });
 
     if (!result_ok_or_throw(env, result)) {
-        std::lock_guard<std::mutex> lk(jctx->readers_mutex);
-        jctx->readers.erase(id_task);
+        erase_reader(jctx, id_task);
         return nullptr;
     }
 
     json response      = result->to_json();
     response["stop"]   = result->is_stop();
 
     if (result->is_stop()) {
-        std::lock_guard<std::mutex> lk(jctx->readers_mutex);
-        jctx->readers.erase(id_task);
+        erase_reader(jctx, id_task);
     }
 
     return json_to_jstring_impl(env, response);
@@ -810,9 +788,7 @@ JNIEXPORT jstring JNICALL Java_net_ladenthin_llama_LlamaModel_receiveCompletionJ
 JNIEXPORT jfloatArray JNICALL Java_net_ladenthin_llama_LlamaModel_embed(JNIEnv *env, jobject obj, jstring jprompt) {
     REQUIRE_SERVER_CONTEXT(nullptr);
 
-    if (!jctx->params.embedding) {
-        env->ThrowNew(c_llama_error,
-                      "Model was not loaded with embedding support (see ModelParameters#setEmbedding(boolean))");
+    if (!require_embedding_support(env, jctx->params.embedding, c_llama_error)) {
         return nullptr;
     }
 
@@ -972,8 +948,7 @@ JNIEXPORT void JNICALL Java_net_ladenthin_llama_LlamaModel_delete(JNIEnv *env, j
 
 JNIEXPORT void JNICALL Java_net_ladenthin_llama_LlamaModel_cancelCompletion(JNIEnv *env, jobject obj, jint id_task) {
     REQUIRE_SERVER_CONTEXT();
-    std::lock_guard<std::mutex> lk(jctx->readers_mutex);
-    jctx->readers.erase(id_task);
+    erase_reader(jctx, id_task);
 }
 
 JNIEXPORT void JNICALL Java_net_ladenthin_llama_LlamaModel_setLogger(JNIEnv *env, jclass clazz, jobject log_format,
@@ -1076,9 +1051,7 @@ JNIEXPORT jstring JNICALL Java_net_ladenthin_llama_LlamaModel_handleEmbeddings(J
                                                                            jstring jparams, jboolean joaiCompat) {
     REQUIRE_SERVER_CONTEXT(nullptr);
 
-    if (!jctx->params.embedding) {
-        env->ThrowNew(c_llama_error,
-                      "Model was not loaded with embedding support (see ModelParameters#setEmbedding(boolean))");
+    if (!require_embedding_support(env, jctx->params.embedding, c_llama_error)) {
         return nullptr;
     }
 

src/main/cpp/jni_helpers.hpp

@@ -70,6 +70,27 @@ struct jllama_context {
     std::map<int, std::unique_ptr<server_response_reader>> readers;
 };
 
+// Removes the streaming reader entry for `id_task` under the readers_mutex.
+// No-op if the id is not in the map. Used to release the JNI side of a
+// completed, cancelled, or failed streaming task.
+inline void erase_reader(jllama_context *jctx, int id_task) {
+    std::lock_guard<std::mutex> lk(jctx->readers_mutex);
+    jctx->readers.erase(id_task);
+}
+
+// Guard: throw and return false if the model was loaded without embedding
+// support enabled. Used by every JNI entry point that produces embeddings.
+[[nodiscard]] inline bool require_embedding_support(JNIEnv         *env,
+                                                     bool            embedding_enabled,
+                                                     jclass          error_class) {
+    if (embedding_enabled) {
+        return true;
+    }
+    env->ThrowNew(error_class,
+                  "Model was not loaded with embedding support (see ModelParameters#setEmbedding(boolean))");
+    return false;
+}
+
 // ---------------------------------------------------------------------------
 // get_jllama_context_impl
 //

src/main/cpp/log_helpers.hpp

@@ -0,0 +1,40 @@
+// SPDX-FileCopyrightText: 2026 Bernard Ladenthin <bernard.ladenthin@gmail.com>
+// SPDX-FileCopyrightText: 2023-2025 Konstantin Herud
+//
+// SPDX-License-Identifier: MIT
+
+#pragma once
+
+// log_helpers.hpp — Pure log-formatting helpers.
+//
+// No JNI, no llama state. Depends only on the ggml_log_level enum (declared
+// in ggml.h) and nlohmann/json. Unit-testable without a JVM or model.
+
+#include "ggml.h"
+#include "nlohmann/json.hpp"
+
+#include <ctime>
+#include <string>
+
+// Returns the canonical short name for a ggml log level. INFO is the default
+// fall-through to mirror llama.cpp's own log routing.
+[[nodiscard]] inline const char *log_level_name(ggml_log_level level) {
+    switch (level) {
+    case GGML_LOG_LEVEL_ERROR: return "ERROR";
+    case GGML_LOG_LEVEL_WARN:  return "WARN";
+    case GGML_LOG_LEVEL_DEBUG: return "DEBUG";
+    case GGML_LOG_LEVEL_INFO:
+    default:                   return "INFO";
+    }
+}
+
+// Pure variant taking an explicit timestamp so tests are deterministic.
+[[nodiscard]] inline std::string format_log_as_json(
+        ggml_log_level level, const char *text, std::time_t timestamp) {
+    nlohmann::json log_obj = {
+        {"timestamp", timestamp},
+        {"level",     log_level_name(level)},
+        {"message",   text ? text : ""},
+    };
+    return log_obj.dump();
+}

src/test/cpp/test_jni_helpers.cpp

@@ -166,6 +166,47 @@ TEST(JllamaContextReaders, Erase_MapBecomesEmpty) {
     EXPECT_TRUE(ctx.readers.empty());
 }
 
+TEST(EraseReader, RemovesExistingEntry) {
+    jllama_context ctx;
+    {
+        std::lock_guard<std::mutex> lk(ctx.readers_mutex);
+        ctx.readers.emplace(11, nullptr);
+    }
+    erase_reader(&ctx, 11);
+    std::lock_guard<std::mutex> lk(ctx.readers_mutex);
+    EXPECT_TRUE(ctx.readers.empty());
+}
+
+TEST(EraseReader, MissingIdIsNoOp) {
+    jllama_context ctx;
+    {
+        std::lock_guard<std::mutex> lk(ctx.readers_mutex);
+        ctx.readers.emplace(1, nullptr);
+        ctx.readers.emplace(2, nullptr);
+    }
+    erase_reader(&ctx, 99); // not present — must not throw or modify state
+    std::lock_guard<std::mutex> lk(ctx.readers_mutex);
+    EXPECT_EQ(ctx.readers.size(), 2u);
+    EXPECT_TRUE(ctx.readers.count(1));
+    EXPECT_TRUE(ctx.readers.count(2));
+}
+
+TEST(EraseReader, OnlyRemovesGivenId) {
+    jllama_context ctx;
+    {
+        std::lock_guard<std::mutex> lk(ctx.readers_mutex);
+        ctx.readers.emplace(5, nullptr);
+        ctx.readers.emplace(6, nullptr);
+        ctx.readers.emplace(7, nullptr);
+    }
+    erase_reader(&ctx, 6);
+    std::lock_guard<std::mutex> lk(ctx.readers_mutex);
+    EXPECT_EQ(ctx.readers.size(), 2u);
+    EXPECT_TRUE(ctx.readers.count(5));
+    EXPECT_FALSE(ctx.readers.count(6));
+    EXPECT_TRUE(ctx.readers.count(7));
+}
+
 TEST(JllamaContextReaders, MultipleTaskIds_IndependentSlots) {
     // Erase one task id while others remain — models cancelCompletion
     // mid-stream without disturbing other active streaming tasks.
@@ -222,6 +263,22 @@ TEST_F(MockJniFixture, GetJllamaContext_ReturnsWrapperNotInnerServer) {
     // the type-level distinction (jllama_context* vs server_context*) is sufficient.
 }
 
+// ============================================================
+// require_embedding_support
+// ============================================================
+
+TEST_F(MockJniFixture, RequireEmbeddingSupport_Enabled_ReturnsTrueNoThrow) {
+    EXPECT_TRUE(require_embedding_support(env, true, dummy_class));
+    EXPECT_FALSE(g_throw_called);
+}
+
+TEST_F(MockJniFixture, RequireEmbeddingSupport_Disabled_ReturnsFalseAndThrows) {
+    EXPECT_FALSE(require_embedding_support(env, false, dummy_class));
+    EXPECT_TRUE(g_throw_called);
+    EXPECT_NE(g_throw_message.find("embedding support"), std::string::npos);
+    EXPECT_NE(g_throw_message.find("setEmbedding"), std::string::npos);
+}
+
 // ============================================================
 // require_json_field_impl
 // ============================================================