Repository navigation
Fix Linux support: by-value struct ABI, pinned llama.cpp b11146, stack-safe C buffers - #2
Merged
Conversation
- Pass llama_model_params / llama_context_params / llama_batch by value on non-Windows via purego (abi_unix.go); Windows keeps pointer path. - Context params layout matches llama.cpp b11146; offset test added. - build.sh: pin llama.cpp b11146, .tar.gz assets, keep symlinks, work from any cwd, safer lib replacement. - Load ggml backends with per-OS library names. - JANUS_GPU_LAYERS=-1 now offloads all layers. - purego v0.11.1 (Go 1.25+). Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ndor llama.h - llama_*_default_params write into a stack buffer: bind them (and the token_to_piece / chat_apply_template output buffers) as unsafe.Pointer so the runtime can adjust the pointer if the goroutine stack moves (n_batch used to come back 0 from fresh goroutines). Add a regression test. - Replace headers/llama.h with the verbatim b11146 header. - build.ps1: pin b11146 by default (override with -LlamaVersion). - Drop orphaned UnsafePtr, stale LD_LIBRARY_PATH hint, Go 1.21+ in CONTRIBUTING; restore the ollama hint in main.go. Co-Authored-By: Claude Opus 5.5 <[email protected]>
tmplC, chat and the token buffer were passed to purego as uintptr; escape analysis showed some of them could live on the goroutine stack, which can move during the call. Bind them as unsafe.Pointer (they now escape to the heap). Fix stale comment and the ggerganov URL in build.ps1. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Owner
|
Thank you so much for this, @thalisantunes! Merged. 🎉 I really appreciate the time you put into getting Janus running on Linux, and the detailed write-up made it easy to follow. I'm not on Linux very often myself, so any further fixes or improvements you'd like to make, on Linux or anywhere else, would be gladly appreciated. Feel free to open more PRs or issues anytime. Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Janus currently builds on Linux but cannot load a model there. This PR makes the Vulkan runner work on Linux/amd64 and keeps the Windows path compatible. Tested end to end on an NVIDIA RTX 5060 Ti (16 GB) via Vulkan.
What was broken on Linux
llama_load_model_from_file,llama_new_context_with_modelandllama_decodetake their params/batch structs by value. Janus passed a pointer (uintptr). That happens to match the Windows x64 ABI (large structs go by hidden pointer) but not SysV amd64, so on Linux model/context creation failed (n_batch and n_ubatch cannot both be zero, NULL context).build.shdownload. The llama.cpp "latest" release is nowv0.5.0, which has no binaries. Binary releases usebNNNNNtags and ship.tar.gz(not.zip) on Linux.ggml_backend_load_allwas only looked up inggml.dll, so no backend (including Vulkan) registered on Linux.JANUS_GPU_LAYERS=-1depended on the library default.-1meant "keep the lib default", but that default changed across llama.cpp versions. With b5400 (default 0) the README config loggedoffloaded 0/29 layers to GPUand ran on CPU while reporting[vulkan].llama_context_paramswas written against an olderllama.h, withn_threadsat offset 16. Current llama.cpp (b11146, and also b11326/b11374, whichbuild.ps1was fetching) addsn_rs_seq,n_outputs_maxandn_outputs_max_per_seqfirst, which movesn_threadsto offset 28. So the thread count ended up in other fields.Changes
internal/bridge/abi_unix.go,abi_windows.go): non-Windows registers the three functions by value through purego and wraps them behind the existing pointer-taking fields. Windows keeps the direct pointer binding, which is exact for Win64. The threeLlamaLibfields (LoadModelFromFile,NewContextWithModel,Decode) now take*LlamaModelParams/*LlamaContextParams/*LlamaBatch. TheUnsafePtrhelper andbatchState.ptr()are removed because they no longer have callers.LlamaContextParamsSizegoes from 512 to 192 bytes, which is purego's limit of 26 stack words on amd64; the real struct is 160 bytes.golang.org/x/sysrequire is dropped (go mod tidyno longer needs it).b11146in bothbuild.shandbuild.ps1(override with--version/-LlamaVersion). The Go struct mirrors match that tag'sllama.h.internal/bridge/headers/llama.his replaced with the b11146 header, andllama_dl_test.golocks the offsets.*_default_paramssret buffers,TokenToPiece,Tokenize,ChatApplyTemplate) were passed asuintptrof goroutine-stack memory. The goroutine stack can move during the purego call, so C wrote to or read from stale memory. Reproduced against the b11146 lib: 150/200 calls from fresh goroutines returnedn_batch=0. This hits/models/load(hot-swap runs on an HTTP goroutine). These are nowunsafe.Pointer, and a regression test (llama_defaults_test.go, non-Windows) runs when the lib is present.libggml.sowithlibggml-base.soas fallback,libggml.dylib,ggml.dll.JANUS_GPU_LAYERS < 0now maps to all layers. With the default config this should also make the existing context-OOM → CPU retry reachable (thenLayers > 0guard); not exercised in testing.build.shandbuild.ps1now point atggml-org/llama.cpp, whereggerganov/llama.cppredirects.build.sh: works from any cwd, extracts.tar.gz, preserves.sosymlinks (libllamaneedslibggml.so.0), replaces old libs only after the new ones are found, no longer runsgo mod tidy, drops theLD_LIBRARY_PATHhint (the libs resolve throughRUNPATH=$ORIGIN), and is now executable.Testing (Linux/amd64, RTX 5060 Ti, Vulkan, llama.cpp b11146)
./build.shfrom a clean checkout, run from another cwd: OK, and the tracked tree stays clean.go vet ./...(linux andGOOS=windows),GOOS=windows GOARCH=amd64 go build ./...,go test -count=1 ./...: pass.offloaded 29/29 layers to GPU,n_threads=32./health,/health?deep=true,/v1/modelsand/v1/chat/completions(normal andstream:true) all work. Hot-swap via/models/loadcreates the context withn_batch=2048.JANUS_GPU_LAYERS=0: works.llama-benchtg128 88.7 tok/s on the same lib. The gap predates this PR; it looks like per-token overhead in the generation loop, but I have not profiled it.Not covered
go vet/go buildfor windows/amd64 were run. The purego bump and theunsafe.Pointerchanges touch shared code.*_default_paramshidden return pointer goes inx8on arm64, which this binding does not handle.llama_load_model_from_filereturns NULL because VRAM runs out while loading weights, there is still no CPU fallback. Left for a follow-up.🤖 Generated with Claude Code