From 2e65a8af365d9fc4257bc98358d45d0586262486 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 10:40:52 +0000 Subject: [PATCH 1/5] CMake: repeat GGML_CUDA_GRAPHS_DEFAULT in the variants path, write jllama-extras.txt on Windows The JLLAMA_CPU_VARIANTS path adds ggml before llama.cpp and therefore skips the block in which llama.cpp changes two ggml defaults; it repeated only GGML_LLAMAFILE_DEFAULT. ggml's own default for GGML_CUDA_GRAPHS is OFF, and a CUDA backend built as a module in this path ran without CUDA graphs and lost two thirds of its token generation (265.6 -> 88.8 tg128 on an RTX 3070 at b11538, docs/handover/local-agent-report-b11538-windows.md, addendum 3). Both defaults are repeated now, and the comment states the rule: every *_DEFAULT llama.cpp sets before its own add_subdirectory(ggml) is repeated here, re-checked with `grep -n _DEFAULT CMakeLists.txt` on every bump. Verified without a CUDA toolkit (none in this environment): ggml declares the option unconditionally, and the variants build's CMakeCache now reads GGML_CUDA_GRAPHS:BOOL=ON like the static build's; a CUDA build itself was not possible here. On Windows the variants build now writes jllama-extras.txt (ggml-base.dll, ggml.dll -- the two libraries jllama.dll links, left out of jllama-files.txt on purpose), byte-identical to what merge-native-artifacts.sh derives in CI: without it LlamaLoader extracted jllama.dll alone, Windows resolved its ggml.dll import from java.exe's directory or PATH, and 57 model-backed tests of a local variants build failed on the load. Both file lists are written with LF (file(GENERATE) wrote CRLF on Windows, measured), and the merge script strips a CR when it reads jllama-files.txt: with the CRLF list the previous script failed with "names 'ggml-cpu-x64.dll', which the build did not produce" (reproduced over a simulated 27-artifact tree), and a CR that slipped through would also have put every CPU module into jllama-extras.txt. A Linux x86-64 variants build configures and builds with the change (18 libraries + jllama-files.txt, no extras file), and the merge script accepts the simulated tree with a pre-existing extras file, rewriting it identically. CLAUDE.md "CPU variants" states the defaults rule; TODO's stage-3 item carries the module-beside-the-CPU-set findings (GPU module built but not copied, the graphs default now repeated, not yet re-measured with CUDA). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01AytmJF9faEiQEVt6eetQS2 --- .github/merge-native-artifacts.sh | 5 ++++- CHANGELOG.md | 11 +++++++++++ CLAUDE.md | 9 ++++++++- TODO.md | 13 ++++++++++++- llama/CMakeLists.txt | 26 +++++++++++++++++++++++--- 5 files changed, 58 insertions(+), 6 deletions(-) diff --git a/.github/merge-native-artifacts.sh b/.github/merge-native-artifacts.sh index 22dcc733f..e066ccbf2 100755 --- a/.github/merge-native-artifacts.sh +++ b/.github/merge-native-artifacts.sh @@ -110,7 +110,10 @@ done find "$DEST" -mindepth 3 -maxdepth 3 -type d | sort | while IFS= read -r dir; do plain="" if [ -f "$dir/jllama-files.txt" ]; then - plain="$(grep -v -e '^#' -e '^$' "$dir/jllama-files.txt" || true)" + # tr -d '\r': a Windows build may write the list with CRLF; a name carrying its CR would fail + # the existence check below and, worse, miss the awk skip-list, so the CPU modules would land in + # jllama-extras.txt and LlamaLoader would load every one of them. + plain="$(tr -d '\r' < "$dir/jllama-files.txt" | grep -v -e '^#' -e '^$' || true)" while IFS= read -r f; do [ -z "$f" ] || [ -f "$dir/$f" ] \ || { echo "::error::${dir#"$DEST"}/jllama-files.txt names '$f', which the build did not produce" >&2; exit 1; } diff --git a/CHANGELOG.md b/CHANGELOG.md index dbb762a4e..6cf632820 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -304,6 +304,17 @@ from version 5.0.0 onward. Pre-fork releases (`1.x`–`4.2.0`) were authored by 4.4.7 (none of the files the agent's carried JLine fixes touch changed between 4.4.6 and 4.4.7). ### Fixed +- **`GGML_CUDA_GRAPHS_DEFAULT` is repeated in the CPU-variants CMake path.** That path adds ggml before + llama.cpp and therefore skips the block in which llama.cpp changes two ggml defaults; it repeated + only `GGML_LLAMAFILE_DEFAULT`. Latent for the shipped CPU jars, but a CUDA backend built as a module + in this path ran without CUDA graphs and lost two thirds of its token generation (measured at b11538 + on an RTX 3070, `docs/handover/local-agent-report-b11538-windows.md`). The CMake comment now states + the rule: every `*_DEFAULT` llama.cpp sets before its own `add_subdirectory(ggml)` is repeated. +- **A Windows `JLLAMA_CPU_VARIANTS` build is loadable without the CI merge step.** `CMakeLists.txt` + writes `jllama-extras.txt` (`ggml-base.dll`, `ggml.dll`, the two libraries `jllama.dll` links) next to + the library, byte-identical to what `merge-native-artifacts.sh` derives in CI; without it + `LlamaLoader` extracted `jllama.dll` alone, Windows resolved its `ggml.dll` import from `java.exe`'s + directory or `PATH`, and every model-backed test of a local variants build failed on the load. - **`OpenAiCompatServer` answers `400`, not `500`, for a request the native layer rejects** -- an empty `input` array for `/v1/embeddings`, a `/v1/completions` body without a `prompt`, a malformed body, a message the chat template cannot take, a prompt over the context size -- on every surface (OpenAI, diff --git a/CLAUDE.md b/CLAUDE.md index 8e56483b4..03b7c3e66 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -97,7 +97,14 @@ below): `SOURCE_SUBDIR`, so it only fetches and patches; ggml is then added first with `BUILD_SHARED_LIBS ON` and llama.cpp afterwards with it `OFF` (llama.cpp skips its own `add_subdirectory(ggml)` when a `ggml` target exists). llama, mtmd and llama-common stay static in - `libjllama.so`. The one ggml default llama.cpp changes itself (`GGML_LLAMAFILE`) is repeated. + `libjllama.so`. **Every ggml default llama.cpp's own CMakeLists changes before its + `add_subdirectory(ggml)` is repeated** -- `GGML_LLAMAFILE_DEFAULT` and `GGML_CUDA_GRAPHS_DEFAULT` at + b11538 -- because adding ggml first skips that block; re-check with `grep -n _DEFAULT CMakeLists.txt` + at the new tag on every bump. The second one was missing until 5.2.x: ggml's own default for CUDA + graphs is OFF, and a CUDA backend built as a module in this path lost two thirds of its token + generation (265.6 -> 88.8 tg128 on an RTX 3070, prompt processing -20%; Vulkan, which has no + equivalent, lost nothing) -- latent, since no CI job builds the variants path with CUDA yet + (`TODO.md` "GPU backends as modules"). - **The directory holds 18 files on x86-64**: `libjllama.so`, `libggml.so`, `libggml-base.so` (unversioned: the loader extracts plain files, so file name, SONAME and `DT_NEEDED` must agree), 14 `libggml-cpu-.so` (`x64`, `sse42`, `sandybridge`, `ivybridge`, `piledriver`, `haswell`, diff --git a/TODO.md b/TODO.md index 7aba9ff38..b7c249b31 100644 --- a/TODO.md +++ b/TODO.md @@ -152,7 +152,18 @@ deliverable 6: dependency. Also: `ggml-cuda.dll` imports only `cublas64_13.dll` (cudart is static) -- the requirement is smaller than the README says; and device indices are not stable across the set of loaded backends (`Vulkan0` was the NVIDIA GPU with CUDA loaded, the AMD iGPU without), so any - device setting must go by name, never by index. + device setting must go by name, never by index. **Measured through this project's own CMake at + b11538** (`docs/handover/local-agent-report-b11538-windows.md`, addendum 3): `-DJLLAMA_CPU_VARIANTS=ON` + together with `-DGGML_CUDA=ON` or `-DGGML_VULKAN=ON` builds (nvcc accepts the plain-clang host + compiler), and a directory holding the 14 CPU modules plus the GPU module offloads all layers; + `verify-native-deps.py` accepts it. Two things stand between that and a shippable jar: **(A)** the + GPU module is built into `build/bin/Release/` and never copied -- the file-list block in + `llama/CMakeLists.txt` knows only the CPU modules, `ggml`, `ggml-base` and `ggml-rpc`, so + `jllama-files.txt` and the copy would have to learn about GPU backend modules; **(B)** the CUDA + module lost two thirds of its token generation because the variants path did not repeat + `GGML_CUDA_GRAPHS_DEFAULT ON` -- fixed in the CMakeLists (both llama.cpp defaults are repeated now, + CLAUDE.md "CPU variants"), not yet re-measured with CUDA. And such a jar is 18 files plus a 44-50 MB + GPU module that `LlamaLoader` extracts on every first use (item 4 below). 4. **Loader: reuse the extraction across runs.** The directory is keyed per build now (`extractionDirectoryName`), but every start still re-extracts: `cleanup()` deletes every `jllama*` path first and `deleteOnExit` removes the files at exit. Copying the 18 files costs ~1 s diff --git a/llama/CMakeLists.txt b/llama/CMakeLists.txt index 74b759dd6..3dbd88977 100644 --- a/llama/CMakeLists.txt +++ b/llama/CMakeLists.txt @@ -318,8 +318,14 @@ if(JLLAMA_CPU_VARIANTS) set(BUILD_SHARED_LIBS ON) set(GGML_BACKEND_DL ON CACHE BOOL "" FORCE) set(GGML_CPU_ALL_VARIANTS ON CACHE BOOL "" FORCE) - # The one ggml default llama.cpp's CMakeLists changes before adding ggml itself. - set(GGML_LLAMAFILE_DEFAULT ON) + # The ggml defaults llama.cpp's own CMakeLists changes before its add_subdirectory(ggml): every + # `set(GGML_*_DEFAULT ...)` there ("change the default for these ggml options") must be + # repeated here, because adding ggml first skips that block entirely. Re-check on every bump + # with `grep -n _DEFAULT CMakeLists.txt` at the new tag. Missing GGML_CUDA_GRAPHS_DEFAULT cost a + # CUDA module built this way two thirds of its token generation (measured, TODO.md "GPU + # backends as modules"): ggml's own default is OFF, llama.cpp turns it ON for itself. + set(GGML_LLAMAFILE_DEFAULT ON) + set(GGML_CUDA_GRAPHS_DEFAULT ON) add_subdirectory(${llama.cpp_SOURCE_DIR}/ggml ${llama.cpp_BINARY_DIR}/ggml) set(BUILD_SHARED_LIBS OFF) add_subdirectory(${llama.cpp_SOURCE_DIR} ${llama.cpp_BINARY_DIR}) @@ -722,7 +728,21 @@ if(JLLAMA_CPU_VARIANTS) endif() string(APPEND JLLAMA_FILES_CONTENT "$\n") endforeach() - file(GENERATE OUTPUT ${JLLAMA_DIR}/jllama-files.txt CONTENT "${JLLAMA_FILES_CONTENT}") + # NEWLINE_STYLE LF on both lists: on Windows file(GENERATE) otherwise writes CRLF (measured), and + # merge-native-artifacts.sh reads them on Linux -- it strips a CR itself now, but the extras + # file below is meant to be byte-identical to what that script writes, which is LF. + file(GENERATE OUTPUT ${JLLAMA_DIR}/jllama-files.txt CONTENT "${JLLAMA_FILES_CONTENT}" NEWLINE_STYLE LF) + if(OS_NAME STREQUAL "Windows") + # The counterpart of the two names left out above: in CI merge-native-artifacts.sh derives + # jllama-extras.txt as "every file next to the library that jllama-files.txt does not name, + # sorted", which here is exactly ggml-base.dll then ggml.dll. Writing the same two lines from + # the build (same order, no comment line, so the merge step rewrites an identical file) makes + # a local variants build loadable without the merge step: without the file LlamaLoader + # extracts jllama.dll alone, System.load resolves its ggml.dll import from java.exe's + # directory or PATH, and every model-backed test fails on the load. + file(GENERATE OUTPUT ${JLLAMA_DIR}/jllama-extras.txt + CONTENT "$\n$\n" NEWLINE_STYLE LF) + endif() message(STATUS "CPU variants: ${JLLAMA_GGML_TARGETS}") endif() From bc3497cbc036b9f897ad297b5e2b1fbaedb0826f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 10:40:53 +0000 Subject: [PATCH 2/5] Tests: -D overrides for the four shipped model paths; JDK note; doc refresh from the local agent's report net.ladenthin.llama.text.model, .draft.model, .reasoning.model and .rerank.model override TestConstants.MODEL_PATH, DRAFT_MODEL_PATH, REASONING_MODEL_PATH and RERANKING_MODEL_PATH, the way every capability-specific model already had one, so a developer can run the suite against models already on the machine. The defaults stay the CI model set: TestConstantsTest now pins that over the DEFAULT_* constants (an override must not red it) and checks each resolved constant against the resolved property, which holds whether or not it is set. CLAUDE.md and README name the JDK requirement the local agent lost a deliverable to: javac must accept -XDaddTypeAnnotationsToSymbol (NullAway's JSpecify mode), which Oracle JDK 21 does not and Temurin 21 does. The executed-test band includes the Windows measurement (1872), the local test plan no longer lists the four paths as the ones without an override, and the handover prompt docs/handover/local-agent-pr-a.md goes -- its deliverables are answered in docs/handover/local-agent-report-b11538-windows.md. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01AytmJF9faEiQEVt6eetQS2 --- CHANGELOG.md | 6 + CLAUDE.md | 13 +- README.md | 11 ++ docs/handover/local-agent-pr-a.md | 144 ------------------ docs/local-test-plan.md | 8 +- .../net/ladenthin/llama/TestConstants.java | 48 +++++- .../ladenthin/llama/TestConstantsTest.java | 35 ++++- 7 files changed, 108 insertions(+), 157 deletions(-) delete mode 100644 docs/handover/local-agent-pr-a.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cf632820..6d5c4549a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ from version 5.0.0 onward. Pre-fork releases (`1.x`–`4.2.0`) were authored by ## [Unreleased] ### Added +- **`-D` overrides for the four shipped test models.** `net.ladenthin.llama.text.model`, `.draft.model`, + `.reasoning.model` and `.rerank.model` override `TestConstants.MODEL_PATH`, `DRAFT_MODEL_PATH`, + `REASONING_MODEL_PATH` and `RERANKING_MODEL_PATH` the way every capability-specific model already had + one, so a developer can run the suite against models already on the machine (README "System + Properties Reference"). The defaults stay the CI model set, which `TestConstantsTest` pins over the + `DEFAULT_*` constants. - **`InvalidRequestException`** (`net.ladenthin.llama.exception`, a `LlamaException`): the JNI layer now throws it wherever llama.cpp's own HTTP server answers `400` (`invalid_request_error`) -- a body that is not valid JSON, a missing `"prompt"` / `"messages"` / `"input_prefix"`, an empty embedding input, an diff --git a/CLAUDE.md b/CLAUDE.md index 03b7c3e66..053554175 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1575,6 +1575,13 @@ Built libraries are placed in `src/main/natives/net/ladenthin/llama/{OS}/{ARCH}/ ### Building the native library for local Java tests +**The JDK must be one whose `javac` accepts `-XDaddTypeAnnotationsToSymbol` -- Temurin 21 (what CI +uses, `distribution: temurin`), not Oracle JDK 21.** `llama/pom.xml` passes that flag because +NullAway's JSpecify mode needs it below JDK 22; on Oracle JDK 21.0.9 `mvn compile` dies in an +Error Prone `IllegalStateException` naming exactly that flag, while Temurin 21.0.12.1 compiles the +same tree (measured, `docs/handover/local-agent-report-b11538-windows.md`, deliverable 1). Check +`mvn -version` before blaming anything else. + `mvn test` does **not** build the native library — Maven only compiles Java and runs surefire. The shared library must already exist on disk under the platform-specific path that `LlamaLoader` resolves at runtime; surefire puts @@ -1646,6 +1653,10 @@ below covers the model bindings: | Property | Default test that uses it | Default (`models/…` unless noted) | |----------|---------------------------|-------| +| `net.ladenthin.llama.text.model` | `LlamaModelTest` and most model-backed tests (`MODEL_PATH`) | `codellama-7b.Q2_K.gguf` | +| `net.ladenthin.llama.draft.model` | speculative decoding + the small-model tests (`DRAFT_MODEL_PATH`) | `AMD-Llama-135m-code.Q2_K.gguf` | +| `net.ladenthin.llama.reasoning.model` | reasoning-budget tests (`REASONING_MODEL_PATH`) | `Qwen3-0.6B-Q4_K_M.gguf` (must emit a thinking block) | +| `net.ladenthin.llama.rerank.model` | reranking tests (`RERANKING_MODEL_PATH`) | `jina-reranker-v1-tiny-en-Q4_0.gguf` (must be a reranker) | | `net.ladenthin.llama.tool.model` | `ToolCallingIntegrationTest`, `OpenAiServerToolCallingIntegrationTest` | `Qwen2.5-1.5B-Instruct-Q4_K_M.gguf` | | `net.ladenthin.llama.nomic.path` | `LlamaEmbeddingsTest#testNomicEmbedLoads` | `nomic-embed-text-v1.5.f16.gguf` (issue #98 regression) | | `net.ladenthin.llama.vision.model` | `MultimodalIntegrationTest` | `SmolVLM-500M-Instruct-Q8_0.gguf` (any vision-capable GGUF works) | @@ -2091,7 +2102,7 @@ record `tests="0" errors="0" skipped="0"` — the class contributes **no** test the form "did this run skip anything?" is blind to it. The only thing that catches it directly is a floor on the number of tests actually executed -- `.github/verify-test-counts.sh`, run by every `test-java-*` job: no class with zero entries, and at least 1800 tests executed (run minus skipped; -the jobs execute 1856 to 1865, a checkout without models 1589). It is why several stale +the jobs execute 1856 to 1872, a checkout without models 1589). It is why several stale assertions (e.g. `LlamaModelTest#testGetMetrics` against a payload shape upstream had dropped at b10408) never failed in CI. The fix is **`TestConstants.resolveModelPath` / `resolveModelProperty`**, which accept either layout — module-relative first, then the reactor root diff --git a/README.md b/README.md index 525e13a54..90526eb7d 100644 --- a/README.md +++ b/README.md @@ -375,6 +375,10 @@ Every `net.ladenthin.llama.*` system property recognised by the library, deep-sc | `net.ladenthin.llama.osinfo.architecture` | unset (uses `os.arch`) | runtime | `OSInfo` | Override for the architecture string used to locate the bundled library inside the JAR. Useful when `os.arch` reports an unexpected value (e.g. inside dockcross / chrooted environments). | | `net.ladenthin.llama.backend` | unset (auto: the first backend on the classpath whose library loads) | runtime | `LlamaLoader` | Names one backend directory (e.g. `cuda13`, `vulkan`, `cpu`) to load exclusively — failure is then fatal instead of trying the next backend. See [Choosing the natives jars](#choosing-the-natives-jars). | | `net.ladenthin.llama.test.ngl` | `43` for the general suite; `0` for `ToolCallingIntegrationTest` | test | Model-backed integration tests | Number of GPU layers used during testing. Pin to `0` on CPU-only hosts: `mvn test -Dnet.ladenthin.llama.test.ngl=0`. The tool test also selects device `none` at zero layers so Metal/CUDA is not initialized. | +| `net.ladenthin.llama.text.model` | `models/codellama-7b.Q2_K.gguf` (tests self-skip if missing) | test | `LlamaModelTest` and most model-backed tests (`TestConstants.MODEL_PATH`) | Path to the main text-generation GGUF. Most tests only need an instruct-capable model; a few assertions are tuned to the CI model. | +| `net.ladenthin.llama.draft.model` | `models/AMD-Llama-135m-code.Q2_K.gguf` (tests self-skip if missing) | test | Speculative-decoding tests and the tests that need a small, fast model (`TestConstants.DRAFT_MODEL_PATH`) | Path to the draft GGUF. | +| `net.ladenthin.llama.reasoning.model` | `models/Qwen3-0.6B-Q4_K_M.gguf` (tests self-skip if missing) | test | Reasoning-budget tests (`TestConstants.REASONING_MODEL_PATH`) | Path to a thinking model (one that emits a thinking block). | +| `net.ladenthin.llama.rerank.model` | `models/jina-reranker-v1-tiny-en-Q4_0.gguf` (tests self-skip if missing) | test | Reranking tests (`TestConstants.RERANKING_MODEL_PATH`) | Path to a reranker GGUF (must load with `enableReranking()`). | | `net.ladenthin.llama.tool.model` | `models/Qwen2.5-1.5B-Instruct-Q4_K_M.gguf` (test self-skips if missing) | test | `ToolCallingIntegrationTest` | Path to a tool-capable GGUF used to verify required blocking and streaming tool calls. The default matches the Qwen2.5 model in upstream llama.cpp's tool-call test matrix. | | `net.ladenthin.llama.nomic.path` | `models/nomic-embed-text-v1.5.f16.gguf` (test self-skips if missing) | test | `LlamaEmbeddingsTest#testNomicEmbedLoads` | Path to a Nomic embedding model (`nomic-embed-text-v1.5.f16.gguf` or a compatible BERT-family encoder). Regression test for upstream issue #98 (BERT-encoder `result_output` assertion). | | `net.ladenthin.llama.vision.model` | `models/SmolVLM-500M-Instruct-Q8_0.gguf` (test self-skips if missing) | test | `MultimodalIntegrationTest` | Path to a vision-capable model GGUF. Any vision-capable GGUF works; CI default is `SmolVLM-500M-Instruct-Q8_0.gguf`. | @@ -1564,6 +1568,13 @@ If you still see a crash inside `msvcp140.dll`, it originates in **another** nat into the same JVM, not in `jllama.dll` -- check the rest of the `hs_err` frame list. Please open an issue with that file rather than editing your JDK installation. +### Contributors: build with Temurin 21, not Oracle JDK 21 + +`llama/pom.xml` passes `-XDaddTypeAnnotationsToSymbol=true` to `javac` (NullAway's JSpecify mode +needs it below JDK 22). **Oracle JDK 21 rejects that flag**: `mvn compile` dies in an Error Prone +`IllegalStateException` naming it (measured on 21.0.9). Eclipse Temurin 21, which CI uses, compiles +the same tree; `mvn -version` shows which JDK Maven runs on. + ### Contributors: do not upgrade jqwik past 1.9.3 > ⚠️ **DO NOT UPGRADE jqwik past 1.9.3.** jqwik 1.10.0 added an anti-AI prompt-injection string to test stdout; the 1.10.1 user guide states the library "is not meant to be used by any 'AI' coding agents at all." 1.9.3 is the last pre-disclosure release and is the pinned version. See `CLAUDE.md` section "jqwik prompt-injection in test output" for the full context. Dependabot is configured to ignore **all** `net.jqwik` updates (every version, including patches) — see the `ignore` rule in [`.github/dependabot.yml`](./.github/dependabot.yml). diff --git a/docs/handover/local-agent-pr-a.md b/docs/handover/local-agent-pr-a.md deleted file mode 100644 index ea57dc15e..000000000 --- a/docs/handover/local-agent-pr-a.md +++ /dev/null @@ -1,144 +0,0 @@ -# Handover prompt for the local agent -- PR A on `claude/hopeful-pascal-9jlbqb` - -Paste the text below the line into the local agent (Windows workstation: Ryzen 7 5800H, RTX 3070, -AMD iGPU, MSVC + LLVM + Docker + JDK 21 + Maven). It describes what this branch already contains, -what the remote session cannot verify from its sandbox, and the exact deliverables. - -**Status (2026-10-09).** PR A (#496) and the agent's own corrections to this file (#498) are merged; -the branch has since moved on (b11538, `InvalidRequestException`). The file stays for one reason: -deliverable 6 (B2) carries the agent's measurements and the four `OPEN` items that the Windows -CPU-variants CI job still needs, and `TODO.md` ("CPU variants") points here for them. Deliverables -1-5 were not reported and are not asked for again; delete the file when B2 lands. - -**Deliverable 6 (B2) carries measurements taken on this same workstation on 2026-10-09**, marked -`MEASURED` or `OPEN` per item: four are done and only need confirming, three are genuinely open, and -the Java suite among them is the one that decides whether the variant path works at all. Verify a -`MEASURED` line rather than skipping it -- a different result there is a finding -- but spend the -time on the `OPEN` ones. - ---- - -You are working in a checkout of `bernardladenthin/java-llama.cpp` on the branch -`claude/hopeful-pascal-9jlbqb`. Read `CLAUDE.md` first (it is long; the sections "Natives jars", -"CPU variants", "Building the native library for local Java tests" and "Testing" matter here). - -## What the branch contains (do not redo it) - -| Item | State | Where | -|---|---|---| -| **A1** llama.cpp b11529 → b11534 | done, 3 commits (b11530, b11531, b11534); 603/603 C++ tests and the model-free Java suite are green on Linux x86-64 | `llama/CMakeLists.txt`, `docs/history/llama-cpp-breaking-changes.md` | -| **A2** floor on *executed* tests | done: `.github/verify-test-counts.sh --min-executed 1800` in every `test-java-*` job | `.github/verify-test-counts.sh`, `java-tests.yml`, `publish.yml` | -| **A3** loader lines to stderr | done: `[jllama] using native backend '…'` and `[jllama] extracted …` print to `System.err` | `LlamaLoader.java` | -| **A4** one-file JBang chat | done: `examples/jbang/Chat.java` names the classes jar + the 7 desktop CPU natives jars (JBang treats a `pom` dependency as a BOM, so `llama-platform` cannot be used there); `check-natives.py` guards the lines | `examples/jbang/Chat.java`, README "Try it with JBang" | - -The sandbox the branch was written in has no models (HuggingFace is blocked), no Windows and no GPU. -Everything below is what only your machine can show. - -## Deliverables, in this order - -1. **Toolchain inventory** (once): clang versions and paths (LLVM on `PATH`, the one inside Visual - Studio under `VC\Tools\Llvm\x64`), Visual Studio and MSVC toolset versions, Windows SDK, Ninja, - sccache, JDK(s), Docker images you have pulled, CUDA toolkit and driver version, Vulkan SDK, - Android SDK/NDK. (There is no Mac and no Android device; macOS and Android are verified through - CI runners and the CI emulator only, so do not plan work that needs either.) - -2. **A1 -- the Java suite with models on Windows.** Build the Ninja CPU library at b11534 the way CI - does (`mvn -q compile` in `llama/`, then `.github\build.bat -G "Ninja Multi-Config" -DOS_NAME=Windows - -DOS_ARCH=x86_64 -DBUILD_TESTING=ON`, `ctest --test-dir build -C Release`), download the CI model set - (`.github/models.csv`, into `models/` at the repo root) and run `mvn -f llama/pom.xml test`. - Report: ctest result, Surefire's final `Tests run: N, Failures, Errors, Skipped` line, and the - names of the skipped tests (`grep -l '.dll` (`x64`, `sse42`, `sandybridge`, `ivybridge`, - `piledriver`, `haswell`, `skylakex`, `cannonlake`, `cascadelake`, `icelake`, `cooperlake`, - `zen4`, `alderlake`, `sapphirerapids`) and `ggml-rpc.dll`, nothing in a `Release/` - subdirectory, and `jllama-files.txt` naming **15** of them -- the modules plus `ggml-rpc`. - **`jllama-extras.txt` is absent, and that is correct, not a defect:** CMake never writes it; - `merge-native-artifacts.sh` derives it in CI from everything in the directory that - `jllama-files.txt` does *not* name, which on Windows is exactly `ggml.dll` and `ggml-base.dll` - (they must be pre-loaded by full path -- there is no `$ORIGIN` on Windows). - - **MEASURED -- `python .github/verify-native-deps.py llama/src/main/natives`.** - "18 native libraries checked, 0 violations". Paste yours anyway; it is one line and it covers - the hybrid-CRT guard in both directions. - - **OPEN and the important one -- the Java suite** (`mvn -f llama/pom.xml test`) against the - variant build, with the models in place. CLAUDE.md's warning applies directly here: a - successful load proves little, because in the earlier Windows measurement *every* crash came - after a load that reported the right device count. So report the Surefire summary line, and - whether `RpcIntegrationTest` and the model-backed classes pass against a build whose CPU - backend is a loadable module rather than a linked library. - - **OPEN -- which variant ggml picks on the 5800H.** Zen 3 has AVX2 and no AVX-512, and - `alderlake` needs AVX-VNNI it does not have, so `haswell` is the expectation. Caveat before you - spend time on the log: CLAUDE.md records that ggml logs module decisions at `GGML_LOG_DEBUG` - only, so `-lv 4` may not name the chosen one. If it does not, list the DLLs the JVM actually - has mapped (Process Explorer, `listdlls`, or `tasklist /m`) instead of guessing. - - **OPEN -- pp512 and tg128** for Qwen3-0.6B Q4_K_M at 8 threads, variant build versus the single - static CPU build. For orientation, measured with `llama-bench` on this machine at - `GGML_NATIVE=ON` (so a single kernel, not the variant selection): clang static reached pp512 - 391.6 and tg128 82.3 t/s with `GGML_OPENMP=OFF`. A variant build landing near that means the - module indirection costs nothing; well below it is a finding. - -## How to work on the branch - -- Pull before every push (`git pull --rebase=false origin claude/hopeful-pascal-9jlbqb`); never force-push, - never rewrite the remote session's commits. Commit only files you own: your reports go to - `docs/handover/local-agent-report-.md` (plain text, numbers in tables, command lines as - code blocks); do not edit source files on this branch unless a step above says so -- propose the - change in the report instead. End your commits with your own `Co-Authored-By` / session trailers, - not the remote session's. -- Before pushing anything that touches `.java`, `.cpp`/`.hpp` or `.github/`: `mvn -f llama/pom.xml - spotless:apply`, `clang-format` 23.1.3 on the C++ files, and the Python checks - (`python .github/check-natives.py`, `check-patches.py`, `check-release-gate.py`, - `check-run-scripts.py`, `check-shared-files.py`, `python -m unittest discover -s - .github/buildcheck/tests -t .github`). -- Do not dispatch or re-run CI workflows; the owner does that. -- Measurements are only useful as text: paste the console lines, not a description of them. diff --git a/docs/local-test-plan.md b/docs/local-test-plan.md index 16b41a0c7..8a42fbe1c 100644 --- a/docs/local-test-plan.md +++ b/docs/local-test-plan.md @@ -38,9 +38,11 @@ servers answer. What it does **not** mean is covered below. 2. **The CI model set** in `models/` at the repository root, by the exact filenames of `.github/models.csv`. Five of the eleven cannot be substituted by another local model because the test needs a *capability*, not a model: the reranker, the embedding model, the vision pair, the - TTS pair, and `stories260K` (which must be **F32** for the trainer). And `MODEL_PATH`, - `DRAFT_MODEL_PATH`, `REASONING_MODEL_PATH` and `RERANKING_MODEL_PATH` are the four paths with - **no** `-D` override (every other model has one), so for those the filename is the only way in. + TTS pair, and `stories260K` (which must be **F32** for the trainer). Every model path has a `-D` + override (README "System Properties Reference"; `MODEL_PATH`, `DRAFT_MODEL_PATH`, + `REASONING_MODEL_PATH` and `RERANKING_MODEL_PATH` got theirs -- `net.ladenthin.llama.text.model`, + `.draft.model`, `.reasoning.model`, `.rerank.model` -- after the first run of this plan), so a + model that is already on the machine can stand in where the test needs no particular capability. 3. **The toolchain** of the thing being tested: MSVC + Ninja for a Windows CPU build, plain clang for a variants build (see `CLAUDE.md` "CPU variants"), the vendor runtime for a GPU build. diff --git a/llama/src/test/java/net/ladenthin/llama/TestConstants.java b/llama/src/test/java/net/ladenthin/llama/TestConstants.java index 98f162e02..52b888b5f 100644 --- a/llama/src/test/java/net/ladenthin/llama/TestConstants.java +++ b/llama/src/test/java/net/ladenthin/llama/TestConstants.java @@ -75,17 +75,57 @@ public static String resolveModelProperty(String key) { public static final int DEFAULT_TEST_NGL = 43; + /** + * System property overriding the main text-generation model, the one most model-backed tests load + * ({@link #DEFAULT_MODEL_PATH} when unset). Any instruct-capable GGUF works for most of them; a few + * assertions are tuned to the CI model and may need reading with another one. + */ + public static final String PROP_MODEL_PATH = LlamaSystemProperties.PREFIX + ".text.model"; + + /** Path used when {@link #PROP_MODEL_PATH} is unset: the CI model (.github/models.csv). */ + public static final String DEFAULT_MODEL_PATH = resolveModelPath("models/codellama-7b.Q2_K.gguf"); + /** Path to the main text generation model used in tests. */ - public static final String MODEL_PATH = resolveModelPath("models/codellama-7b.Q2_K.gguf"); + public static final String MODEL_PATH = resolveModelProperty(PROP_MODEL_PATH, DEFAULT_MODEL_PATH); + + /** + * System property overriding the small draft model used for speculative decoding and by the tests + * that only need a fast, small model ({@link #DEFAULT_DRAFT_MODEL_PATH} when unset). + */ + public static final String PROP_DRAFT_MODEL_PATH = LlamaSystemProperties.PREFIX + ".draft.model"; + + /** Path used when {@link #PROP_DRAFT_MODEL_PATH} is unset: the CI model (.github/models.csv). */ + public static final String DEFAULT_DRAFT_MODEL_PATH = resolveModelPath("models/AMD-Llama-135m-code.Q2_K.gguf"); /** Path to the draft model used for speculative decoding tests. */ - public static final String DRAFT_MODEL_PATH = resolveModelPath("models/AMD-Llama-135m-code.Q2_K.gguf"); + public static final String DRAFT_MODEL_PATH = resolveModelProperty(PROP_DRAFT_MODEL_PATH, DEFAULT_DRAFT_MODEL_PATH); + + /** + * System property overriding the thinking model used for the reasoning-budget tests + * ({@link #DEFAULT_REASONING_MODEL_PATH} when unset); it must emit a thinking block. + */ + public static final String PROP_REASONING_MODEL_PATH = LlamaSystemProperties.PREFIX + ".reasoning.model"; + + /** Path used when {@link #PROP_REASONING_MODEL_PATH} is unset: the CI model (.github/models.csv). */ + public static final String DEFAULT_REASONING_MODEL_PATH = resolveModelPath("models/Qwen3-0.6B-Q4_K_M.gguf"); /** Path to the Qwen3 thinking model used for reasoning budget tests. */ - public static final String REASONING_MODEL_PATH = resolveModelPath("models/Qwen3-0.6B-Q4_K_M.gguf"); + public static final String REASONING_MODEL_PATH = + resolveModelProperty(PROP_REASONING_MODEL_PATH, DEFAULT_REASONING_MODEL_PATH); + + /** + * System property overriding the reranking model ({@link #DEFAULT_RERANKING_MODEL_PATH} when + * unset); it must be a reranker, i.e. load with {@code enableReranking()}. + */ + public static final String PROP_RERANKING_MODEL_PATH = LlamaSystemProperties.PREFIX + ".rerank.model"; + + /** Path used when {@link #PROP_RERANKING_MODEL_PATH} is unset: the CI model (.github/models.csv). */ + public static final String DEFAULT_RERANKING_MODEL_PATH = + resolveModelPath("models/jina-reranker-v1-tiny-en-Q4_0.gguf"); /** Path to the reranking model used in tests (loaded with {@code enableReranking()}). */ - public static final String RERANKING_MODEL_PATH = resolveModelPath("models/jina-reranker-v1-tiny-en-Q4_0.gguf"); + public static final String RERANKING_MODEL_PATH = + resolveModelProperty(PROP_RERANKING_MODEL_PATH, DEFAULT_RERANKING_MODEL_PATH); /** System property overriding the GGUF used by the real tool-calling integration tests. */ public static final String PROP_TOOL_MODEL_PATH = LlamaSystemProperties.PREFIX + ".tool.model"; diff --git a/llama/src/test/java/net/ladenthin/llama/TestConstantsTest.java b/llama/src/test/java/net/ladenthin/llama/TestConstantsTest.java index b91d919fb..9f4aba191 100644 --- a/llama/src/test/java/net/ladenthin/llama/TestConstantsTest.java +++ b/llama/src/test/java/net/ladenthin/llama/TestConstantsTest.java @@ -86,13 +86,18 @@ public void propertyLookupResolvesAndHonoursTheDefault() { * that no constant names is downloaded for nothing, and a constant naming a model the list lacks * makes its test self-skip in CI -- green, and silently. Both directions are asserted, which is * also why CI passes no {@code -Dnet.ladenthin.llama.*} model property: the defaults are the CI set. + * + *

Only the {@code DEFAULT_*} constants count: every shipped model path has a {@code -D} override + * now, so the resolved constants ({@code MODEL_PATH}, ...) may name whatever a developer passed. */ @Test public void theModelDefaultsAreExactlyTheCiModelSet() throws Exception { java.util.Set defaults = new java.util.TreeSet<>(); for (java.lang.reflect.Field field : TestConstants.class.getFields()) { Object value = field.get(null); - if (value instanceof String && ((String) value).endsWith(".gguf")) { + if (field.getName().startsWith("DEFAULT_") + && value instanceof String + && ((String) value).endsWith(".gguf")) { defaults.add(Paths.get((String) value).getFileName().toString()); } } @@ -112,14 +117,34 @@ public void theShippedModelConstantsGoThroughTheResolver() { // Guards the wiring itself: if a future edit drops the resolveModelPath(...) wrapper from a // constant, that constant stops matching its resolved literal and every test gated on it // silently self-skips again in CI. Holds in both layouts — with the GGUF present the - // resolver returns the same absolute path on both sides, without it the same literal. - assertEquals(TestConstants.resolveModelPath("models/codellama-7b.Q2_K.gguf"), TestConstants.MODEL_PATH); + // resolver returns the same absolute path on both sides, without it the same literal. The four + // shipped paths are property-backed since the -D overrides: the default must be the resolved + // literal, and the constant must be the resolved property -- which holds whether or not the + // property is set, so a developer's override cannot red this test. + assertEquals(TestConstants.resolveModelPath("models/codellama-7b.Q2_K.gguf"), TestConstants.DEFAULT_MODEL_PATH); + assertEquals( + TestConstants.resolveModelProperty(TestConstants.PROP_MODEL_PATH, "models/codellama-7b.Q2_K.gguf"), + TestConstants.MODEL_PATH); + assertEquals( + TestConstants.resolveModelPath("models/AMD-Llama-135m-code.Q2_K.gguf"), + TestConstants.DEFAULT_DRAFT_MODEL_PATH); assertEquals( - TestConstants.resolveModelPath("models/AMD-Llama-135m-code.Q2_K.gguf"), TestConstants.DRAFT_MODEL_PATH); + TestConstants.resolveModelProperty( + TestConstants.PROP_DRAFT_MODEL_PATH, "models/AMD-Llama-135m-code.Q2_K.gguf"), + TestConstants.DRAFT_MODEL_PATH); assertEquals( - TestConstants.resolveModelPath("models/Qwen3-0.6B-Q4_K_M.gguf"), TestConstants.REASONING_MODEL_PATH); + TestConstants.resolveModelPath("models/Qwen3-0.6B-Q4_K_M.gguf"), + TestConstants.DEFAULT_REASONING_MODEL_PATH); + assertEquals( + TestConstants.resolveModelProperty( + TestConstants.PROP_REASONING_MODEL_PATH, "models/Qwen3-0.6B-Q4_K_M.gguf"), + TestConstants.REASONING_MODEL_PATH); assertEquals( TestConstants.resolveModelPath("models/jina-reranker-v1-tiny-en-Q4_0.gguf"), + TestConstants.DEFAULT_RERANKING_MODEL_PATH); + assertEquals( + TestConstants.resolveModelProperty( + TestConstants.PROP_RERANKING_MODEL_PATH, "models/jina-reranker-v1-tiny-en-Q4_0.gguf"), TestConstants.RERANKING_MODEL_PATH); assertEquals( TestConstants.resolveModelPath("models/Qwen2.5-1.5B-Instruct-Q4_K_M.gguf"), From c6e69396791102be21a12a57761fcb761cda1fb0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 10:40:53 +0000 Subject: [PATCH 3/5] CI: ship cpu-windows-x86-64 as the CPU-variants build (plain clang), add the clang C++ test job build-windows-x86_64 now builds -DJLLAMA_CPU_VARIANTS=ON with plain clang (-DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++, the command line the local agent verified at b11538: 18 DLLs, verify-native-deps.py clean, the Java suite green against the artifact once jllama-extras.txt exists), the way the two Linux CPU jars ship since 5.2.0: 14 ggml-cpu-.dll modules of which ggml loads the best for the running CPU. Plain clang because only the GNU driver produces all 14 variants (CMake sets MSVC for clang-cl too, and ggml's `if (NOT MSVC)` drops five); the CMakeLists refuses cl.exe and clang-cl. The job drops -DBUILD_TESTING (the option refuses it: jllama_test links the CPU and RPC backends, modules here) and the sccache install (upstream's unconditional `-Xclang -fno-pch-timestamp` makes every clang TU non-cacheable -- 719 of 720 requests, 0.00% hits, measured -- so it only cost time), and prints `clang --version` first, since the LLVM on PATH decides whether patches/0017's compiler half is a warning (clang 20) or the error it fixes (22+). test-cpp-windows-x86_64 runs the C++ suite in a static plain-clang build with the tests (the split test-cpp-linux-x86_64 / -aarch64 have), so Windows x86-64 has the suite under both toolchains -- the two MSVC-classifier jobs keep running it with cl.exe -- and patches/0017's compiler half is reachable in CI at all. It gates both publish jobs through package. test-java-windows-x86_64 is unchanged in substance and now runs inference through the variants artifact (the loader pre-loads ggml-base.dll and ggml.dll from the artifact's jllama-extras.txt). Not verified here: the first run of both Windows jobs (no Windows machine in this session; the variants build itself and the Java suite against it are the agent's measurements, the static clang build with gtest and verify-patches-applied.sh under Git Bash are not measured). CLAUDE.md ("CPU variants", "Where C++ tests run": 6 build+ctest / 5 test-only, "Windows natives"), README's classifier rows, TODO item 1 (done, the measurements stand) and the CHANGELOG follow. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01AytmJF9faEiQEVt6eetQS2 --- .github/workflows/publish.yml | 88 ++++++++++++++++++++++++--------- CHANGELOG.md | 16 ++++++ CLAUDE.md | 91 ++++++++++++++++++++++------------- README.md | 5 +- TODO.md | 34 ++++++++----- 5 files changed, 163 insertions(+), 71 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 3e6edd497..0a552ca5c 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -1431,24 +1431,32 @@ jobs: path: ${{ github.workspace }}/llama/src/main/natives/net/ladenthin/llama/ # --------------------------------------------------------------------------- - # Windows Ninja Multi-Config + sccache — the DEFAULT Windows CPU natives. - # The Visual Studio generator ignores CMAKE_{C,CXX}_COMPILER_LAUNCHER, so only the - # Ninja Multi-Config generator can front cl.exe with sccache over Depot WebDAV - # (build.bat probe-guards it). Both generators use the same MSVC toolchain (cl.exe, - # static /MT CRT) on the same runner, so the produced jllama.dll/llama.dll/ggml.dll - # are functionally equivalent with identical runtime dependencies — the only delta - # is build-system plumbing + caching. The Ninja build therefore ships as the - # cpu-windows-* natives jars; the MSVC build above as msvc-windows-*, for anyone who - # wants the Visual-Studio-generator natives. Upstream llama.cpp also - # builds its Windows artifacts with Ninja Multi-Config + MSVC. + # Windows x86-64 CPU natives (cpu-windows-x86-64, the default): the CPU-variants build, as the + # two Linux CPU jars ship it -- GGML_BACKEND_DL + GGML_CPU_ALL_VARIANTS, one ggml-cpu-.dll + # per instruction-set level (14 on x86-64), of which ggml loads the best for the running CPU at + # start-up (CLAUDE.md "CPU variants"). The compiler is plain clang (the GNU driver, the LLVM on + # the runner image's PATH), not cl.exe and not clang-cl: CMake sets MSVC for clang-cl as for + # cl.exe, and ggml's `if (NOT MSVC)` then drops 5 of the 14 variants, so llama/CMakeLists.txt + # refuses both. It needs the MSVC headers, libraries and linker from the developer environment + # all the same (clang targets the MSVC ABI), and it ships the same hybrid CRT as every Windows + # CPU jar -- the CMakeLists spells it for clang (-fms-runtime-lib=static + the UCRT linker + # flags), and verify-native-deps.py holds the directory to the OS imports. Two things this job + # deliberately lacks: + # * -DBUILD_TESTING: the option refuses it (jllama_test calls the CPU and RPC backends, which + # are modules here, not link-time libraries) -- test-cpp-windows-x86_64 runs the C++ suite in + # a static build with the same compiler, the split the Linux CPU jobs have. + # * sccache: upstream adds `-Xclang -fno-pch-timestamp` to every clang TU (llama.cpp's + # CMakeLists), which sccache 0.18.0 refuses to cache -- measured: every TU a miss, the build + # green and the cache inert -- so the install would only cost time. The cl.exe TUs of the + # other Windows Ninja jobs still cache; this one builds cold on every run. + # The 32-bit job below stays the Ninja + cl.exe static build (upstream builds no x86 variants + # either), and the two MSVC-classifier jobs above keep the Visual Studio generator. # --------------------------------------------------------------------------- build-windows-x86_64: - name: Build and Test Windows 2025 x86_64 (Ninja Multi-Config + sccache, default) + name: Build Windows 2025 x86_64 (CPU variants, clang, default) needs: [startgate, build-webui] runs-on: windows-2025-vs2026 - env: - SCCACHE_WEBDAV_TOKEN: ${{ secrets.DEPOT_TOKEN }} steps: - uses: actions/checkout@v7 - name: Download shared WebUI assets @@ -1460,19 +1468,16 @@ jobs: uses: ilammy/msvc-dev-cmd@v1 with: arch: x64 - - name: Install sccache (shared compiler cache) - if: env.USE_CACHE == 'true' && env.SCCACHE_WEBDAV_TOKEN != '' - continue-on-error: true - uses: ./.github/actions/install-sccache-windows - name: Display CPU Info shell: bash run: bash .github/print-host-info.sh + # `clang --version` first: which LLVM the image put on PATH decides whether patches/0017's + # compiler half is exercised as a warning (clang 20) or as the error it fixes (clang 22+). - name: Build libraries shell: cmd run: | - .github\build.bat -G "Ninja Multi-Config" -DGGML_OPENMP=OFF -DOS_NAME=Windows -DOS_ARCH=x86_64 -DBUILD_TESTING=ON - - name: Run C++ unit tests - run: ctest --test-dir llama/build --output-on-failure + clang --version + .github\build.bat -G "Ninja Multi-Config" -DJLLAMA_CPU_VARIANTS=ON -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DGGML_OPENMP=OFF -DOS_NAME=Windows -DOS_ARCH=x86_64 - name: Upload artifacts uses: actions/upload-artifact@v7 with: @@ -2254,6 +2259,38 @@ jobs: - name: Run C++ unit tests run: ctest --test-dir llama/build --output-on-failure + test-cpp-windows-x86_64: + name: C++ Tests Windows 2025 x86_64 (clang) + needs: startgate + # The C++ suite with the compiler the shipped Windows x86-64 natives are built with (plain + # clang, see build-windows-x86_64), against the default static build: the shipped CPU-variants + # configuration cannot host jllama_test (its CPU and RPC backends are modules), the same split + # as test-cpp-linux-x86_64 / -aarch64. The two MSVC-classifier jobs run the suite with cl.exe, + # so Windows x86-64 has it under both toolchains -- and patches/0017's compiler half (plain + # clang on a windows-msvc target rejecting the typed _mm_prefetch pointer) is reachable only + # here. No WebUI artifact (the test build uses the empty-asset stub) and no sccache (see the + # build job: clang TUs do not cache). + runs-on: windows-2025-vs2026 + steps: + - uses: actions/checkout@v7 + - name: Set up MSVC developer environment (x64) + uses: ilammy/msvc-dev-cmd@v1 + with: + arch: x64 + - name: Display CPU Info + shell: bash + run: bash .github/print-host-info.sh + - name: Build libraries + shell: cmd + run: | + clang --version + .github\build.bat -G "Ninja Multi-Config" -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DGGML_OPENMP=OFF -DOS_NAME=Windows -DOS_ARCH=x86_64 -DBUILD_TESTING=ON + - name: Verify llama.cpp patches are applied + shell: bash + run: .github/verify-patches-applied.sh + - name: Run C++ unit tests + run: ctest --test-dir llama/build --output-on-failure + build-macos-arm64-metal-15: name: Build and Test macOS 15 arm64 (Metal) needs: [startgate, build-webui] @@ -2434,8 +2471,12 @@ jobs: natives-artifact: natives-metal-macos-aarch64 error-artifact: error-log-macos-15-metal + # The model-backed suite against the shipped CPU-variants artifact (jllama.dll + ggml.dll + + # ggml-base.dll + the 14 CPU modules): the loader reads the artifact's jllama-extras.txt / + # jllama-files.txt from the test classpath, pre-loads the two ggml libraries and lets ggml pick + # the CPU module -- a load alone proves little (CLAUDE.md "CPU variants"), this runs inference. test-java-windows-x86_64: - name: Java Tests Windows 2025 x86_64 (default / Ninja) + name: Java Tests Windows 2025 x86_64 (default / CPU variants, clang) needs: [build-windows-x86_64, verify-model-cache] uses: ./.github/workflows/java-tests.yml with: @@ -2444,8 +2485,8 @@ jobs: error-artifact: windows-output # Java/inference validation of the MSVC-built x86_64 DLL (the analogue of - # test-java-windows-x86_64 for the default Ninja build). Loads the MSVC jllama.dll - # via JNI and runs the full model-backed suite, so both Windows generators are + # test-java-windows-x86_64 for the default CPU-variants build). Loads the MSVC jllama.dll + # via JNI and runs the full model-backed suite, so both Windows x86-64 CPU toolchains are # validated end-to-end before the `msvc-windows` natives jar ships. test-java-windows-x86_64-msvc: name: Java Tests Windows 2025 x86_64 (MSVC classifier) @@ -2490,6 +2531,7 @@ jobs: - build-windows-x86_64-openvino - test-cpp-linux-x86_64 - test-cpp-linux-aarch64 + - test-cpp-windows-x86_64 - build-macos-arm64-metal-15 - test-java-linux-x86_64 - test-java-macos-arm64-metal diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d5c4549a..9590872d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ from version 5.0.0 onward. Pre-fork releases (`1.x`–`4.2.0`) were authored by ## [Unreleased] ### Added +- **`test-cpp-windows-x86_64`**: the C++ suite on Windows x86-64 with plain clang (a static build with + the tests), the compiler the shipped `cpu-windows-x86-64` natives are built with now; the two MSVC + jobs keep running it with `cl.exe`. Gates both publish jobs like every other job. - **`-D` overrides for the four shipped test models.** `net.ladenthin.llama.text.model`, `.draft.model`, `.reasoning.model` and `.rerank.model` override `TestConstants.MODEL_PATH`, `DRAFT_MODEL_PATH`, `REASONING_MODEL_PATH` and `RERANKING_MODEL_PATH` the way every capability-specific model already had @@ -77,6 +80,19 @@ from version 5.0.0 onward. Pre-fork releases (`1.x`–`4.2.0`) were authored by core's; `check-natives.py` fails when they differ. ### Changed +- **`cpu-windows-x86-64` is the CPU-variants build, built with clang** (`build-windows-x86_64`, + `-DJLLAMA_CPU_VARIANTS=ON`): like the two Linux CPU jars since 5.2.0, the directory holds + `jllama.dll`, `ggml.dll`, `ggml-base.dll`, `ggml-rpc.dll` and one `ggml-cpu-.dll` per + instruction-set level (14), of which ggml loads the best for the running CPU -- an AVX-512 / AVX-VNNI + / AMX machine uses its kernels and a CPU without AVX2 works, where the one static library was built + for one level. Plain clang because only the GNU driver yields all 14 variants (`clang-cl` and + `cl.exe` lose five, CLAUDE.md "CPU variants"); same hybrid CRT, no new runtime dependency + (`verify-native-deps.py` holds the directory to the OS imports). `LlamaLoader` pre-loads + `ggml-base.dll` and `ggml.dll` by full path from `jllama-extras.txt` before `jllama.dll`, so the + import never resolves against a foreign llama.cpp install on `PATH`. The job no longer installs + sccache (upstream's `-Xclang -fno-pch-timestamp` makes every clang TU a cache miss) and no longer + runs `ctest` (the option refuses `BUILD_TESTING`; see `test-cpp-windows-x86_64` under Added). The + 32-bit `cpu-windows-x86` jar and the `msvc-windows-*` jars are unchanged. - **The message of a rejected request is the bare reason** (`"prompt" must not be empty`), no longer the JSON object `{"code":400,"message":…,"type":"invalid_request_error"}` the old `throw_invalid_request` wrapped it in: the type is the exception class now (`InvalidRequestException`), as it is for every diff --git a/CLAUDE.md b/CLAUDE.md index 053554175..c979089f9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -86,12 +86,21 @@ was **one** library for one level -- x86-64 the Haswell baseline (x86-64-v3, the crash and not an exception, at the first matrix multiplication, and AVX-512/VNNI/AMX stayed unused on CPUs that have them), aarch64 plain ARMv8. `-DJLLAMA_CPU_VARIANTS=ON` builds it; the two Linux build jobs pass it, every other platform still ships the single static library, and the option -refuses them. **Windows x86-64 is buildable this way since 5.2.x and not yet wired into CI** -(`TODO.md`): the option accepts it, refuses `cl.exe`/`clang-cl` loudly -- ggml's `if (NOT MSVC)` -block drops 5 of the 14 variants and `clang-cl` cannot build `alderlake` at all -- and the CMake -side is verified locally (18 DLLs in one directory, `verify-native-deps.py` clean). What is -missing is the CI job, which needs the ctest split the Linux jobs have (see "Where C++ tests run" -below): +refuses them. **Windows x86-64 followed in 5.2.x** (`cpu-windows-x86-64`, job `build-windows-x86_64`): +the same 18 files as DLLs, built with **plain clang** -- the option refuses `cl.exe`/`clang-cl` +loudly, because ggml's `if (NOT MSVC)` block drops 5 of the 14 variants for both and `clang-cl` +cannot build `alderlake` at all -- with the ctest split the Linux jobs have +(`test-cpp-windows-x86_64`, a static clang build; see "Where C++ tests run" below) and the Java +suite plus the fat-jar smoke running inference through the artifact. Two Windows specifics: there +is no `$ORIGIN`, so the two libraries `jllama.dll` links (`ggml-base.dll`, `ggml.dll`) are left out +of `jllama-files.txt` and named in `jllama-extras.txt` instead, which `LlamaLoader` loads by full +path **before** `jllama.dll` (otherwise Windows resolves the import from `java.exe`'s directory or +`PATH`, where a foreign llama.cpp install can answer); CMake writes that file itself since 5.2.x, +byte-identical to what `merge-native-artifacts.sh` derives, so a local build is loadable without +the merge step (before, 57 Java tests failed on the load). And sccache is off on that job: upstream +adds `-Xclang -fno-pch-timestamp` to every clang TU, which sccache 0.18.0 does not cache (measured: +every TU a miss, build green) -- the job builds cold. 32-bit Windows and Windows arm64 ship no +variants (upstream builds none either). - **Only ggml becomes shared.** `FetchContent_MakeAvailable` is pointed at a non-existent `SOURCE_SUBDIR`, so it only fetches and patches; ggml is then added first with @@ -131,11 +140,12 @@ below): meet at **2.28**: x86-64 rises from 2.17, aarch64 falls from ~2.39. `verify-native-deps.py` reads every library's version-needs table and holds both directories (and CUDA) to `GLIBC_2.28`; the modules themselves reference `GLIBC_2.27` at most (measured in the image). -- **No `ctest` in the two build jobs**: `jllama_test` calls the CPU/RPC backends directly, so the - option refuses `BUILD_TESTING`; `test-cpp-linux-x86_64` and `test-cpp-linux-aarch64` build the - default static library with the tests. The Java suite (`test-java-linux-x86_64`, the langchain4j - integration job through `-Dnet.ladenthin.llama.lib.path`, the fat-jar smokes) runs real inference - through the variant build -- a load alone proves little: in the Windows measurement every crash +- **No `ctest` in the three build jobs**: `jllama_test` calls the CPU/RPC backends directly, so the + option refuses `BUILD_TESTING`; `test-cpp-linux-x86_64`, `test-cpp-linux-aarch64` and + `test-cpp-windows-x86_64` build the default static library with the tests, each with the compiler + of the shipped build. The Java suite (`test-java-linux-x86_64`, `test-java-windows-x86_64`, the + langchain4j integration job through `-Dnet.ladenthin.llama.lib.path`, the fat-jar smokes) runs real + inference through the variant build -- a load alone proves little: in the Windows measurement every crash came after a successful load with a correct device count. - **The natives-jar checks know directories with many files**: `merge-native-artifacts.sh` requires every file `jllama-files.txt` names and keeps them out of `jllama-extras.txt`; `package-fatjars.sh` @@ -238,7 +248,7 @@ repository name (`?repo` entries, see `sharedfiles.py`), `SUPPORT.md` and `ISSUE **Composite actions (`.github/actions/`, this repo only).** `restore-models` (restore the shared GGUF cache + `validate-models.sh`; the one place a job gets its models — 16 call sites) and -`install-sccache-windows` (10 call sites; the caller keeps `if:` and `continue-on-error`), +`install-sccache-windows` (9 call sites; the caller keeps `if:` and `continue-on-error`), `build-core` (parent + `llama`, optionally more modules, built without tests/checks/javadoc/signing, `install` or `package` -- the one copy of that skip-flag list, 9 call sites) and `publish-cpu-aar-local` (download + stage the Android CPU natives, publish the CPU AAR to mavenLocal; @@ -276,11 +286,12 @@ new job:** > specific to a configuration the tests cannot run in are covered by the layers that *can* see > them -- the packaging check and the Java suite against the real artifact.** -Today that means 7 jobs build an artifact **and** run `ctest` (`build-linux-s390x`, +Today that means 6 jobs build an artifact **and** run `ctest` (`build-linux-s390x`, `build-macos-arm64-metal-15`, `build-windows-arm64`, `build-windows-x86`, `build-windows-x86-msvc`, -`build-windows-x86_64`, `build-windows-x86_64-msvc`) and 4 only test -(`test-cpp-linux-x86_64`, `test-cpp-linux-aarch64` -- forced, see below -- plus -`build-macos-arm64-metal` and `build-macos-arm64-no-metal`, whose artifacts are not shipped at all). +`build-windows-x86_64-msvc`) and 5 only test +(`test-cpp-linux-x86_64`, `test-cpp-linux-aarch64`, `test-cpp-windows-x86_64` -- forced, see below +-- plus `build-macos-arm64-metal` and `build-macos-arm64-no-metal`, whose artifacts are not shipped +at all). **Why riding along is safe, measured rather than assumed:** `-DBUILD_TESTING=ON` does not change the shipped library. Compared on a Windows MSVC configure with the flag off and on, the `jllama` @@ -485,15 +496,21 @@ At runtime the device must provide its own OpenCL ICD (`libOpenCL.so`); Qualcomm Adreno drivers do. Devices without an ICD should use the CPU-only `llama-android` AAR. -## Windows natives (Ninja CPU + MSVC + CUDA/Vulkan/OpenCL GPU) - -The Windows x86-64 natives ship in **five** forms (natives jars): the CPU build with the -**`Ninja Multi-Config`** generator (`cpu-windows-x86-64` / `cpu-windows-x86`, the ones in -`llama-platform`), the Visual Studio / MSVC build (`msvc-windows-x86-64` / `msvc-windows-x86`), and -three GPU backends, **`cuda13-windows-x86-64`**, **`vulkan-windows-x86-64`** and -**`opencl-windows-x86-64`** (all Ninja). - -**Why Ninja is the default (the flip).** The Visual Studio generator ignores +## Windows natives (clang CPU variants + Ninja/MSVC CPU + CUDA/Vulkan/OpenCL GPU) + +The Windows x86-64 natives ship in **five** forms (natives jars): the CPU build (`cpu-windows-x86-64`, +in `llama-platform`: since 5.2.x the **CPU-variants build with plain clang**, Ninja Multi-Config, +see "CPU variants"; its 32-bit sibling `cpu-windows-x86` stays the Ninja + `cl.exe` static build), +the Visual Studio / MSVC build (`msvc-windows-x86-64` / `msvc-windows-x86`), and three GPU backends, +**`cuda13-windows-x86-64`**, **`vulkan-windows-x86-64`** and **`opencl-windows-x86-64`** (all Ninja + +`cl.exe`). + +**Why Ninja became the default (the flip), and why x86-64 then moved on to clang.** The Ninja + +`cl.exe` build replaced the Visual Studio one as `cpu` first, for the reason below; the x86-64 `cpu` +jar then became the clang CPU-variants build in 5.2.x, because only plain clang produces all 14 +variants (TODO.md "CPU variants", measured: clang is +5.6% on prompt processing and equal on +generation against `cl.exe`, static against static). MSVC stays as `msvc-windows-*`, the second, +independent toolchain. The Visual Studio generator ignores `CMAKE_{C,CXX}_COMPILER_LAUNCHER`, so only Ninja Multi-Config can front `cl.exe` with sccache over Depot WebDAV. **Both generators use the same MSVC toolchain** (`cl.exe`, hybrid CRT via `CMAKE_MSVC_RUNTIME_LIBRARY` + the UCRT linker flags, same Release flags, same runner), so the produced @@ -501,8 +518,9 @@ Depot WebDAV. **Both generators use the same MSVC toolchain** (`cl.exe`, hybrid dependencies** — the only difference is build-system plumbing + caching. Making Ninja the `cpu` build gives the most-pulled natives the sccache cache; MSVC stays available (directory `msvc`, which the loader tries before `cpu`) for anyone who wants the Visual-Studio-generator build. (Upstream llama.cpp also builds its Windows artifacts with -Ninja Multi-Config + MSVC.) Both Windows CPU builds are validated end-to-end with the full -model-backed Java suite (`test-java-windows-x86_64` = Ninja, `test-java-windows-x86_64-msvc` = MSVC). +Ninja Multi-Config + MSVC.) Both Windows x86-64 CPU builds are validated end-to-end with the full +model-backed Java suite (`test-java-windows-x86_64` = clang variants, `test-java-windows-x86_64-msvc` += MSVC). **`GGML_OPENMP=OFF` on every Windows CPU job -- it is faster, not a concession.** All four x86-64/x86 CPU jobs (both generators) pass it, as the arm64 jobs already did for a dependency reason. Measured @@ -564,7 +582,10 @@ the Visual Studio generator build `msvc`, the Ninja one `cpu`): (the Windows analogue of `build_opencl_android.sh`). 3. **`.github/workflows/publish.yml`** — build jobs (all `windows-2025-vs2026`, `ilammy/msvc-dev-cmd@v1`, sccache v0.18.0 zip + Depot WebDAV), each uploading `natives-`: - - `build-windows-x86_64` / `build-windows-x86` — **Ninja CPU**. + - `build-windows-x86_64` — **CPU variants, plain clang** (`-DJLLAMA_CPU_VARIANTS=ON + -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++`; no `-DBUILD_TESTING`, no sccache + install -- `test-cpp-windows-x86_64` builds the static library with the tests, same compiler). + `build-windows-x86` — **Ninja + `cl.exe` CPU**. - `build-windows-x86_64-msvc` / `build-windows-x86-msvc` — **MSVC CPU**. - `build-windows-x86_64-cuda` — CUDA `13.4` assembled from NVIDIA's redist archives (upstream's `windows-setup-cuda` component list; `Jimver/cuda-toolkit` stops at 13.3.1) + `-DGGML_CUDA=ON`. @@ -580,11 +601,15 @@ way CMake's `FindVulkan` couldn't read → switched to `jakoch/install-vulkan-sd FindVulkan-compatible). Because all five Windows build jobs are in the `package`/publish `needs:` graph, a GPU-toolchain failure blocks packaging — the same release-gating policy every build job follows. -**sccache on every Windows Ninja job.** All ten Ninja build jobs install sccache (x86_64 or the -native `aarch64` release) through the composite action `.github/actions/install-sccache-windows` -(arch from `RUNNER_ARCH`; its version must equal `SCCACHE_DL_VERSION` in `build.sh`) with the same -`USE_CACHE` / `SCCACHE_WEBDAV_*` env; only the two MSVC-classifier -jobs cannot, because the Visual Studio generator ignores compiler launchers. It cannot red a build, by +**sccache on every Windows Ninja job but one.** Nine of the ten Ninja build jobs install sccache +(x86_64 or the native `aarch64` release) through the composite action +`.github/actions/install-sccache-windows` (arch from `RUNNER_ARCH`; its version must equal +`SCCACHE_DL_VERSION` in `build.sh`) with the same `USE_CACHE` / `SCCACHE_WEBDAV_*` env; the two +MSVC-classifier jobs cannot, because the Visual Studio generator ignores compiler launchers, and +`build-windows-x86_64` (plain clang, CPU variants) does not, because its TUs do not cache: upstream +passes `-Xclang -fno-pch-timestamp` to every clang TU and sccache 0.18.0 refuses that flag +(measured at b11538: 719 of 720 compile requests non-cacheable, `Can't handle UnknownFlag arguments with -Xclang`, 0.00% hits, build green). Re-test on an sccache bump, and if it +caches, add the install step back. It cannot red a build, by three guards in `build.bat`: the install step is `continue-on-error`; a probe compiles through sccache before it is trusted; and — because that probe only proves `cl.exe`, while arm64 builds with `clang-cl`, ROCm with its own `clang` and SYCL with `icx` — **a configure or build that fails with sccache as the diff --git a/README.md b/README.md index 90526eb7d..2a45de400 100644 --- a/README.md +++ b/README.md @@ -240,11 +240,12 @@ platforms you target, e.g. `cpu-linux-x86-64`. | `cpu-linux-x86-64` | CPU | Linux x86-64 | A JDK 8+ JVM; glibc ≥ 2.28 (manylinux_2_28: RHEL 8, Ubuntu 20.04, Debian 10 and later). Ships one CPU backend module per instruction-set level (x86-64 baseline, SSE4.2, AVX, AVX2, AVX-512, AVX-VNNI, AMX), of which ggml loads the best for the running CPU at start-up — a CPU without AVX2 works, and an AVX-512/AMX machine uses its kernels. | | `cpu-linux-aarch64` | CPU | Linux aarch64 | glibc ≥ 2.28 (RHEL 8, Ubuntu 20.04, Debian 10, Amazon Linux 2023 and later) — built in the manylinux_2_28 image on an arm64 runner. Ships one CPU backend module per ARM feature level (armv8.0 up to armv9.2: dotprod, fp16, SVE, i8mm, SVE2, SME), chosen at start-up like the x86-64 ones. | | `cpu-linux-s390x` | CPU | Linux s390x (IBM Z, big-endian) | A JDK 8+ JVM. | -| `cpu-windows-x86-64` / `cpu-windows-x86` | CPU | Windows x86-64 / x86 | A JDK 8+ JVM; **Windows 10 or newer**. Built with Ninja Multi-Config + MSVC, hybrid CRT: the STL and vcruntime are static (no `msvcp140.dll` / `vcruntime140.dll`, no VC++ redistributable), the Universal CRT is the OS one (`ucrtbase.dll`), so Microsoft's UCRT security updates apply. | +| `cpu-windows-x86-64` | CPU | Windows x86-64 | A JDK 8+ JVM; **Windows 10 or newer**. Ships one CPU backend module per instruction-set level (as the Linux x86-64 jar: x86-64 baseline, SSE4.2, AVX, AVX2, AVX-512, AVX-VNNI, AMX), of which ggml loads the best for the running CPU at start-up. Built with clang, hybrid CRT: the STL and vcruntime are static (no `msvcp140.dll` / `vcruntime140.dll`, no VC++ redistributable), the Universal CRT is the OS one (`ucrtbase.dll`), so Microsoft's UCRT security updates apply. | +| `cpu-windows-x86` | CPU | Windows x86 (32-bit JVM) | A JDK 8+ JVM; **Windows 10 or newer**. Built with Ninja Multi-Config + MSVC, same hybrid CRT. | | `cpu-windows-aarch64` | CPU | Windows on ARM (Snapdragon X / Surface) | A JDK 8+ JVM; **Windows 10 or newer**. Built natively on `windows-11-arm` with `clang-cl`, same hybrid CRT as the x86 jars. | | `metal-macos-aarch64` | Metal + CPU | macOS aarch64 (Apple silicon) | A JDK 8+ JVM. | | `cpu-android-aarch64` / `cpu-android-x86-64` | CPU | Android | For Android use the [`llama-android` AAR](#importing-in-android); these jars are the same libraries for other Android JVM setups. | -| `msvc-windows-x86-64` / `msvc-windows-x86` | CPU (Visual Studio generator) | Windows x86-64 / x86 | Same CPU backend and MSVC toolchain as `cpu-windows-*`, built with the Visual Studio generator instead of Ninja — an alternate-toolchain option; tried before the `cpu` jar when both are present. **Windows 10 or newer**, like every Windows jar. | +| `msvc-windows-x86-64` / `msvc-windows-x86` | CPU (Visual Studio generator) | Windows x86-64 / x86 | One static library built with the Visual Studio generator and MSVC (`cl.exe`), same hybrid CRT — the second, independent toolchain next to the clang-built `cpu-windows-x86-64`; tried before the `cpu` jar when both are present. **Windows 10 or newer**, like every Windows jar. | | `cuda13-linux-x86-64` | CUDA 13 | Linux x86-64 with NVIDIA GPU | NVIDIA driver + CUDA 13 runtime libraries (`libcudart.so.13`, `libcublas.so.13`). | | `cuda13-windows-x86-64` | CUDA 13 | Windows x86-64 with NVIDIA GPU | NVIDIA driver + CUDA 13 Toolkit (`cudart64_13.dll`, `cublas64_13.dll`, `cublasLt64_13.dll` on `PATH`). | | `vulkan-linux-x86-64` | Vulkan | Linux x86-64 with a Vulkan 1.2+ GPU (NVIDIA / AMD / Intel) | A Vulkan runtime (`libvulkan.so.1`), which current GPU drivers install. The most portable Linux GPU option. glibc ≈ 2.39 (built on `ubuntu-latest`). | diff --git a/TODO.md b/TODO.md index b7c249b31..198ec78ae 100644 --- a/TODO.md +++ b/TODO.md @@ -19,16 +19,22 @@ so everything below is genuinely still open. ### CPU variants (`JLLAMA_CPU_VARIANTS`) -- Windows, GPU modules, loader follow-ups -Linux x86-64 and aarch64 ship the variant build since 5.2.0 (CLAUDE.md "CPU variants"). The rest, -with what a measurement on a Windows 11 machine (Ryzen 7 5800H, RTX 3070, JDK 21, upstream b11476 -binaries; 2026-10-08) established. The local agent's follow-up measurements at b11534 on the same -machine, through this project's own CMake (plain clang 23.1.3 builds all 14 variants, 18 DLLs, -`verify-native-deps.py` 0 violations) and the four items still `OPEN` for the CI job -- the -`build.bat` command line, the Java suite against the variant build, which variant ggml picks on -Zen 3, pp512/tg128 against the static build -- are in `docs/handover/local-agent-pr-a.md`, -deliverable 6: - -1. **Windows x86-64 build.** Only plain `clang`/`clang++` with the GNU driver -- upstream's +Linux x86-64 and aarch64 ship the variant build since 5.2.0, Windows x86-64 since 5.2.x +(CLAUDE.md "CPU variants"). The rest, with what a measurement on a Windows 11 machine (Ryzen 7 +5800H, RTX 3070, JDK 21, upstream b11476 binaries; 2026-10-08) established. The local agent's +follow-up measurements at b11534/b11538 on the same machine, through this project's own CMake +(plain clang 23.1.3 builds all 14 variants, 18 DLLs, `verify-native-deps.py` 0 violations; the +`build.bat` command line the CI job now runs, the Java suite green against the variant build once +`jllama-extras.txt` exists, `haswell` picked on Zen 3, pp512/tg128 against the static build) are in +`docs/handover/local-agent-report-b11538-windows.md`, deliverable 3 (the GPU measurements and the +module-beside-the-CPU-set experiment in its addenda 2 and 3): + +1. **Windows x86-64 build -- DONE in 5.2.x**: `build-windows-x86_64` builds it with plain clang + (no sccache: the clang TUs do not cache, see CLAUDE.md "Windows natives"), + `test-cpp-windows-x86_64` runs the C++ suite in a static clang build, `test-java-windows-x86_64` + and the `windows-x86-64` fat-jar smoke run inference through it; CMake writes + `jllama-extras.txt` itself. Not yet verified: the first CI run (no Windows machine in the + session that wired it). The findings that led there, all still valid: only plain `clang`/`clang++` with the GNU driver -- upstream's `cmake/x64-windows-llvm.cmake`, four lines -- produces all 14 x86 variants: CMake sets `MSVC` for `clang-cl` as for `cl.exe`, and ggml's `if (NOT MSVC)` then drops `ivybridge`, `piledriver`, `cooperlake`, `zen4` and `sapphirerapids` (a Zen 4/5 machine falls back to `icelake` and loses the @@ -61,9 +67,11 @@ deliverable 6: disappears *structurally* rather than being worked around -- the loader race over an old resident `msvcp140.dll` cannot happen when there is no import to resolve. So **the licence/REUSE decision on Microsoft "Distributable Code" is void**, and `nativedeps.py`'s `Windows/*/cpu` allowlist gets - *shorter*, not longer. One wiring consequence: `llama/CMakeLists.txt`'s static-CRT block is - `if(MSVC AND ...)`, and CMake's `MSVC` is **false** for plain clang -- widen that guard or pass - the flags from the job, or the clang build silently gets the dynamic CRT back. + *shorter*, not longer. One wiring consequence, since handled: `llama/CMakeLists.txt`'s + static-CRT block was `if(MSVC AND ...)`, and CMake's `MSVC` is **false** for plain clang -- it + now has an `elseif(WIN32 AND Clang)` branch spelling the same hybrid CRT, so the clang build + cannot silently get the dynamic CRT back (and `verify-native-deps.py` would red `package` if it + did). **(b) `GGML_OPENMP=OFF` -- and it is a throughput *win*, not a trade.** It was known to save the `libomp.dll` dependency (confirmed: without it the clang build imports `libomp140.x86_64.dll`, From b875b5545aefe7127acedd5ec01e98e51e281c67 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 10:58:44 +0000 Subject: [PATCH 4/5] NativeServer attach mode serves the model's own server_routes; endpoint toggles become ModelParameters llama_server_attach (patches/0007) built a server_routes on the attach worker's stack frame. server_routes' constructor registers a sleeping-state callback on the model's task queue, and server_queue::on_sleeping_state only appends -- there is no unregister -- so after NativeServer.close() the frame was gone and the callback stayed: the model's next idle sleep (--sleep-idle-seconds) ran update_cached_responses() on a destroyed object (a lock on freed memory, a dangling meta pointer). Found at the b11361 bump, TODO.md "NativeServer attach mode leaves a sleep callback behind". The attach entry now takes the caller's server_routes -- llama_server_attach(argc, argv, server_context &, server_routes &) -- and native_server.cpp passes jllama_context::routes, the object the model load builds before load_model() (as upstream's llama_server() does) and keeps for the model's lifetime; a vocab-only model, which has none, is refused with a LlamaException. The declaration in the patch and native_server_bridge.h change accordingly; the hunk counts were recounted, check-patches.py is clean, and all 11 patches apply on a pristine b11538 checkout. Consequence, documented in NativeServer's Javadoc, the README and the CHANGELOG: the route handlers read the model's params, so route-level settings -- the endpoint toggles --metrics / --props / --slots, --slot-save-path, the slot count, the sampling defaults -- come from the model's ModelParameters, and such a flag in the attach argv is parsed and ignored; the argv keeps the HTTP side (--host, --port, --api-key, --ssl-*, --threads-http, ...). Because no ModelParameters setter existed for the toggles, ModelFlag gains METRICS, PROPS, SLOTS and NO_SLOTS (all registered by llama.cpp's server parser, which test_model_flags.cpp checks) with enableMetricsEndpoint(), enablePropsEndpoint(), enableSlotsEndpoint() and disableSlotsEndpoint(); ModelFlagTest and ModelParametersExtendedTest cover them. NativeServerAttachSleepIntegrationTest (model-backed: the draft model with --sleep-idle-seconds 1 and --metrics) attaches a server, closes it, lets the model sleep and wake, attaches a second server, scrapes /metrics through it, and sleeps and wakes once more. Not run here (no model in this environment, Hugging Face unreachable), so the defect was not shown red first; CI runs it on every Java job. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01AytmJF9faEiQEVt6eetQS2 --- CHANGELOG.md | 14 ++ CLAUDE.md | 2 +- README.md | 10 +- TODO.md | 15 -- .../0007-server-attach-http-frontend.patch | 19 ++- llama/src/main/cpp/native_server.cpp | 17 +- llama/src/main/cpp/native_server_bridge.h | 3 +- .../net/ladenthin/llama/args/ModelFlag.java | 18 ++- .../llama/parameters/ModelParameters.java | 41 +++++ .../ladenthin/llama/server/NativeServer.java | 25 ++- .../ladenthin/llama/args/ModelFlagTest.java | 6 +- .../ModelParametersExtendedTest.java | 20 +++ ...ativeServerAttachSleepIntegrationTest.java | 145 ++++++++++++++++++ 13 files changed, 298 insertions(+), 37 deletions(-) create mode 100644 llama/src/test/java/net/ladenthin/llama/server/NativeServerAttachSleepIntegrationTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 9590872d3..8df934428 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ from version 5.0.0 onward. Pre-fork releases (`1.x`–`4.2.0`) were authored by ## [Unreleased] ### Added +- **`ModelParameters.enableMetricsEndpoint()` / `enablePropsEndpoint()` / `enableSlotsEndpoint()` / + `disableSlotsEndpoint()`** (`--metrics`, `--props`, `--slots`, `--no-slots`): the HTTP frontend's + endpoint toggles, needed since `NativeServer` attach mode serves the model's own routes (see Fixed). - **`test-cpp-windows-x86_64`**: the C++ suite on Windows x86-64 with plain clang (a static build with the tests), the compiler the shipped `cpu-windows-x86-64` natives are built with now; the two MSVC jobs keep running it with `cl.exe`. Gates both publish jobs like every other job. @@ -326,6 +329,17 @@ from version 5.0.0 onward. Pre-fork releases (`1.x`–`4.2.0`) were authored by 4.4.7 (none of the files the agent's carried JLine fixes touch changed between 4.4.6 and 4.4.7). ### Fixed +- **`NativeServer` attach mode no longer leaves a sleep callback into freed memory behind.** + `llama_server_attach` (`patches/0007`) built a `server_routes` on its own stack frame; its + constructor registers a sleeping-state callback on the model's task queue, which has no unregister, + so after `NativeServer.close()` the model's next idle sleep (`--sleep-idle-seconds`) ran the callback + on a destroyed object. The attached server now serves the model's own `server_routes` + (`jllama_context::routes`, alive as long as the model); pinned by + `NativeServerAttachSleepIntegrationTest`. **Behaviour change:** route-level settings therefore come + from the model's `ModelParameters` -- the endpoint toggles `--metrics` / `--props` / `--slots` (new + setters, see Added), `--slot-save-path`, the slot count, the sampling defaults -- and an attach + argument of that kind is parsed and ignored; the attach argv keeps the HTTP side (`--host`, `--port`, + `--api-key`, `--ssl-*`, `--threads-http`, ...). - **`GGML_CUDA_GRAPHS_DEFAULT` is repeated in the CPU-variants CMake path.** That path adds ggml before llama.cpp and therefore skips the block in which llama.cpp changes two ggml defaults; it repeated only `GGML_LLAMAFILE_DEFAULT`. Latent for the shipped CPU jars, but a CUDA backend built as a module diff --git a/CLAUDE.md b/CLAUDE.md index c979089f9..6d9c05a03 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1195,7 +1195,7 @@ Current patches: | `0001-win32-arg-parse-embed-guard.patch` | Windows JNI regression from llama.cpp **#24779** (introduced b9739): on Windows `common_params_parse` re-derived argv from the **process** command line (`GetCommandLineW`) and adopted it, so an embedded/JNI caller (`java.exe`) lost its `--model …` args → "Failed to parse model parameters". b9789 narrowed the unconditional override to a **count-guard** (`if (static_cast(utf8.buf.size()) == argc) { argv = utf8.ptrs.data(); }`), but that is exactly the variant the project already found breaks its Windows server-integration tests (when the embedded argv length coincides with `java.exe`'s). The patch carries the **complete upstream change** (so it can be submitted to llama.cpp verbatim and then dropped here): **(1)** `common_params_parse` parses **exactly the argv it is given** (no `GetCommandLineW` magic) and a new `common_params_parse_main()` wrapper holds the UTF-8 recovery for the standalone tools' `main()` (`common/arg.{cpp,h}`); **(2)** the **~34 standalone `main()` call sites** (every `common_params_parse(argc, argv, …)` across `tools/*`, `examples/*` and the `tests/*` programs) flip to `common_params_parse_main()`; **(3)** a `tests/test-arg-parser.cpp` regression case pins that `common_params_parse` honors a caller-supplied argv. The embedded caller (`jllama.cpp`) keeps calling `common_params_parse` and is never overridden. **Our subproject build compiles only the `arg.{cpp,h}` core** — `LLAMA_BUILD_TOOLS`/`LLAMA_BUILD_TESTS` are OFF for a FetchContent subproject — so the flips + test are applied-but-not-compiled here; they were validated via a one-off `-DLLAMA_BUILD_TOOLS=ON -DLLAMA_BUILD_TESTS=ON` build (the new test compiles and its asserts pass; `test-arg-parser`'s only red there is the live `ggml.ai` download check, which is sandbox-network, not the patch). Because it spans **35 files** it must be refreshed on every llama.cpp bump (the applier fails loud). **Refreshed at the b10679 bump:** upstream rewrote `tests/test-save-load-state.cpp`'s `main()` to take a `--models DIR` option, which it strips itself into a `filtered_argv` before calling `common_params_parse(fargc, filtered_argv.data(), …)`. That call site therefore stopped qualifying for the `_main()` flip — by this patch's own rule a caller that builds its own argv must use `common_params_parse` directly, so its argv is kept — and the hunk was **dropped** rather than refreshed (37 → 36 files). Caveat for whoever submits this upstream: that `main()` now filters a possibly-mojibake Windows argv *before* any UTF-8 recovery, so the fully correct upstream form there is recover-then-filter, not a one-line flip. It is out of scope for the downstream carry because `LLAMA_BUILD_TESTS` is OFF here, so the file is never compiled. **Refreshed the same way at the b11236 bump:** upstream #29426 rewrote `tests/test-recurrent-state-rollback.cpp`'s `main()` into the identical `--models DIR` / `filtered_argv` shape, so that hunk was dropped too (36 → 35 files); the same recover-then-filter caveat applies to it. **Still required at b10679, verified rather than assumed:** `common_params_parse` in pristine `b10679:common/arg.cpp` still carries the `#ifdef _WIN32` count-guarded `argv = utf8.ptrs.data()` override, and `common_params_parse_main` appears nowhere in `b10679:common/arg.h` — upstream has not adopted the fix. The upstream-facing write-up, including a standalone reproducer that makes llama.cpp's own `test-arg-parser` fail on unmodified `master`, lives in [docs/upstream-investigation-win32-argv-substitution.md](docs/upstream-investigation-win32-argv-substitution.md). **Reported upstream as [ggml-org/llama.cpp#26416](https://github.com/ggml-org/llama.cpp/issues/26416)** (2026-08-01, label `bug-unconfirmed`, first bad commit `508a475`); the issue asks which of the two directions the maintainers prefer before a PR is opened, so this patch stays downstream until they answer. | | `0002-server-preserve-caller-load-progress-callback.patch` | Load-progress-callback regression introduced in llama.cpp **b9789**: `server_context::load_model` (`tools/server/server-context.cpp`) now **unconditionally** installs the server's own load-progress reporter on `params_base.load_progress_callback` immediately before `common_init_from_params`, clobbering any callback the embedding caller already set. libjllama's `LoadProgressCallback` feature wires `common_params.load_progress_callback` to a JNI trampoline *before* calling `load_model`, so the bump silently killed it — `LoadProgressCallbackTest` saw zero progress updates and the abort-on-`false` path never threw. The patch guards the assignment with `if (params_base.load_progress_callback == nullptr)`, so the server installs its own reporter **only when the caller hasn't** — a caller-supplied callback survives and fires during load. Standalone `llama-server` (no caller callback, so the field is null) is unaffected. Same JNI-vs-standalone divergence class as `0001`. **The guard is `== nullptr || == load_progress_callback`, and the second disjunct must never be dropped:** `load_progress_text` is a **local** of `load_model()`, and upstream re-assigns both fields on every call so the `user_data` always points at the current frame. `load_model()` runs a **second** time when resuming from the sleeping state (`--sleep-idle-seconds`), and by then `params_base` holds *our own* callback from the first load — a bare nullptr check skips the re-assignment and leaves `user_data` pointing into a **dead stack frame**, which segfaults inside `load_progress_callback()` on the first request after an idle window. That was a latent defect in this patch from the day it was written; only a second `load_model()` can reach it, and nothing exercised sleep until `IdleSleepWakeIntegrationTest` was added. | | `0003-pr22393-server-add-slot-prompt-similarity-getter-setter.patch` | **Upstream-PR carry** of [ggml-org/llama.cpp#22393](https://github.com/ggml-org/llama.cpp/pull/22393) ("server : add slot_prompt_similarity getter/setter"). Purely additive: adds `server_context::get_slot_prompt_similarity()` / `set_slot_prompt_similarity(float)` (`tools/server/server-context.{cpp,h}`) so an embedding/JNI caller can query and tune the slot-selection threshold at runtime without reloading the model. Verbatim copy of the PR, which **upstream closed without merging** (rejected as exposing unsafe internal state — see the patch header). Carried permanently; it will not be droppable via a version bump. | -| `0007-server-attach-http-frontend.patch` | **Adds `llama_server_attach(argc, argv, server_context&)`** so the `NativeServer` *attach mode* can serve an **already-loaded `LlamaModel`** over the upstream HTTP frontend — no second model load, no `start_loop()`; the LlamaModel's worker keeps driving the shared `server_context` and the HTTP routes post tasks to its queue (the queue is the synchronization point). Mechanically: (1) extracts the **pure core route table** (`health` … `slots`) out of `llama_server()` into `static void llama_server_register_common_routes(ctx_http, routes)` (shared, so the two entry points cannot drift on the core endpoint set). **Scope note (narrowed at the b10154 bump):** the helper deliberately carries **only** the stable, state-independent route table — **not** the resumable-streaming routes (their handlers differ between router / non-router), the GCP-compat shim, or the experimental **CORS-proxy / MCP-server / built-in-tools** wiring. b10154 (upstream MCP-server support) moved the streaming routes into the middle of that block and coupled tools/CORS to a per-call `server_mcp mcp_mgr` lifecycle, so the earlier contiguous "route-table + CORS-proxy + tools" extraction is no longer possible; `llama_server()` keeps all of that inline, **byte-identical to upstream b10154** (only the route-table block is factored out). (2) adds `llama_server_attach`, which parses only the HTTP-side argv via `common_params_parse`, starts the stream-session GC + `server_http_context`, registers the common route table, the **non-router** resumable-streaming handlers (upstream b10154 paths `/v1/stream` GET/DEL + `/v1/streams/lookup` POST), the GCP-compat shim, and **403 "disabled" stubs for `/cors-proxy` + `/tools`** (attach mode does not wire the experimental CORS-proxy / MCP / built-in-tools host — those belong to a full `llama-server`, not an embedded model), marks ready immediately (model already loaded), and blocks on the HTTP thread until `llama_server_request_shutdown()` — never calling `common_init()`, backend init, `ctx_server.terminate()` or `llama_backend_free()` (the embedding caller owns those). Applies after `0001`+`0006` (same file); closes the "NativeServer — reuse an already-loaded LlamaModel" TODO. Upstream-submittable ("server: let embedding callers attach the HTTP frontend to an existing server_context"). **Refreshed at the b10519 bump:** upstream #26347 dropped the API key from the `/models` + `/v1/models` public-endpoint set and deleted the two trailing `// public endpoint (no API key check)` comments on those route registrations. Those two lines sit inside this patch's route-table removal block, so `git apply` failed ("patch does not apply", `server.cpp:258`) at **every** tag from b10519 on; the fix was to drop the now-wrong comment from all four affected lines (2 on the `-` side, 2 in the extracted helper on the `+` side), keeping the helper byte-identical to the block it replaces. **This is the invariant to re-check on every bump:** the `+` side of `llama_server_register_common_routes()` must stay a verbatim copy of the route table it factors out of `llama_server()`. **Refreshed at the b11104 bump** (upstream #28690, multi-address `--host`): `server_http_context` lost its single `thread` and `listening_address` members in favour of `join()` and a `listening_addresses` vector, one listener thread per bound address. The patch still *applied* cleanly there — only its own `+` lines named the removed members — so the applier could not see it; `llama_server_attach` now logs every address and blocks in `ctx_http.join()`, exactly as upstream's `llama_server()` does. **Refreshed at the b11361 bump** (#29818 adds `POST /v1/systemone` to the route table: one line, added on both sides so the helper stays verbatim) **and at b11401** (#29895 adds a `server_child &` overload of `llama_server()` beside the declarations and a `server_child child;` at the top of the argv entry point, the context of `0007`'s first two hunks; only context moved, the `+`/`-` lines are unchanged). Attach mode builds its own `server_routes`, whose sleep callback outlives it -- on file in `TODO.md`. | +| `0007-server-attach-http-frontend.patch` | **Adds `llama_server_attach(argc, argv, server_context&)`** so the `NativeServer` *attach mode* can serve an **already-loaded `LlamaModel`** over the upstream HTTP frontend — no second model load, no `start_loop()`; the LlamaModel's worker keeps driving the shared `server_context` and the HTTP routes post tasks to its queue (the queue is the synchronization point). Mechanically: (1) extracts the **pure core route table** (`health` … `slots`) out of `llama_server()` into `static void llama_server_register_common_routes(ctx_http, routes)` (shared, so the two entry points cannot drift on the core endpoint set). **Scope note (narrowed at the b10154 bump):** the helper deliberately carries **only** the stable, state-independent route table — **not** the resumable-streaming routes (their handlers differ between router / non-router), the GCP-compat shim, or the experimental **CORS-proxy / MCP-server / built-in-tools** wiring. b10154 (upstream MCP-server support) moved the streaming routes into the middle of that block and coupled tools/CORS to a per-call `server_mcp mcp_mgr` lifecycle, so the earlier contiguous "route-table + CORS-proxy + tools" extraction is no longer possible; `llama_server()` keeps all of that inline, **byte-identical to upstream b10154** (only the route-table block is factored out). (2) adds `llama_server_attach`, which parses only the HTTP-side argv via `common_params_parse`, starts the stream-session GC + `server_http_context`, registers the common route table, the **non-router** resumable-streaming handlers (upstream b10154 paths `/v1/stream` GET/DEL + `/v1/streams/lookup` POST), the GCP-compat shim, and **403 "disabled" stubs for `/cors-proxy` + `/tools`** (attach mode does not wire the experimental CORS-proxy / MCP / built-in-tools host — those belong to a full `llama-server`, not an embedded model), marks ready immediately (model already loaded), and blocks on the HTTP thread until `llama_server_request_shutdown()` — never calling `common_init()`, backend init, `ctx_server.terminate()` or `llama_backend_free()` (the embedding caller owns those). Applies after `0001`+`0006` (same file); closes the "NativeServer — reuse an already-loaded LlamaModel" TODO. Upstream-submittable ("server: let embedding callers attach the HTTP frontend to an existing server_context"). **Refreshed at the b10519 bump:** upstream #26347 dropped the API key from the `/models` + `/v1/models` public-endpoint set and deleted the two trailing `// public endpoint (no API key check)` comments on those route registrations. Those two lines sit inside this patch's route-table removal block, so `git apply` failed ("patch does not apply", `server.cpp:258`) at **every** tag from b10519 on; the fix was to drop the now-wrong comment from all four affected lines (2 on the `-` side, 2 in the extracted helper on the `+` side), keeping the helper byte-identical to the block it replaces. **This is the invariant to re-check on every bump:** the `+` side of `llama_server_register_common_routes()` must stay a verbatim copy of the route table it factors out of `llama_server()`. **Refreshed at the b11104 bump** (upstream #28690, multi-address `--host`): `server_http_context` lost its single `thread` and `listening_address` members in favour of `join()` and a `listening_addresses` vector, one listener thread per bound address. The patch still *applied* cleanly there — only its own `+` lines named the removed members — so the applier could not see it; `llama_server_attach` now logs every address and blocks in `ctx_http.join()`, exactly as upstream's `llama_server()` does. **Refreshed at the b11361 bump** (#29818 adds `POST /v1/systemone` to the route table: one line, added on both sides so the helper stays verbatim) **and at b11401** (#29895 adds a `server_child &` overload of `llama_server()` beside the declarations and a `server_child child;` at the top of the argv entry point, the context of `0007`'s first two hunks; only context moved, the `+`/`-` lines are unchanged). **Changed in 5.2.x, not a refresh: `llama_server_attach(argc, argv, server_context &, server_routes &)` serves the caller's `server_routes`** (`jllama_context::routes`, the one the model load builds) **instead of constructing one on its own frame.** `server_routes`' constructor registers a sleeping-state callback on the model's task queue, `server_queue::on_sleeping_state` only appends, so the frame-local object left a callback into freed memory behind when the attach returned, and the model's next idle sleep (`--sleep-idle-seconds`) ran it -- undefined behaviour, found at the b11361 bump and pinned by `NativeServerAttachSleepIntegrationTest` (model-backed, CI). Consequence: the route handlers read the **model's** params, so the endpoint toggles (`--metrics`, `--props`, `--slots`), `--slot-save-path`, the slot count and the sampling defaults are `ModelParameters` now (`enableMetricsEndpoint()` & co., added for that), and the attach argv configures the HTTP side only. | | `0008-server-models-worker-cmd-override.patch` | **Makes router mode usable in-JVM.** The router (`server-models.cpp`) spawns each model worker by re-executing its own binary (`get_server_exec_path()` = `/proc/self/exe` & friends) — inside a JVM that binary is `java`, not a llama-server, so embedded router workers could never start. The patch adds env `LLAMA_SERVER_WORKER_CMD` (whitespace-split; read in `server_model_meta::update_args`) which replaces only the leading binary-path token of the rendered worker args, letting an embedding host relaunch workers through its own bootstrap — e.g. `java -cp app.jar net.ladenthin.llama.server.NativeServer` (each worker is then a fresh JVM running the classic single-model `NativeServer`). Exposed in Java as `NativeServer.setWorkerCommand(String...)` (JNI `setenv`); exercised by `RouterModeIntegrationTest` (Linux CI). Upstream-submittable (also useful for containerized/wrapped deployments). **Refreshed at the b11429 bump:** #29987 (untagged `4d60b4d08`, the commit before b11429) moved `server_model_meta::update_args` and changed the function after it (`update_caps(const common_params &)`), the hunk's trailing context; the `+` lines are unchanged. Since b11401 (#29895) a worker is a router *child* whose stdout carries the state commands, which a JVM worker's early `System.out` line now trips (cosmetic, `TODO.md`). | | `0006-server-embed-native-server-jni.patch` | **Makes `server.cpp`'s `llama_server` embeddable in the JVM** so the `NativeServer` JNI bridge can run the full upstream HTTP server (WebUI included) inside `libjllama` — see "Two server modes" below. b9870 already exposes `int llama_server(int, char**)` (non-static; no `main` in the file), so the patch only adds embedded-mode support: (1) a `g_llama_server_embedded` flag + `llama_server_set_embedded()` / `llama_server_request_shutdown()` (declared in the committed `src/main/cpp/native_server_bridge.h`); (2) skips installing the process-wide SIGINT/SIGTERM handlers when embedded (they would hijack the JVM's); (3) in embedded mode parses the **forwarded** argv via `common_params_parse` instead of `common_params_parse_main` (whose `GetCommandLineW` recovery would pick up `java.exe`'s command line — the same Windows class of bug `0001` fixes). `llama_server_request_shutdown()` mirrors the SIGTERM path (invokes the installed `shutdown_handler` → `ctx_server.terminate()` unblocks `start_loop()`), giving JNI an out-of-band stop since `ctx_server` is loop-local. **The handler is guarded, and both guards are load-bearing:** upstream's `shutdown_handler` lambdas capture `llama_server()`'s locals (`ctx_http`, `models_routes`, `mcp_mgr`, `ctx_server`) by reference and are never cleared, while `native_server.cpp` signals *repeatedly* until the worker reports it finished (a stop issued before the handler is installed, or before `start_loop()` begins, would otherwise be lost). A request landing after `llama_server()` returned therefore ran the lambda over destroyed objects — a `SIGSEGV` in `server_http_context::stop()` in `RouterModeIntegrationTest.tearDown` (run 36485159455), with router mode's `clean_up()` (unloading workers) making the window wide. The patch (1) takes `g_shutdown_handler_mutex` for every read, write and invocation of the handler (the caller is on another thread) and (2) declares a `llama_server_shutdown_handler_reset` **after** the captured locals, so the handler is cleared before any of them is destroyed; because invocation holds the same lock, that reset waits for an invocation in progress. `0007`'s `llama_server_attach` carries the same guard. Runnable guard: `src/test/cpp/test_native_server_shutdown.cpp`. Applies **after `0001`** (which flips this call site to `common_params_parse_main`), so its context is the post-`0001` tree; regenerate against `0001`+source on a bump. Only touches `tools/server/server.cpp`. | | `0012-model-guard-zero-split-sum-and-name-the-device-index.patch` | **A GPU that reports zero free memory makes every model load fail with the unactionable `error loading model: vector`.** `llama_model_base::load_tensors` (`src/llama-model.cpp`) weights the per-device layer split by `ggml_backend_dev_memory()`'s `free`, then normalises: `splits[i] /= split_sum`. With a single device reporting `free == 0` that is `0/0` → **NaN** in every split point; NaN compares false against everything, so the `std::upper_bound` below returns the end iterator, `layer_gpu == n_devices()`, and `devices.at(layer_gpu)` throws `std::out_of_range` — whose libc++ `what()` is the bare string `"vector"`, which `llama.cpp`'s `catch (const std::exception &)` prints verbatim. Upstream's `free == 0 && total == 0` host-memory fallback does **not** fire, because `total` is `recommendedMaxWorkingSetSize` and is non-zero. **Reachable since b10618..b10797**: upstream `8c0b9cd04` ("metal : fix memory query under low-memory conditions", [#27701](https://github.com/ggml-org/llama.cpp/pull/27701)) changed `ggml-metal-device.m` to `*free = *total > cur ? *total - cur : 0`; before that clamp an over-committed device (`currentAllocatedSize > recommendedMaxWorkingSetSize`) *underflowed* to a huge `size_t`, which normalised fine, so the same precondition was harmless. That is why the `Java Tests macOS …` jobs went red at the b10792→b10797 step while every Linux/Windows job stayed green — **and why only a GPU build can fail this way at all**: `act_gpu_layers` is `devices.empty() ? 0 : …`, so with no GPU backend `devices` is empty, every layer returns early on `cpu_dev`, and the `.at()` line is unreachable. **Shape:** the two blocks are lifted out of `load_tensors` into free functions declared in `src/llama-model.h`, purely so they can be driven by a test — the failing state needs a real over-committed GPU and cannot be arranged through any public API. `llama_model_splits_normalize()` carries **the fix**: on `split_sum == 0` it `LLAMA_LOG_WARN`s and falls back to an even split (`splits[i] = float(i+1)/splits.size()`), the only neutral choice when no device can be preferred and exactly right for a single device. `llama_model_splits_select_device()` carries **the diagnostic**: it bounds-checks the index and throws a `std::runtime_error` naming the function, the offloaded layer, the device index, the split-point count **and the split points themselves** — with NaN splits that message prints `nan` and names the cause outright, which is precisely what was missing when this had to be diagnosed by reading source. **A second, backend-independent trigger reaches the same line**, found while writing this up and verified against the unfixed library: `--tensor-split` values are parsed with `std::stof` and never range-checked (`common/arg.cpp`), so `-ts 1,-1` cancels out, `split_sum` is 0 again, the split points become `[inf, -nan]`, and every layer maps one past the last device — on CUDA, Vulkan or ROCm just as much as on Metal, with no memory pressure involved. That is what makes this an ordinary upstream defect rather than a Metal edge case, and the warning names both causes rather than only the memory one. Also adds upstream `tests/test-model-split.cpp` (5 cases in upstream's `testing.h` style) + its `llama_build_and_test` registration. Touches `src/llama-model.{cpp,h}`, `tests/test-model-split.cpp` and `tests/CMakeLists.txt` — **none** of which any other patch touches, so it is independent of all of them. Upstream-submittable ("model: fall back to an even split when no device reports free memory"); **not yet filed upstream**. **Runnable guard: `src/test/cpp/test_model_split.cpp`** — a FetchContent subproject builds with `LLAMA_BUILD_TESTS=OFF`, so the upstream test above is applied-but-never-compiled here (same as `0001`'s test). That file drives the same two functions from `jllama_test`, which runs on **every** platform in `C++ Tests`, so a bump that drops this patch fails the build at link time everywhere instead of surfacing as one red macOS Java job. **Verification limit — read before assuming this can be dropped:** the *failing path* still cannot be reached without a GPU backend, so the guard pins the arithmetic (what actually broke), not the end-to-end load; the end-to-end proof is the macOS CI job. On a bump, re-check whether upstream added its own `split_sum == 0` guard (grep `split_sum` in `src/llama-model.cpp`) and **drop this patch rather than refreshing it** if they did — the fail-loud applier detects "does not apply", never "upstream already fixed this". | diff --git a/README.md b/README.md index 2a45de400..de98d1987 100644 --- a/README.md +++ b/README.md @@ -1009,9 +1009,13 @@ try (LlamaModel model = new LlamaModel(new ModelParameters().setModel("models/mo } ``` -In attach mode the arguments carry only the HTTP-side flags (`--host`, `--port`, `--api-key`, …; -no `-m`), the server reports healthy immediately (the model is already loaded), and the **caller -keeps ownership of the model** — close the server before the model, never the other way around. +In attach mode the arguments carry only the HTTP-side flags (`--host`, `--port`, `--api-key`, +`--ssl-key-file`, …; no `-m`); the routes served are the model's own, so route-level settings — the +endpoint toggles `--metrics` / `--props` / `--slots`, `--slot-save-path`, the slot count — are set on +the model's `ModelParameters` (`enableMetricsEndpoint()`, `enablePropsEndpoint()`, +`enableSlotsEndpoint()` / `disableSlotsEndpoint()`), not in the attach arguments. The server reports +healthy immediately (the model is already loaded), and the **caller keeps ownership of the model** — +close the server before the model, never the other way around. #### Router mode — multi-model management diff --git a/TODO.md b/TODO.md index 198ec78ae..0488dc7af 100644 --- a/TODO.md +++ b/TODO.md @@ -297,21 +297,6 @@ answered, read→write→read loop changed the file). Still open: - **Model recommendation table** for the agent (which local GGUFs actually complete an edit→build→test loop) — needs a GPU host, not CI. -### NativeServer attach mode leaves a sleep callback behind (found at the b11361 bump, not reproduced) - -`llama_server_attach` (`patches/0007`) builds a `server_routes` on its own stack frame over the -`LlamaModel`'s `server_context`. Its constructor registers a sleeping-state callback on the model's -queue (`server_queue::on_sleeping_state` only appends, there is no unregister), and that callback -captures the `server_routes`. When the attached `NativeServer` is closed, the frame returns and the -object is gone, but the callback stays in the queue of the model, which lives on. The next time that -model enters idle sleep, the callback runs on a destroyed object. Reachable only with a model loaded -with `--sleep-idle-seconds` that was served by an attached `NativeServer` and then kept in use after -the server closed. Since b11361 `LlamaModel` holds a `server_routes` of its own for its whole lifetime -(`jllama_context::routes`, for `handleSystemOne`); the natural fix is to let attach mode serve -*that* object instead of building a second one, which changes `llama_server_attach`'s signature in -`0007` and `native_server.cpp`. Needs a test with sleep enabled (`IdleSleepWakeIntegrationTest` is the -template) before the fix, to show it red first. - ### LlamaLoader extraction-directory isolation (optional follow-up, low priority) Left over from the 2026-06-20 code audit (18/18 findings fixed in PRs #258/#260, regression tests in diff --git a/llama/patches/0007-server-attach-http-frontend.patch b/llama/patches/0007-server-attach-http-frontend.patch index a700ba2f3..44587739e 100644 --- a/llama/patches/0007-server-attach-http-frontend.patch +++ b/llama/patches/0007-server-attach-http-frontend.patch @@ -6,7 +6,7 @@ index 4c6052d78..cb03f239a 100644 // to be used via CLI (argc / argv are used by router mode only) int llama_server(common_params & params, int argc, char ** argv); int llama_server(common_params & params, int argc, char ** argv, server_child & child); -+int llama_server_attach(int argc, char ** argv, server_context & ctx_server); ++int llama_server_attach(int argc, char ** argv, server_context & ctx_server, server_routes & routes); void llama_server_terminate(); void llama_server_terminate() { if (shutdown_handler) { @@ -119,7 +119,7 @@ index 4c6052d78..cb03f239a 100644 // resumable streaming: a child binds the local session factories, the router binds // proxies that resolve the owning child, see server-stream.h -@@ -628,3 +640,91 @@ int llama_server(common_params & params, int argc, char ** argv, server_child & +@@ -628,3 +640,98 @@ int llama_server(common_params & params, int argc, char ** argv, server_child & return 0; } @@ -136,7 +136,15 @@ index 4c6052d78..cb03f239a 100644 +// server / built-in-tools endpoints are NOT wired in attach mode (they belong to a full +// llama-server host, not an embedded model); they answer 403 "disabled" like a stock build with +// those features off. -+int llama_server_attach(int argc, char ** argv, server_context & ctx_server) { ++// `routes` is the caller's own server_routes over that context -- the one its model load built, ++// as llama_server() builds one before load_model(). It must be the caller's: server_routes ++// registers a sleeping-state callback on the context's task queue in its constructor and the ++// queue never unregisters, so a server_routes built on this frame left a callback into a ++// destroyed object behind when the attach returned, and the model's next idle sleep ran it. ++// The caller keeps `routes` alive as long as the context. The route handlers therefore read the ++// MODEL's params (--metrics / --props / --slots, n_parallel, the sampling defaults, ...); the argv ++// configures the HTTP side only (--host, --port, --api-key, --ssl-*, --threads-http, ...). ++int llama_server_attach(int argc, char ** argv, server_context & ctx_server, server_routes & routes) { + std::setlocale(LC_NUMERIC, "C"); + + common_params params; @@ -157,7 +165,6 @@ index 4c6052d78..cb03f239a 100644 + return 1; + } + -+ server_routes routes(params, ctx_server); + llama_server_register_common_routes(ctx_http, routes); + + // single-model (non-router) resumable-streaming handlers (upstream b10154 paths) @@ -193,8 +200,8 @@ index 4c6052d78..cb03f239a 100644 + return 1; + } + -+ // the caller's model is already loaded, so the frontend is ready immediately -+ routes.update_meta(ctx_server); ++ // the caller's model is already loaded and its routes carry the model's metadata (the caller ++ // ran update_meta() after the load), so the frontend is ready immediately + ctx_http.is_ready.store(true); + + llama_server_set_shutdown_handler([&](int) { diff --git a/llama/src/main/cpp/native_server.cpp b/llama/src/main/cpp/native_server.cpp index 1fc5d564c..d06829b74 100644 --- a/llama/src/main/cpp/native_server.cpp +++ b/llama/src/main/cpp/native_server.cpp @@ -190,6 +190,17 @@ JNIEXPORT jlong JNICALL Java_net_ladenthin_llama_server_NativeServer_startAttach // not the attached model's worker, so the LlamaModel remains usable after the server closes // (documented order: close server first, then model). If a future upstream change breaks // either assumption, this attach path needs revisiting. + // The routes are the model's own (jllama_context::routes, built before its load as upstream + // does), never a second server_routes: its constructor registers a sleeping-state callback on + // the model's task queue, which has no unregister, so a server_routes that died with the attach + // left a callback into freed memory that the model's next idle sleep ran. Consequently the + // endpoint toggles (--metrics, --props, --slots) and every other route-level setting come from + // the model's ModelParameters; the attach argv configures the HTTP side. + if (!jctx->routes) { + release_jllama_context_impl(jctx); + throw_llama_exception(env, "a vocab-only model has no server context to attach to"); + return 0; + } auto *srv = new native_server(); fill_native_server_args(env, jargs, srv); bind_native_server_argv(srv); @@ -199,8 +210,10 @@ JNIEXPORT jlong JNICALL Java_net_ladenthin_llama_server_NativeServer_startAttach llama_server_set_embedded(true); server_context *ctx_server = &jctx->server; - srv->worker = std::thread([srv, ctx_server, jctx]() { - srv->exit_code = llama_server_attach(static_cast(srv->argv.size()), srv->argv.data(), *ctx_server); + server_routes *routes = jctx->routes.get(); + srv->worker = std::thread([srv, ctx_server, routes, jctx]() { + srv->exit_code = + llama_server_attach(static_cast(srv->argv.size()), srv->argv.data(), *ctx_server, *routes); srv->finished.store(true); release_jllama_context_impl(jctx); }); diff --git a/llama/src/main/cpp/native_server_bridge.h b/llama/src/main/cpp/native_server_bridge.h index d86bebe5e..8239af6fb 100644 --- a/llama/src/main/cpp/native_server_bridge.h +++ b/llama/src/main/cpp/native_server_bridge.h @@ -27,8 +27,9 @@ // single-instance-per-process rule covers both entry points). struct server_context; +struct server_routes; int llama_server(int argc, char **argv); -int llama_server_attach(int argc, char **argv, server_context &ctx_server); +int llama_server_attach(int argc, char **argv, server_context &ctx_server, server_routes &routes); void llama_server_set_embedded(bool embedded); void llama_server_request_shutdown(); diff --git a/llama/src/main/java/net/ladenthin/llama/args/ModelFlag.java b/llama/src/main/java/net/ladenthin/llama/args/ModelFlag.java index 3b580f1b3..0807fe4ad 100644 --- a/llama/src/main/java/net/ladenthin/llama/args/ModelFlag.java +++ b/llama/src/main/java/net/ladenthin/llama/args/ModelFlag.java @@ -145,7 +145,23 @@ public enum ModelFlag { * {@link net.ladenthin.llama.exception.ModelUnavailableException} so callers can distinguish an * air-gapped miss from a genuine misconfiguration.

*/ - OFFLINE("--offline"); + OFFLINE("--offline"), + + /** + * Enable the Prometheus-compatible {@code GET /metrics} endpoint of the HTTP frontend (off by default + * upstream). Read from the model's parameters in {@code NativeServer} attach mode, since the routes + * are the model's own. + */ + METRICS("--metrics"), + + /** Allow changing global properties via {@code POST /props} (off by default upstream; {@code GET} is always on). */ + PROPS("--props"), + + /** Expose the slots monitoring endpoint {@code GET /slots} (upstream's default). */ + SLOTS("--slots"), + + /** Hide the slots monitoring endpoint {@code GET /slots}. */ + NO_SLOTS("--no-slots"); private final String cliFlag; private final CliContract contract; diff --git a/llama/src/main/java/net/ladenthin/llama/parameters/ModelParameters.java b/llama/src/main/java/net/ladenthin/llama/parameters/ModelParameters.java index 8f46103b9..eb39125ca 100644 --- a/llama/src/main/java/net/ladenthin/llama/parameters/ModelParameters.java +++ b/llama/src/main/java/net/ladenthin/llama/parameters/ModelParameters.java @@ -1143,6 +1143,47 @@ public ModelParameters enableReranking() { return setFlag(ModelFlag.RERANKING); } + /** + * Enable the Prometheus-compatible {@code GET /metrics} endpoint ({@code --metrics}; off by default). + * Relevant for the HTTP frontend only: {@code NativeServer} attach mode serves the model's own routes, + * so the endpoint toggles are set here, not in the attach arguments. + * + * @return this builder + */ + public ModelParameters enableMetricsEndpoint() { + return setFlag(ModelFlag.METRICS); + } + + /** + * Allow changing global properties via {@code POST /props} ({@code --props}; off by default, + * {@code GET /props} is always available). HTTP frontend only, see {@link #enableMetricsEndpoint()}. + * + * @return this builder + */ + public ModelParameters enablePropsEndpoint() { + return setFlag(ModelFlag.PROPS); + } + + /** + * Expose the slots monitoring endpoint {@code GET /slots} ({@code --slots}, upstream's default). HTTP + * frontend only, see {@link #enableMetricsEndpoint()}. + * + * @return this builder + */ + public ModelParameters enableSlotsEndpoint() { + return setFlag(ModelFlag.SLOTS); + } + + /** + * Hide the slots monitoring endpoint {@code GET /slots} ({@code --no-slots}). HTTP frontend only, see + * {@link #enableMetricsEndpoint()}. + * + * @return this builder + */ + public ModelParameters disableSlotsEndpoint() { + return setFlag(ModelFlag.NO_SLOTS); + } + /** * Set minimum chunk size to attempt reusing from the cache via KV shifting. * diff --git a/llama/src/main/java/net/ladenthin/llama/server/NativeServer.java b/llama/src/main/java/net/ladenthin/llama/server/NativeServer.java index b91527267..5a4ae90b1 100644 --- a/llama/src/main/java/net/ladenthin/llama/server/NativeServer.java +++ b/llama/src/main/java/net/ladenthin/llama/server/NativeServer.java @@ -38,10 +38,13 @@ * already-loaded {@code LlamaModel} — no second copy of the weights, no second * model load: the model's own worker thread keeps driving inference and the HTTP routes post * tasks to its queue. In attach mode the arguments carry only the HTTP-side flags - * ({@code --host}, {@code --port}, {@code --api-key}, …; no {@code -m}), and the caller keeps - * full ownership of the model: do not {@code close()} the model while the server is - * attached — stop the server first (server before model, like unwinding - * try-with-resources).

+ * ({@code --host}, {@code --port}, {@code --api-key}, {@code --ssl-key-file}, …; no {@code -m}); + * the routes served are the model's own, so route-level settings — the endpoint toggles + * {@code --metrics} / {@code --props} / {@code --slots}, {@code --slot-save-path}, the slot count, + * the sampling defaults — are the model's {@link net.ladenthin.llama.parameters.ModelParameters} + * ({@code enableMetricsEndpoint()}, …), not attach arguments. The caller keeps full ownership of + * the model: do not {@code close()} the model while the server is attached — + * stop the server first (server before model, like unwinding try-with-resources).

* *

Router mode. Starting without any model argument puts the upstream server * in router mode ({@code --models-dir}, {@code GET/POST /models}, per-request model selection). @@ -125,9 +128,17 @@ public NativeServer(String... args) { * of loading a second copy of the weights. * *

The arguments carry only the HTTP-side llama-server flags ({@code --host}, - * {@code --port}, {@code --api-key}, {@code --slots}, …); no model argument is needed or - * used. Because the model is already loaded, the server reports ready on {@code GET /health} - * as soon as the socket is up.

+ * {@code --port}, {@code --api-key}, {@code --ssl-key-file}, {@code --threads-http}, …); no + * model argument is needed or used. The HTTP routes are the model's own + * ({@code server_routes}, built with the model, so that the sleeping-state callback they + * register on the model's task queue never outlives its object), which is why route-level + * settings come from the model's {@link net.ladenthin.llama.parameters.ModelParameters}: the + * endpoint toggles {@code --metrics} / {@code --props} / {@code --slots} + * ({@link net.ladenthin.llama.parameters.ModelParameters#enableMetricsEndpoint()}, + * {@code enablePropsEndpoint()}, {@code enableSlotsEndpoint()} / {@code disableSlotsEndpoint()}), + * {@code --slot-save-path}, the slot count and the sampling defaults; such a flag in the attach + * arguments is parsed and ignored. Because the model is already loaded, the server reports + * ready on {@code GET /health} as soon as the socket is up.

* *

Lifecycle contract: the caller keeps full ownership of {@code model}. * The model must stay open for as long as the server runs — close the server first, then the diff --git a/llama/src/test/java/net/ladenthin/llama/args/ModelFlagTest.java b/llama/src/test/java/net/ladenthin/llama/args/ModelFlagTest.java index 9d61ceea8..d180fb99c 100644 --- a/llama/src/test/java/net/ladenthin/llama/args/ModelFlagTest.java +++ b/llama/src/test/java/net/ladenthin/llama/args/ModelFlagTest.java @@ -47,6 +47,10 @@ public static Collection data() { {ModelFlag.MMPROJ_OFFLOAD, "--mmproj-offload"}, {ModelFlag.NO_MMPROJ_OFFLOAD, "--no-mmproj-offload"}, {ModelFlag.OFFLINE, "--offline"}, + {ModelFlag.METRICS, "--metrics"}, + {ModelFlag.PROPS, "--props"}, + {ModelFlag.SLOTS, "--slots"}, + {ModelFlag.NO_SLOTS, "--no-slots"}, }); } @@ -72,7 +76,7 @@ public void testEnumCount() { // Use ModelParameters#setLoadMode. // DUMP_KV_CACHE -- removed upstream with no replacement. // src/test/cpp/test_model_flags.cpp enforces this against the real option table. - assertEquals(31, ModelFlag.values().length); + assertEquals(35, ModelFlag.values().length); } @ParameterizedTest(name = "{0} -> {1}") diff --git a/llama/src/test/java/net/ladenthin/llama/parameters/ModelParametersExtendedTest.java b/llama/src/test/java/net/ladenthin/llama/parameters/ModelParametersExtendedTest.java index 1cc20415c..d96552cc6 100644 --- a/llama/src/test/java/net/ladenthin/llama/parameters/ModelParametersExtendedTest.java +++ b/llama/src/test/java/net/ladenthin/llama/parameters/ModelParametersExtendedTest.java @@ -745,6 +745,26 @@ public void testEnableReranking() { assertThat(p.parameters.get("--reranking"), is(nullValue())); } + @Test + public void testEnableMetricsEndpoint() { + ModelParameters p = new ModelParameters().enableMetricsEndpoint(); + assertThat(p.parameters, hasKey("--metrics")); + assertThat(p.parameters.get("--metrics"), is(nullValue())); + } + + @Test + public void testEnablePropsEndpoint() { + ModelParameters p = new ModelParameters().enablePropsEndpoint(); + assertThat(p.parameters, hasKey("--props")); + assertThat(p.parameters.get("--props"), is(nullValue())); + } + + @Test + public void testEnableAndDisableSlotsEndpoint() { + assertThat(new ModelParameters().enableSlotsEndpoint().parameters, hasKey("--slots")); + assertThat(new ModelParameters().disableSlotsEndpoint().parameters, hasKey("--no-slots")); + } + @Test public void testSetVocabOnly() { ModelParameters p = new ModelParameters().setVocabOnly(); diff --git a/llama/src/test/java/net/ladenthin/llama/server/NativeServerAttachSleepIntegrationTest.java b/llama/src/test/java/net/ladenthin/llama/server/NativeServerAttachSleepIntegrationTest.java new file mode 100644 index 000000000..ecdc891ae --- /dev/null +++ b/llama/src/test/java/net/ladenthin/llama/server/NativeServerAttachSleepIntegrationTest.java @@ -0,0 +1,145 @@ +// SPDX-FileCopyrightText: 2026 Bernard Ladenthin +// +// SPDX-License-Identifier: MIT + +package net.ladenthin.llama.server; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.fail; + +import java.io.File; +import java.io.IOException; +import java.net.ServerSocket; +import java.util.concurrent.TimeUnit; +import net.ladenthin.llama.ClaudeGenerated; +import net.ladenthin.llama.LlamaModel; +import net.ladenthin.llama.TestConstants; +import net.ladenthin.llama.parameters.InferenceParameters; +import net.ladenthin.llama.parameters.ModelParameters; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +/** + * Attach mode over a model that goes to idle sleep ({@code --sleep-idle-seconds}): the server is + * closed, the model sleeps and wakes, and a second server attaches to the same model. + * + *

The defect this pins: {@code llama_server_attach} ({@code patches/0007}) used to build a + * {@code server_routes} of its own on the attach worker's stack. Its constructor registers a + * sleeping-state callback on the model's task queue, which has no unregister, so after + * {@link NativeServer#close()} the model's next idle sleep ran that callback on a destroyed object + * (a lock on freed memory, a dangling {@code meta} pointer) -- undefined behaviour, in practice a + * crashed JVM. Since the fix the attached server serves the model's own {@code server_routes} + * ({@code jllama_context::routes}), which outlive every attach; the callback that runs on sleep is + * the model's. + * + *

The same change decides where the endpoint toggles come from: the routes are the model's, so + * {@code --metrics} / {@code --props} / {@code --slots} are {@link ModelParameters} now + * ({@link ModelParameters#enableMetricsEndpoint()}, ...), not attach arguments. The second server + * proves it by scraping {@code /metrics}, which upstream disables by default. + */ +@ClaudeGenerated( + purpose = "Pin that closing an attached NativeServer leaves nothing behind in the model's " + + "sleep callbacks: the model sleeps and wakes after the server is gone, a second " + + "server attaches, and the endpoint toggles come from the model's parameters.") +public class NativeServerAttachSleepIntegrationTest extends OpenAiServerTestSupport { + + /** Idle window before the model sleeps; short so the test does not dominate the suite. */ + private static final int SLEEP_IDLE_SECONDS = 1; + + /** Comfortably longer than {@link #SLEEP_IDLE_SECONDS}, so the sleep transition has run. */ + private static final long IDLE_WAIT_MILLIS = 3_000L; + + private static LlamaModel model; + + @BeforeAll + public static void setup() { + Assumptions.assumeTrue( + new File(TestConstants.DRAFT_MODEL_PATH).exists(), + "Draft model not found, skipping NativeServerAttachSleepIntegrationTest"); + int gpuLayers = Integer.getInteger(TestConstants.PROP_TEST_NGL, TestConstants.DEFAULT_TEST_NGL); + model = new LlamaModel(new ModelParameters() + .setModel(TestConstants.DRAFT_MODEL_PATH) + .setCtxSize(512) + .setGpuLayers(gpuLayers) + .setSleepIdleSeconds(SLEEP_IDLE_SECONDS) + .enableMetricsEndpoint()); + } + + @AfterAll + public static void tearDown() { + if (model != null) { + model.close(); + } + } + + private static int findFreePort() throws IOException { + try (ServerSocket socket = new ServerSocket(0)) { + return socket.getLocalPort(); + } + } + + private void awaitHealthy(int port) throws Exception { + long deadline = System.currentTimeMillis() + 30_000L; + IOException last = null; + while (System.currentTimeMillis() < deadline) { + try { + if (get(port, "/health", "").code == 200) { + return; + } + } catch (IOException e) { + last = e; + } + Thread.sleep(200L); + } + fail("attached server did not become healthy within 30s" + (last != null ? ": " + last : "")); + } + + /** + * Without the fix this method does not fail an assertion: the first idle sleep after the first + * server closed runs the dead callback, and the fork dies. The {@link Timeout} turns a hang into + * a failure; a crash is reported by surefire as a crashed fork. + */ + @Test + @Timeout(value = 180, unit = TimeUnit.SECONDS) + public void theModelSleepsAndWakesAfterTheAttachedServerClosed_andASecondServerAttaches() throws Exception { + int firstPort = findFreePort(); + try (NativeServer first = + new NativeServer(model, "--host", "127.0.0.1", "--port", Integer.toString(firstPort)).start()) { + awaitHealthy(firstPort); + Response completion = + post(firstPort, "/completion", "{\"prompt\":\"Hello\",\"n_predict\":2,\"temperature\":0}", ""); + assertThat(completion.body, completion.code, is(200)); + } + + // The model is idle now and crosses its threshold: the sleep transition runs every callback + // the queue holds -- with the defect, the one the first server left behind. + Thread.sleep(IDLE_WAIT_MILLIS); + String afterFirstSleep = model.complete(new InferenceParameters("Say hi.").withNPredict(2)); + assertNotNull(afterFirstSleep, "completion after the first sleep returned null"); + + int secondPort = findFreePort(); + try (NativeServer second = + new NativeServer(model, "--host", "127.0.0.1", "--port", Integer.toString(secondPort)).start()) { + awaitHealthy(secondPort); + Response props = get(secondPort, "/props", ""); + assertThat(props.body, props.code, is(200)); + assertThat(props.body, containsString("default_generation_settings")); + // The routes are the model's: --metrics came from ModelParameters, not from the attach argv. + Response metrics = get(secondPort, "/metrics", ""); + assertThat(metrics.body, metrics.code, is(200)); + assertThat(metrics.body, containsString("llamacpp:")); + } + + // A second sleep after the second server closed: two dead callbacks before the fix. + Thread.sleep(IDLE_WAIT_MILLIS); + String afterSecondSleep = model.complete(new InferenceParameters("And again.").withNPredict(2)); + assertNotNull(afterSecondSleep, "completion after the second sleep returned null"); + assertNotNull(model.getMetrics(), "getMetrics() must still be serviced after waking"); + } +} From 7f01f8544ccfd3fdabc88e0b24dde32487e26c21 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 10:59:01 +0000 Subject: [PATCH 5/5] Log sink: attach llama.cpp's log worker to the JVM once per thread, not once per log line LlamaModel.setLogger's trampoline runs on common_log's worker thread, a plain std::thread llama.cpp creates and re-creates on every pause/resume, which has never seen the JVM. Until now get_jni_env_attaching did AttachCurrentThread + DetachCurrentThread for every log line: leak-free, but each attach creates a java.lang.Thread object and fires the JVMTI thread events (TODO.md measured 13 lines of a failed load arriving on 13 distinct Thread objects), noticeable at --verbose volumes. A thread_local guard now records the attach and detaches in its destructor when the worker thread exits -- common_log::pause() joins the worker, a normal exit on which C++ TLS destructors run, also for a dlopen'd library (glibc keeps it mapped while TLS destructors are registered), and JNI_OnUnload pauses the worker through common_log_set_callback(nullptr) before anything else. The thread is attached as a daemon under the name jllama-log-worker (visible in thread dumps), so DestroyJavaVM never waits for the singleton logger's worker; a thread the JVM already owns is used as it is and never detached. Each delivery runs inside PushLocalFrame/PopLocalFrame, since a thread that stays attached and has no Java frame would otherwise accumulate local references until it exits. LlamaLoggerTest pins it (model-free, needs libjllama): every delivering thread is named jllama-log-worker, and N > 1 lines arrive on fewer than N distinct Thread objects. The per-line time cost was not measured: the model-free fixture logs too few lines for a number, and no model is available here. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01AytmJF9faEiQEVt6eetQS2 --- CHANGELOG.md | 7 ++ CLAUDE.md | 2 +- TODO.md | 15 ---- llama/src/main/cpp/jllama.cpp | 70 +++++++++++++------ .../net/ladenthin/llama/LlamaLoggerTest.java | 20 ++++-- 5 files changed, 70 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8df934428..85f5d7a22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,6 +83,13 @@ from version 5.0.0 onward. Pre-fork releases (`1.x`–`4.2.0`) were authored by core's; `check-natives.py` fails when they differ. ### Changed +- **The log sink attaches llama.cpp's log worker to the JVM once per thread, not once per line.** + `LlamaModel.setLogger`'s trampoline used to `AttachCurrentThread` + `DetachCurrentThread` for every + log line, i.e. one `java.lang.Thread` object and a pair of JVMTI thread events per line (13 lines of a + failed load, 13 `Thread` objects). A `thread_local` guard now keeps the attach (as a daemon thread + named `jllama-log-worker`, visible in thread dumps) until the worker thread exits, which + `common_log::pause()` makes a normal exit; each delivery runs in its own local-reference frame. + `LlamaLoggerTest` pins both. - **`cpu-windows-x86-64` is the CPU-variants build, built with clang** (`build-windows-x86_64`, `-DJLLAMA_CPU_VARIANTS=ON`): like the two Linux CPU jars since 5.2.0, the directory holds `jllama.dll`, `ggml.dll`, `ggml-base.dll`, `ggml-rpc.dll` and one `ggml-cpu-.dll` per diff --git a/CLAUDE.md b/CLAUDE.md index 6d9c05a03..8a2c017e6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1199,7 +1199,7 @@ Current patches: | `0008-server-models-worker-cmd-override.patch` | **Makes router mode usable in-JVM.** The router (`server-models.cpp`) spawns each model worker by re-executing its own binary (`get_server_exec_path()` = `/proc/self/exe` & friends) — inside a JVM that binary is `java`, not a llama-server, so embedded router workers could never start. The patch adds env `LLAMA_SERVER_WORKER_CMD` (whitespace-split; read in `server_model_meta::update_args`) which replaces only the leading binary-path token of the rendered worker args, letting an embedding host relaunch workers through its own bootstrap — e.g. `java -cp app.jar net.ladenthin.llama.server.NativeServer` (each worker is then a fresh JVM running the classic single-model `NativeServer`). Exposed in Java as `NativeServer.setWorkerCommand(String...)` (JNI `setenv`); exercised by `RouterModeIntegrationTest` (Linux CI). Upstream-submittable (also useful for containerized/wrapped deployments). **Refreshed at the b11429 bump:** #29987 (untagged `4d60b4d08`, the commit before b11429) moved `server_model_meta::update_args` and changed the function after it (`update_caps(const common_params &)`), the hunk's trailing context; the `+` lines are unchanged. Since b11401 (#29895) a worker is a router *child* whose stdout carries the state commands, which a JVM worker's early `System.out` line now trips (cosmetic, `TODO.md`). | | `0006-server-embed-native-server-jni.patch` | **Makes `server.cpp`'s `llama_server` embeddable in the JVM** so the `NativeServer` JNI bridge can run the full upstream HTTP server (WebUI included) inside `libjllama` — see "Two server modes" below. b9870 already exposes `int llama_server(int, char**)` (non-static; no `main` in the file), so the patch only adds embedded-mode support: (1) a `g_llama_server_embedded` flag + `llama_server_set_embedded()` / `llama_server_request_shutdown()` (declared in the committed `src/main/cpp/native_server_bridge.h`); (2) skips installing the process-wide SIGINT/SIGTERM handlers when embedded (they would hijack the JVM's); (3) in embedded mode parses the **forwarded** argv via `common_params_parse` instead of `common_params_parse_main` (whose `GetCommandLineW` recovery would pick up `java.exe`'s command line — the same Windows class of bug `0001` fixes). `llama_server_request_shutdown()` mirrors the SIGTERM path (invokes the installed `shutdown_handler` → `ctx_server.terminate()` unblocks `start_loop()`), giving JNI an out-of-band stop since `ctx_server` is loop-local. **The handler is guarded, and both guards are load-bearing:** upstream's `shutdown_handler` lambdas capture `llama_server()`'s locals (`ctx_http`, `models_routes`, `mcp_mgr`, `ctx_server`) by reference and are never cleared, while `native_server.cpp` signals *repeatedly* until the worker reports it finished (a stop issued before the handler is installed, or before `start_loop()` begins, would otherwise be lost). A request landing after `llama_server()` returned therefore ran the lambda over destroyed objects — a `SIGSEGV` in `server_http_context::stop()` in `RouterModeIntegrationTest.tearDown` (run 36485159455), with router mode's `clean_up()` (unloading workers) making the window wide. The patch (1) takes `g_shutdown_handler_mutex` for every read, write and invocation of the handler (the caller is on another thread) and (2) declares a `llama_server_shutdown_handler_reset` **after** the captured locals, so the handler is cleared before any of them is destroyed; because invocation holds the same lock, that reset waits for an invocation in progress. `0007`'s `llama_server_attach` carries the same guard. Runnable guard: `src/test/cpp/test_native_server_shutdown.cpp`. Applies **after `0001`** (which flips this call site to `common_params_parse_main`), so its context is the post-`0001` tree; regenerate against `0001`+source on a bump. Only touches `tools/server/server.cpp`. | | `0012-model-guard-zero-split-sum-and-name-the-device-index.patch` | **A GPU that reports zero free memory makes every model load fail with the unactionable `error loading model: vector`.** `llama_model_base::load_tensors` (`src/llama-model.cpp`) weights the per-device layer split by `ggml_backend_dev_memory()`'s `free`, then normalises: `splits[i] /= split_sum`. With a single device reporting `free == 0` that is `0/0` → **NaN** in every split point; NaN compares false against everything, so the `std::upper_bound` below returns the end iterator, `layer_gpu == n_devices()`, and `devices.at(layer_gpu)` throws `std::out_of_range` — whose libc++ `what()` is the bare string `"vector"`, which `llama.cpp`'s `catch (const std::exception &)` prints verbatim. Upstream's `free == 0 && total == 0` host-memory fallback does **not** fire, because `total` is `recommendedMaxWorkingSetSize` and is non-zero. **Reachable since b10618..b10797**: upstream `8c0b9cd04` ("metal : fix memory query under low-memory conditions", [#27701](https://github.com/ggml-org/llama.cpp/pull/27701)) changed `ggml-metal-device.m` to `*free = *total > cur ? *total - cur : 0`; before that clamp an over-committed device (`currentAllocatedSize > recommendedMaxWorkingSetSize`) *underflowed* to a huge `size_t`, which normalised fine, so the same precondition was harmless. That is why the `Java Tests macOS …` jobs went red at the b10792→b10797 step while every Linux/Windows job stayed green — **and why only a GPU build can fail this way at all**: `act_gpu_layers` is `devices.empty() ? 0 : …`, so with no GPU backend `devices` is empty, every layer returns early on `cpu_dev`, and the `.at()` line is unreachable. **Shape:** the two blocks are lifted out of `load_tensors` into free functions declared in `src/llama-model.h`, purely so they can be driven by a test — the failing state needs a real over-committed GPU and cannot be arranged through any public API. `llama_model_splits_normalize()` carries **the fix**: on `split_sum == 0` it `LLAMA_LOG_WARN`s and falls back to an even split (`splits[i] = float(i+1)/splits.size()`), the only neutral choice when no device can be preferred and exactly right for a single device. `llama_model_splits_select_device()` carries **the diagnostic**: it bounds-checks the index and throws a `std::runtime_error` naming the function, the offloaded layer, the device index, the split-point count **and the split points themselves** — with NaN splits that message prints `nan` and names the cause outright, which is precisely what was missing when this had to be diagnosed by reading source. **A second, backend-independent trigger reaches the same line**, found while writing this up and verified against the unfixed library: `--tensor-split` values are parsed with `std::stof` and never range-checked (`common/arg.cpp`), so `-ts 1,-1` cancels out, `split_sum` is 0 again, the split points become `[inf, -nan]`, and every layer maps one past the last device — on CUDA, Vulkan or ROCm just as much as on Metal, with no memory pressure involved. That is what makes this an ordinary upstream defect rather than a Metal edge case, and the warning names both causes rather than only the memory one. Also adds upstream `tests/test-model-split.cpp` (5 cases in upstream's `testing.h` style) + its `llama_build_and_test` registration. Touches `src/llama-model.{cpp,h}`, `tests/test-model-split.cpp` and `tests/CMakeLists.txt` — **none** of which any other patch touches, so it is independent of all of them. Upstream-submittable ("model: fall back to an even split when no device reports free memory"); **not yet filed upstream**. **Runnable guard: `src/test/cpp/test_model_split.cpp`** — a FetchContent subproject builds with `LLAMA_BUILD_TESTS=OFF`, so the upstream test above is applied-but-never-compiled here (same as `0001`'s test). That file drives the same two functions from `jllama_test`, which runs on **every** platform in `C++ Tests`, so a bump that drops this patch fails the build at link time everywhere instead of surfacing as one red macOS Java job. **Verification limit — read before assuming this can be dropped:** the *failing path* still cannot be reached without a GPU backend, so the guard pins the arithmetic (what actually broke), not the end-to-end load; the end-to-end proof is the macOS CI job. On a bump, re-check whether upstream added its own `split_sum == 0` guard (grep `split_sum` in `src/llama-model.cpp`) and **drop this patch rather than refreshing it** if they did — the fail-loud applier detects "does not apply", never "upstream already fixed this". | -| `0014-common-log-callback-sink.patch` | **Gives `common_log` a callback sink: `common_log_set_callback(log, cb, user_data)` (`common/log.{h,cpp}`).** This is what `LlamaModel.setLogger` hooks. Before it, the Java logger was a `llama_log_set()` callback, which has two holes, both found while chasing `slot print_timing` lines interleaving with the Atmosphere agent's streamed answer: **(a)** every model load runs `common_init()`, which re-points `llama_log_set()` at `common_log_default_callback` (`common.cpp:394`), so `setLogger(…)` *before* `new LlamaModel(…)` silently lost the callback; **(b)** the server's own `SRV_*`/`SLT_*` macros are `LOG_INF` and write straight into `common_log`, which `llama_log_set()` never carried, so the per-request `slot …`/`srv …` lines could not be routed to Java at all (the reason `LlamaModelTest#testLogText/JSON` sat `@Disabled` for years). `common_log` upstream offers file, colors, prefix, timestamps, verbosity and JSONL but no hook. The patch adds one: while a callback is set, the worker thread hands every entry to it **instead of** printing to stdout/stderr (a `--log-file` still receives them); the callback gets the bare formatted message (no prefix/timestamp/colors) with the `ggml_log_callback` signature; swapping the callback pauses the worker first, so queued entries reach the *previous* sink (which is what makes `setLogger(format, null)` a synchronous drain). With the sink, `common_init()`'s `llama_log_set()` reset is harmless — it points at the default callback that feeds `common_log`, i.e. exactly the path into the sink — so the ordering problem (a) disappears without any re-install logic, and the `srv`/`slot` lines (b) arrive because they are `common_log` entries. `jllama.cpp`'s `setLogger` therefore sets `common_log_set_callback(common_log_main(), trampoline)` **plus** `llama_log_set(common_log_default_callback)` (so llama/ggml lines feed `common_log` even before the first load). Two consequences to know: the callback runs on `common_log`'s **worker thread**, a plain `std::thread` llama.cpp re-creates on every pause/resume and never attaches to the JVM — the trampoline attaches per call and detaches again (`get_jni_env_attaching`; a thread that exits while attached leaks a `JavaThread`, and this thread is not ours — the leak-free choice, not the cheapest: each attach creates a `java.lang.Thread` object, so a `thread_local` guard that detaches once at thread exit is the optimisation on file in `TODO.md`), `setLogger` must call `common_log_set_callback` **outside** `g_log_mutex`, because the pause joins the worker, which needs that mutex to read the callback, and `setLogger` callers are serialized by a **separate** `g_set_logger_mutex`: two unserialized swaps race on the worker's `std::thread` (one joins it while the other assigns a fresh thread over the still-joinable object = `std::terminate`, the whole JVM), which `LlamaLoggerTest#concurrentSetLoggerCallsDoNotRaceOnTheLogWorker` reproduced before the mutex existed. Two caveats the Javadoc carries: a caller must not hold a lock the *previous* callback needs (the drain runs it on the worker while the caller waits), and the verbosity threshold is process-wide and reset by **every** load (`common_params_parse` ends with `common_log_set_verbosity_thold(params.verbosity)`, default 3), so a load without `-lv` puts it back to 3 — a review assumed the opposite, and `LlamaLoggerTest#verbosityThresholdIsProcessWideAndEveryLoadSetsIt` now pins the measured behaviour. And the verbosity threshold applies *before* the sink: at the default (`3`) the Java logger sees errors, warnings and the server's INFO lines, while llama/ggml INFO lines (`common_log_get_verbosity` maps them to TRACE = 4) arrive only from `setLogVerbosity(4)` on — the same filtering the console gets, and a behaviour change for consumers who captured the unfiltered `llama_log_set()` stream before. **Runnable guards:** `src/test/cpp/test_common_log_callback.cpp` (6 tests over a private `common_log_init()` instance: delivery, bare text under prefix+timestamps, clear, swap-drains-to-old-sink, file kept, levels pass through) links the function on every platform, so a bump that drops the patch reds `C++ Tests` at link time; `LlamaLoggerTest` (model-free, needs only `libjllama`: a logger set before a deliberately failing load on a non-GGUF file sees the `srv … loading model` INFO line and llama's ERROR line, in TEXT and JSON) and the re-enabled `LlamaModelTest#testLogText/testLogJSON` plus `#testLoggerSetBeforeLoadSurvivesTheLoad` (vocab-only load) cover the Java side. Upstream-submittable ("common : add a callback sink to common_log for embedding hosts"); **not yet filed upstream**. Touches only `common/log.{h,cpp}`, which no other patch touches. **Refreshed at the b11401 bump** (#29895 adds a `colors` member next to the sink's members and a `get_colors()` next to `set_callback()`; only context moved, the `+`/`-` lines are unchanged, and upstream still has no hook -- it added `common_log_get_colors` only). **On a bump, check whether upstream added a hook of its own (grep `callback` in `common/log.h`) and, if so, DROP this patch and port `setLogger` to theirs rather than refreshing it.** | +| `0014-common-log-callback-sink.patch` | **Gives `common_log` a callback sink: `common_log_set_callback(log, cb, user_data)` (`common/log.{h,cpp}`).** This is what `LlamaModel.setLogger` hooks. Before it, the Java logger was a `llama_log_set()` callback, which has two holes, both found while chasing `slot print_timing` lines interleaving with the Atmosphere agent's streamed answer: **(a)** every model load runs `common_init()`, which re-points `llama_log_set()` at `common_log_default_callback` (`common.cpp:394`), so `setLogger(…)` *before* `new LlamaModel(…)` silently lost the callback; **(b)** the server's own `SRV_*`/`SLT_*` macros are `LOG_INF` and write straight into `common_log`, which `llama_log_set()` never carried, so the per-request `slot …`/`srv …` lines could not be routed to Java at all (the reason `LlamaModelTest#testLogText/JSON` sat `@Disabled` for years). `common_log` upstream offers file, colors, prefix, timestamps, verbosity and JSONL but no hook. The patch adds one: while a callback is set, the worker thread hands every entry to it **instead of** printing to stdout/stderr (a `--log-file` still receives them); the callback gets the bare formatted message (no prefix/timestamp/colors) with the `ggml_log_callback` signature; swapping the callback pauses the worker first, so queued entries reach the *previous* sink (which is what makes `setLogger(format, null)` a synchronous drain). With the sink, `common_init()`'s `llama_log_set()` reset is harmless — it points at the default callback that feeds `common_log`, i.e. exactly the path into the sink — so the ordering problem (a) disappears without any re-install logic, and the `srv`/`slot` lines (b) arrive because they are `common_log` entries. `jllama.cpp`'s `setLogger` therefore sets `common_log_set_callback(common_log_main(), trampoline)` **plus** `llama_log_set(common_log_default_callback)` (so llama/ggml lines feed `common_log` even before the first load). Two consequences to know: the callback runs on `common_log`'s **worker thread**, a plain `std::thread` llama.cpp re-creates on every pause/resume and never attaches to the JVM — the trampoline attaches it **once per worker thread** as a daemon, named `jllama-log-worker`, and a `thread_local` guard detaches it when the thread exits (`get_jni_env_attaching`; `common_log::pause()` joins the worker, a normal exit on which TLS destructors run, and `JNI_OnUnload` pauses it before anything else). Until 5.2.x it attached and detached per line, which created a `java.lang.Thread` object per log line (13 lines of a failed load, 13 `Thread` objects -- `LlamaLoggerTest` pins the single attach now). Each call runs inside a `PushLocalFrame`/`PopLocalFrame` pair, because a thread that stays attached and has no Java frame would otherwise accumulate local references, `setLogger` must call `common_log_set_callback` **outside** `g_log_mutex`, because the pause joins the worker, which needs that mutex to read the callback, and `setLogger` callers are serialized by a **separate** `g_set_logger_mutex`: two unserialized swaps race on the worker's `std::thread` (one joins it while the other assigns a fresh thread over the still-joinable object = `std::terminate`, the whole JVM), which `LlamaLoggerTest#concurrentSetLoggerCallsDoNotRaceOnTheLogWorker` reproduced before the mutex existed. Two caveats the Javadoc carries: a caller must not hold a lock the *previous* callback needs (the drain runs it on the worker while the caller waits), and the verbosity threshold is process-wide and reset by **every** load (`common_params_parse` ends with `common_log_set_verbosity_thold(params.verbosity)`, default 3), so a load without `-lv` puts it back to 3 — a review assumed the opposite, and `LlamaLoggerTest#verbosityThresholdIsProcessWideAndEveryLoadSetsIt` now pins the measured behaviour. And the verbosity threshold applies *before* the sink: at the default (`3`) the Java logger sees errors, warnings and the server's INFO lines, while llama/ggml INFO lines (`common_log_get_verbosity` maps them to TRACE = 4) arrive only from `setLogVerbosity(4)` on — the same filtering the console gets, and a behaviour change for consumers who captured the unfiltered `llama_log_set()` stream before. **Runnable guards:** `src/test/cpp/test_common_log_callback.cpp` (6 tests over a private `common_log_init()` instance: delivery, bare text under prefix+timestamps, clear, swap-drains-to-old-sink, file kept, levels pass through) links the function on every platform, so a bump that drops the patch reds `C++ Tests` at link time; `LlamaLoggerTest` (model-free, needs only `libjllama`: a logger set before a deliberately failing load on a non-GGUF file sees the `srv … loading model` INFO line and llama's ERROR line, in TEXT and JSON) and the re-enabled `LlamaModelTest#testLogText/testLogJSON` plus `#testLoggerSetBeforeLoadSurvivesTheLoad` (vocab-only load) cover the Java side. Upstream-submittable ("common : add a callback sink to common_log for embedding hosts"); **not yet filed upstream**. Touches only `common/log.{h,cpp}`, which no other patch touches. **Refreshed at the b11401 bump** (#29895 adds a `colors` member next to the sink's members and a `get_colors()` next to `set_callback()`; only context moved, the `+`/`-` lines are unchanged, and upstream still has no hook -- it added `common_log_get_colors` only). **On a bump, check whether upstream added a hook of its own (grep `callback` in `common/log.h`) and, if so, DROP this patch and port `setLogger` to theirs rather than refreshing it.** | | `0015-rpc-embeddable-client-and-stoppable-server.patch` | **Makes ggml-rpc usable inside a JVM** (see "RPC backend" below). Upstream treats every client-side RPC problem as `GGML_ABORT` — a malformed endpoint, a server that is not running, a failed handshake — which in a JVM kills the application over a typo in `--rpc`. **(1)** The *registration* path reports failure instead: `rpc_dispatcher::start()` returns `false`, `try_get_dispatcher()` `nullptr`, `ggml_backend_rpc_get_device_count()` `0`, `ggml_backend_rpc_add_server()` `nullptr`, and `add_rpc_devices()` (`common/arg.cpp`) throws `std::invalid_argument` naming the server where it used to register `nullptr` and silently run without it. Every path *after* registration keeps the original contract through `get_dispatcher()`, which still aborts — a server that vanishes mid-inference stays fatal (TODO). **(2)** `add_server()`'s cache hit is re-checked (a stopped server used to pass registration and abort on the first tensor upload), and `ggml_backend_rpc_get_device_memory()` of a gone server returns 0/0 instead of aborting — RPC devices stay in ggml's process-wide registry forever, and `common_init()` queries the memory of **every** registered device at `-lv 4`, so a stopped server would otherwise take down an unrelated later load. **(3)** The server becomes stoppable: `ggml_backend_rpc_stop_server()` (disconnects the client being served, wakes `accept()` with a loopback connection — the one portable way) and `ggml_backend_rpc_server_listening()`; the loop now reaches its cleanup and frees its backends on every early return, but deliberately **not** `rpc_transport_shutdown()`, whose `WSACleanup()` would invalidate every other RPC socket in the process. **(4)** transport: `MSG_NOSIGNAL`/`SO_NOSIGPIPE`, `socket_t::shutdown()`, and the fds `connect()`/`create_server()`/`accept()` leaked on their error paths are closed. Touches `ggml/include/ggml-rpc.h`, `ggml/src/ggml-rpc/{ggml-rpc.cpp,transport.cpp,transport.h}` and one function in `common/arg.cpp` (`add_rpc_devices`, away from `0001`'s `common_params_parse` hunks). **Runnable guard: `src/test/cpp/test_rpc.cpp`** — links both new functions (a bump that drops the patch fails `C++ Tests` at link time on every platform) and drives registration, stop and the memory query over loopback. Upstream-submittable; **not yet filed upstream**. On a bump, check whether upstream grew its own stop function or non-aborting registration (grep `stop_server` / `GGML_ABORT("Failed to connect` in `ggml-rpc.cpp`). **Refreshed at the b11450 bump** (#26610, `-sm tensor` over RPC, ~800 lines in `ggml-rpc.cpp`, protocol 7 → 8): three hunks lost their context -- new dispatcher members beside `start()`, and new `get_proc_address` names before `return NULL` (the two new names now sit after `ggml_backend_comm_allreduce_tensor`); the `+`/`-` lines are unchanged. The server-to-server comm #26610 adds (a listener on `0.0.0.0`, an `accept()` a stop does not wake) is on file in `TODO.md`. | | `0017-ggml-cpu-x86-prefetch-byte-offset-and-msvc-type.patch` | **Makes `GGML_CPU_ALL_VARIANTS=ON` buildable with plain clang on Windows, and fixes a prefetch address that is wrong everywhere.** The four `_mm_prefetch` calls in `ggml/src/ggml-cpu/arch/x86/quants.c` (the SSSE3-without-AVX branch of `ggml_vec_dot_q4_0_q8_0`, and the only `_mm_prefetch` calls in the whole ggml tree) carry two independent defects; both are fixed by routing the address through one new `static inline` helper in the new header `ggml/src/ggml-cpu/arch/x86/prefetch.h`, which is included relatively, so **no CMake change is needed**. **(1) Type -- fatal only on a current clang:** MSVC declares the intrinsic as `void _mm_prefetch(char const *, int)`; clang's own `xmmintrin.h` defines a casting macro but only `#ifndef _MSC_VER`, so on a `*-windows-msvc` target clang leaves the declaration to Microsoft and a typed pointer is `-Wincompatible-pointer-types`. **clang 22 promoted that to an error by default** ([llvm-project #157364](https://github.com/llvm/llvm-project/pull/157364); the [22.1.0 release notes](https://releases.llvm.org/22.1.0/tools/clang/docs/ReleaseNotes.html) add that `-Wno-error=incompatible-pointer-types` downgrades it) -- *not* clang 16, which promoted only `-Wincompatible-FUNCTION-pointer-types`; an earlier version of this row said otherwise. Measured on the real file at `-msse4.2`, unpatched → patched: clang **20.1.8** 4 warnings → 0/0, clang **22.1.3** 4 errors → 0/0, clang **23.1.3** 4 errors → 0/0. Plain clang is exactly what `cmake/x64-windows-llvm.cmake` selects -- and plain clang is the only way to get all **14** CPU variants, because `clang-cl` makes CMake's `MSVC` true and the `if (NOT MSVC)` block in `ggml/src/CMakeLists.txt` then drops `zen4`, `cooperlake`, `sapphirerapids`, `ivybridge` and `piledriver`. **(2) Arithmetic:** `&x[ib] + sizeof(block_q4_0)` adds `sizeof()` *elements*, not bytes -- +324 and +1156 instead of +18 and +34 (the sizes `ggml-common.h` pins by `static_assert`), and +648 instead of +36 for the `2 *` pair. That half is wrong on every platform, including where the call compiles. **Measured:** no throughput difference (see the test-table row), so this is a correctness and portability fix, not a performance one. **Upstream-submittable and meant to be submitted.** **Why upstream's own build is green is measured, not an open question:** their `windows-cpu` release job builds exactly this with plain clang, `GGML_CPU_ALL_VARIANTS=ON` and no suppressing flag (`CMAKE_ARGS` carries only build-target toggles), and ships `ggml-cpu-sse42.dll` -- because the `windows-2025-vs2026` image has **LLVM 20.1.8** on `PATH`, where these are warnings. That is narrower than it looks: the same image also ships **clang 22.1.3** inside Visual Studio 2026 (`VC/Tools/Llvm/x64`), which *fails* on the unpatched file -- upstream reaches the working compiler only because `C:\Program Files/LLVM/bin` comes first on `PATH`. It is **not** a header difference: all three MSVC toolsets on that image (14.29.30133, 14.44.35207, 14.51.36231) declare it identically as `extern void _mm_prefetch(char const*_A, int _Sel);` -- that was the first hypothesis, and a one-off probe workflow (deleted again once it had answered) disproved it by printing the declaration from each toolset on the image. So this patch is **ahead of** upstream rather than diverging: their build breaks the moment the image ships a clang that makes the diagnostic fatal, and until then they compile four warnings and a prefetch address 18x/34x too far. **Runnable guard: `src/test/cpp/test_prefetch.cpp`.** On a bump, run `git grep -n _mm_prefetch ggml/src/ggml-cpu/arch/x86/quants.c` at the new tag: if the call sites already cast **before** the addition, **drop this patch rather than refreshing it**. | | `0016-model-kolibri1.patch` | **Adds Aleph Alpha's Kolibri-1 (architecture `kolibri1`) ahead of upstream** -- a 78B German/English reasoning MoE (3.46B active) that upstream does not support yet ([ggml-org/llama.cpp#29922](https://github.com/ggml-org/llama.cpp/issues/29922), no PR at the time). A **temporary carry, not a fix**: drop it the moment upstream registers the architecture (`git grep -n kolibri src/llama-arch.cpp` at the new tag), and keep `test_kolibri1.cpp` -- its numerical comparisons must then pass against upstream's implementation, while its GGUF-format rows follow whatever format upstream's converter fixes (see the test-table row). **Architecture** (Aleph Alpha's vLLM reference, `aleph_alpha_inference/kolibri1.py` at `049a6a7`): GQA with per-head q/k RMSNorm; `layer_types` interleaves sliding-window layers (NEOX RoPE) with full-attention layers that have **no** positional encoding; sandwich norms around attention and MoE; every layer routed MoE + one ungated shared expert; the router selects top-k on **`logits + expert_bias`** and weights by the **unbiased `sigmoid(logits)`** -- not DeepSeek-V3's sigmoid router (selection on `sigmoid(logits) + bias`), which picks other experts as soon as the bias is non-zero. The graph therefore builds the selection itself and passes it to `build_moe_ffn` (`selected_experts_in`), leaving upstream's shared MoE code untouched. **Derived from two community ports, which are incompatible with each other's GGUFs** -- Eliasfpv28's AFMoE-based one (base b11378, `expert_gating_func` 2, pre-tokenizer `qwen2`) and the Qwen3-MoE-based one behind Hob-forge's GGUFs (base b11381, as carried in [mjwsolo/localcode#101](https://github.com/mjwsolo/localcode/pull/101): gating 5, pre-tokenizer `kolibri1`, rejects gating 2). This patch **loads both dialects**: gating 2, 5 or absent (all mean the one router), `kolibri1` mapped to the Qwen2 pre-tokenizer in `llama-vocab.cpp`, `expert_weights_norm` / `expert_shared_count` / `output.weight` optional. Every source, with license, is listed in the patch header; `REUSE.toml` annotates the file `MIT AND Apache-2.0` (Eliasfpv28's additions are Apache-2.0). Touches `src/llama-arch.{h,cpp}`, `src/llama-model.cpp` (mapping + NEOX rope list), `src/llama-vocab.cpp` (one name), `src/models/models.h` and the new `src/models/kolibri1.cpp`; no converter. **Runnable guard: `src/test/cpp/test_kolibri1.cpp`** (writes tiny random GGUFs in both dialects and compares every logit with an independent double-precision reference). **Not verified here:** the real 78B model (HuggingFace is unreachable from the sessions this was written in, and the smallest GGUF is 28.6 GB) and GPU backends -- the community ports report real-model checks (top-1 67/67 against a float64 reference for Q8_0, tool calls, German text). | diff --git a/TODO.md b/TODO.md index 0488dc7af..614db694d 100644 --- a/TODO.md +++ b/TODO.md @@ -251,21 +251,6 @@ module-beside-the-CPU-set experiment in its addenda 2 and 3): ### Logging sink (`patches/0014`) — follow-ups -- **Keep the log worker attached instead of attaching per line.** `LlamaModel.setLogger`'s trampoline - runs on `common_log`'s worker thread, which llama.cpp creates (and re-creates on every - pause/resume) and which is not ours; today `get_jni_env_attaching` does `AttachCurrentThread` + - `DetachCurrentThread` **per log line**. Leak-free and simple, but every attach creates a - `java.lang.Thread` object and fires JVMTI `ThreadStart`/`ThreadEnd`, which is noticeable at - `--verbose` volumes and makes profilers/debuggers crawl. The cheaper shape is a `thread_local` - guard object whose destructor detaches once at thread exit (C++ TLS destructors run on normal - thread exit on glibc/macOS/MSVC, including for a `dlopen`'d library, and `std::thread::join` in - `common_log::pause()` is a normal exit). Caveats to design in: attach as daemon - (`AttachCurrentThreadAsDaemon`, so `DestroyJavaVM` never waits for the leaked singleton's worker), - skip the detach when `g_vm` is already gone (`JNI_OnUnload` ran), and pin the behaviour with the - existing model-free `LlamaLoggerTest` plus a count of `java.lang.Thread` objects seen by the - callback — `deliveryIsAsynchronousOnTheLogWorkerAndRemovingTheLoggerDrains` already prints it - (measured: 13 lines of a failed load → 13 distinct `Thread` objects, i.e. one per line). Not a - correctness issue; measure the time cost before doing it. - **File the patch upstream.** `common_log_set_callback` is a small, self-contained addition to `common/log.{h,cpp}` with no jllama specifics; upstream acceptance would retire the carry. diff --git a/llama/src/main/cpp/jllama.cpp b/llama/src/main/cpp/jllama.cpp index 8f6d0f3d4..01083c532 100644 --- a/llama/src/main/cpp/jllama.cpp +++ b/llama/src/main/cpp/jllama.cpp @@ -672,20 +672,36 @@ JNIEnv *get_jni_env_or_null() noexcept { } /** - * A JNIEnv for the current thread, attaching the thread to the JVM if it is not attached yet. + * A JNIEnv for the current thread, attaching the thread to the JVM once if it is not attached yet. * - * The log sink runs on common_log's worker thread, a plain std::thread that llama.cpp creates - * (and re-creates on every pause/resume) and that has never seen the JVM. The thread is not ours, - * and a thread that exits while attached leaves a dangling JavaThread behind, so this attaches per - * call and the caller detaches again. That is the simple, leak-free choice, not the cheapest one: - * every attach creates a java.lang.Thread object (and fires JVMTI thread events), which is - * noticeable at --verbose volumes. A thread_local guard whose destructor detaches once at thread - * exit would keep the thread attached across lines; see TODO.md. Returns nullptr (nothing to log - * through) when the JVM is gone or refuses the attach. `attached` tells the caller whether it owes - * a DetachCurrentThread. + * The log sink runs on common_log's worker thread, a plain std::thread that llama.cpp creates (and + * re-creates on every pause/resume) and that has never seen the JVM. Until 5.2.x the trampoline + * attached and detached per log line -- leak-free, but every attach creates a java.lang.Thread + * object and fires the JVMTI thread events (measured: 13 lines of a failed load, 13 Thread objects), + * which is noticeable at --verbose volumes. Now the attach lasts for the life of the thread: a + * thread_local guard records it, and its destructor detaches when the thread exits. + * common_log::pause() joins the worker, a normal exit on which C++ TLS destructors run (glibc, + * macOS, MSVC -- also for a dlopen'd library, whose registered TLS destructors keep it mapped until + * those threads are gone, and JNI_OnUnload joins the worker through common_log_set_callback(nullptr) + * before anything else). Attached as a daemon, so DestroyJavaVM never waits for the singleton + * logger's worker; should the worker outlive the VM (the static common_log is destroyed at process + * exit, after DestroyJavaVM), HotSpot's DetachCurrentThread returns as soon as it sees the VM has + * exited. A thread the JVM already owns (GetEnv == JNI_OK) is used as it is and never detached. + * Returns nullptr (nothing to log through) when the JVM is gone or refuses the attach. */ -JNIEnv *get_jni_env_attaching(bool &attached) noexcept { - attached = false; +namespace { +struct jni_thread_attachment { + bool attached = false; + ~jni_thread_attachment() { + if (attached && g_vm != nullptr) { + g_vm->DetachCurrentThread(); + } + } +}; +thread_local jni_thread_attachment t_jni_attachment; +} // namespace + +JNIEnv *get_jni_env_attaching() noexcept { JNIEnv *env = nullptr; if (g_vm == nullptr) { return nullptr; @@ -694,11 +710,16 @@ JNIEnv *get_jni_env_attaching(bool &attached) noexcept { if (res == JNI_OK) { return env; } - if (res == JNI_EDETACHED && g_vm->AttachCurrentThread(reinterpret_cast(&env), nullptr) == JNI_OK) { - attached = true; - return env; + if (res != JNI_EDETACHED) { + return nullptr; + } + char name[] = "jllama-log-worker"; // the JVM copies it; shows in thread dumps instead of Thread-N + JavaVMAttachArgs args = {JNI_VERSION_1_6, name, nullptr}; + if (g_vm->AttachCurrentThreadAsDaemon(reinterpret_cast(&env), &args) != JNI_OK) { + return nullptr; } - return nullptr; + t_jni_attachment.attached = true; + return env; } bool log_json; @@ -1791,12 +1812,18 @@ JNIEXPORT void JNICALL Java_net_ladenthin_llama_LlamaModel_setLogger(JNIEnv *env jobject cb_ref = o_log_callback; log_callback = [cb_ref](enum ggml_log_level level, const char *text, void *user_data) noexcept { // common_log delivers from its own worker thread, which is not attached to the - // JVM; attach for the call and detach again (see get_jni_env_attaching). - bool attached = false; - JNIEnv *env = get_jni_env_attaching(attached); + // JVM; it is attached once per worker thread and detached when that thread exits + // (see get_jni_env_attaching). + JNIEnv *env = get_jni_env_attaching(); if (env == nullptr || text == nullptr) { return; } + // The thread stays attached and has no Java frame, so local references would + // accumulate until it exits: scope every call's references to a local frame. + if (env->PushLocalFrame(4) != JNI_OK) { + env->ExceptionClear(); // out of memory; drop this log line + return; + } // Log lines can embed payload text (prompts, model metadata), so the // message must cross as standard UTF-8, not Modified UTF-8. jstring message = utf8_to_jstring(env, text); @@ -1808,11 +1835,8 @@ JNIEXPORT void JNICALL Java_net_ladenthin_llama_LlamaModel_setLogger(JNIEnv *env if (env->ExceptionCheck()) { env->ExceptionClear(); // a throwing logger must not poison the worker } - env->DeleteLocalRef(message); - } - if (attached) { - g_vm->DetachCurrentThread(); } + env->PopLocalFrame(nullptr); }; } } diff --git a/llama/src/test/java/net/ladenthin/llama/LlamaLoggerTest.java b/llama/src/test/java/net/ladenthin/llama/LlamaLoggerTest.java index 3b33b190d..2967f5dfe 100644 --- a/llama/src/test/java/net/ladenthin/llama/LlamaLoggerTest.java +++ b/llama/src/test/java/net/ladenthin/llama/LlamaLoggerTest.java @@ -10,6 +10,7 @@ import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -243,9 +244,11 @@ void verbosityThresholdIsProcessWideAndEveryLoadSetsIt() throws IOException { * Messages are delivered on llama.cpp's log worker thread, never on the thread that logged * or the one that installed the logger; and removing the logger returns only after every queued * message has been delivered. Both are the facts behind the two deadlock rules in the Javadoc - * (no {@code setLogger} from a callback; no lock held that the previous callback needs). The - * distinct-thread count is recorded, not pinned: today every line attaches the worker afresh - * (one {@code java.lang.Thread} per line), a {@code thread_local} guard would make it one. + * (no {@code setLogger} from a callback; no lock held that the previous callback needs). And the + * worker is attached to the JVM once per thread, not once per line: before 5.2.x every line did + * {@code AttachCurrentThread} + {@code DetachCurrentThread}, i.e. 13 lines of a failed load arrived + * on 13 distinct {@code java.lang.Thread} objects; now a {@code thread_local} guard keeps the + * attach until the worker thread exits, and the thread carries the name {@code jllama-log-worker}. */ @Test void deliveryIsAsynchronousOnTheLogWorkerAndRemovingTheLoggerDrains() throws Exception { @@ -267,9 +270,16 @@ void deliveryIsAsynchronousOnTheLogWorkerAndRemovingTheLoggerDrains() throws Exc assertThat("nothing may arrive after setLogger(format, null) returned", delivered.get(), is(atReturn)); assertThat("delivery never runs on the caller's thread", deliveringThreads.contains(caller), is(false)); assertThat( - "every delivering thread is a native-attached one, not a Java-created one", - deliveringThreads.stream().allMatch(t -> t.getName().startsWith("Thread-")), + "every delivering thread is the natively attached log worker, not a Java-created thread: " + + deliveringThreads, + deliveringThreads.stream().allMatch(t -> t.getName().equals("jllama-log-worker")), is(true)); + if (atReturn > 1) { + assertThat( + "the worker is attached once per thread, not once per line (" + atReturn + " lines)", + deliveringThreads.size(), + lessThan(atReturn)); + } System.out.println("[LlamaLoggerTest] " + atReturn + " lines delivered on " + deliveringThreads.size() + " distinct Thread object(s)"); }