Skip to content

common: add json.h abstraction - #27511

Merged
ngxson merged 20 commits into
masterfrom
xsn/common_json
Aug 22, 2026
Merged

common: add json.h abstraction#27511
ngxson merged 20 commits into
masterfrom
xsn/common_json

Conversation

@ngxson

@ngxson ngxson commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Overview

Add a light-weight common/json.h to avoid re-compiling json.hpp in multiple places. Also allow swapping another pimpl in the future if needed

Goals:

  • pimpl nlohmann::json
  • least changes to downstream code (almost drop-in replacement)

Changes to downstream code:

  • Rename nlohmann::json to common_json
  • obj.push_back({key, val}) is changed to obj[key] = val for clarify; push_back can be confused with std::vector in some places
  • Some places having weird std::something<...> = obj["abc"], change to explicit = obj.get<std::something<...>>() to avoid excessive template instances

Results

End-to-end (compile common + server, exclude libllama/libmtmd, -j12):

  • wall: 18.9s -> 15.5s (-17.9%)
  • CPU (user): 98.3s -> 79.8s (-18.8%)

Single-TU dev loop (touch tools/server/server-context.cpp, rebuild + link):

  • wall: 5.3s -> 4.0s (-23.9%)

Binary size:

  • common/json.cpp.o adds 2.83 MB exactly once
  • libserver-context.a: 41.5 MB -> 24.9 MB (-40.0%)
  • linked libllama-server-impl.dylib: 6.86 MB -> 6.33 MB (-7.7%), __TEXT 4.95 MB -> 4.69 MB (-5.3%)

TODO:

  • add json.cpp/.h
  • migrate common/*
  • migrate the rest of the code base
  • test correctness --> did some smoke tests + fuzzing
  • see how much we gain on compile time
  • remove the deps jinja --> common (the json-to-internal code path) maybe a follow-up

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: yes, large part of code is AI-generated

@github-actions github-actions Bot added documentation Improvements or additions to documentation jinja parser Issues related to the jinja parser labels Aug 21, 2026
@github-actions github-actions Bot added the testing Everything test related label Aug 21, 2026
@ngxson
ngxson marked this pull request as ready for review August 22, 2026 08:46
@ngxson
ngxson requested review from a team, CISC, ggerganov and pwilkin as code owners August 22, 2026 08:46

@ServeurpersoCom ServeurpersoCom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Read it on a checkout: the only silent change is a braced pair in value position, array before and object now, and they are all converted to json::array() here, tests included. Nice side effect, operator[] const throws where master aborted on the assert.

Two nits, non blocking: tests/gguf-model-data.cpp still includes nlohmann/json.hpp directly, and the object iterator re-walks with std::next(begin(), idx) on every dereference.

Every copy now sits behind one translation unit, which is also where a depth bound would live. Is that the intent, so #27434 can be closed without waiting for nlohmann/json#5389?

@ggerganov ggerganov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice work! I see 10% build time reduction on my M2 Ultra with 8 threads for the total project and about ~20% reduction for user-space code (common, tool, examples, tests).

Btw, I think there is a low-hanging fruit for reducing compilation time by amalgamating the models/*.cpp files as part of the build process.

Comment thread common/json.h Outdated
Comment on lines +347 to +349
// json.cpp defines this specialization, it must be declared before any use of it
template <> common_json common_json::get<common_json>() const;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: I think this declaration can be safely removed

@ngxson

ngxson commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator Author

@ServeurpersoCom thanks for testing, I added your points as code comments

Is that the intent, so #27434 can be closed without waiting for nlohmann/json#5389?

maintainers are actively working on the upstream PR, so that this point we can be sure that it will be fixed upstream. let's close the PR #27434 to avoid duplicated works

Btw, I think there is a low-hanging fruit for reducing compilation time by amalgamating the models/*.cpp files as part of the build process.

@ggerganov TIL that cmakelists has a feature called UNITY_BUILD, may worth giving it a try?

@ngxson

ngxson commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator Author

rebased to make sure I'm not missing any code paths that were not migrated, will merge this PR once is CI is green

@ngxson
ngxson merged commit d9f918d into master Aug 22, 2026
24 of 27 checks passed
@eyalezer

eyalezer commented Aug 22, 2026

Copy link
Copy Markdown

@ngxson: is it just me or this merge broke my build on windows using clang? links issues (undefined symbols)

@ServeurpersoCom

ServeurpersoCom commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Interesting; I haven't tested Windows in advance at all, but it's inevitable: when I rebase my working branch, the little dedicated Windows machine (exposed at https://www.serveurperso.com/ia/windows/ !) is going to run into trouble, I check this now!

@ServeurpersoCom

ServeurpersoCom commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

It's simply the build folder that needs to be deleted on my side. Windows OK

@eyalezer

Copy link
Copy Markdown

@ServeurpersoCom - nope... still fails at my end even after using a clean build folder...
i have to be honest i haven't really tried to do any digging yet... i'll keep you posted.

BTW i do wonder now what was the potential fixed you proposed earlier?

@ServeurpersoCom

Copy link
Copy Markdown
Contributor

@ServeurpersoCom - nope... still fails at my end even after using a clean build folder... i have to be honest i haven't really tried to do any digging yet... i'll keep you posted.

BTW i do wonder now what was the potential fixed you proposed earlier?

Are you building master itself, or a branch of your own on top of it? If you carry local changes under tools/server or common, that is where I would look first: every JSON type and call in that code has to move to the new common_json. The header include becomes common/json.h instead of nlohmann/json.hpp, any nlohmann::ordered_json in a signature or a using alias becomes common_json, and a braced array literal in value position no longer builds, so {a, b, c} has to be written as json::array({a, b, c}).

@eyalezer

Copy link
Copy Markdown

Are you building master itself, or a branch of your own on top of it? If you carry local changes under tools/server or common, that is where I would look first: every JSON type and call in that code has to move to the new common_json. The header include becomes common/json.h instead of nlohmann/json.hpp, any nlohmann::ordered_json in a signature or a using alias becomes common_json, and a braced array literal in value position no longer builds, so {a, b, c} has to be written as json::array({a, b, c}).

indeed on a fork.... which i keep on rebased from master here.
thanks for the brief mate... i would go look there first thing.
don't sweat it, it might be something in my local fork i missed 😅

i'll keep you posted either way... i just started looking into it and thanks for the quick response ❤️

@eyalezer

Copy link
Copy Markdown

Found it — it was the -flto=full in my build flags. Dropping LTO made it link clean.

For anyone hitting the same thing on Windows + clang + LTO: explicit template instantiations in a static/shared lib can come out as weak (W) symbols instead of exported (T), so the linker reports them as undefined even though they're in the object.

@ServeurpersoCom: sorry if made you worry man 😅 looks like it a it's a clang-on-Windows LTO bug.

@maddes8cht

Copy link
Copy Markdown
Contributor

@ngxson @eyalezer

I can confirm this regression on Windows when building with clang-cl and LTO enabled. But in my opinion, simply dropping LTO is not a viable long-term solution for those of us relying on LTO to minimize CPU overhead during graph execution and prompt processing.

The root cause appears to be tied to the recent Autoparser refactoring, which introduced new JSON-template instantiations inside static libraries. Under Windows (COFF format), clang-cl + LTO incorrectly marks explicit template instantiations inside static libraries as weak (W) symbols instead of exported text (T) symbols. When the linker builds the final executable with LTO, it assumes these symbols will be provided elsewhere and optimizes them away, resulting in undefined reference errors.

Instead of forcing users to abandon Clang or LTO, could we fix this at the CMake or source level?

  1. CMake Object Libraries: Changing the affected static libraries to OBJECT libraries (e.g., add_library(common OBJECT ...)) bypasses the static-lib linker step entirely, feeding the .obj files directly to the final executable link. This completely avoids the Clang/Windows weak-symbol LTO bug.
  2. Export Attributes: Wrapping the explicit template instantiations in __declspec(dllexport) (or the project's equivalent API macros) forces Clang to emit them as strong exported symbols.

I'd love to keep using clang-cl with LTO for the CPU overhead reduction.

@ngxson

ngxson commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator Author

could you try #27575 ?

@maddes8cht

Copy link
Copy Markdown
Contributor

Confirmed! I tested PR #27575 and it resolves the LTO linking issue for me.

Built successfully with clang-cl + -flto=thin on Windows 11, and all binaries (llama-server, llama-cli) work as expected.
More info in the pr.

Thanks for pointing me to the fix!

@ggerganov

Copy link
Copy Markdown
Member

@ggerganov TIL that cmakelists has a feature called UNITY_BUILD, may worth giving it a try?

@ngxson I haven't used this feature before, but it seems like what we need.

fewtarius added a commit to fewtarius/CachyLLama that referenced this pull request Aug 24, 2026
Brings 218 upstream commits into CachyLLama without losing any of our
features. Key carried-over changes from upstream:
- llama.cpp v0.2.0 / ggml v0.21.0 version bumps
- Vulkan FA MMQ fp32 scaling (ggml-org#27413), PAD_REFLECT_1D (ggml-org#26586), tiled
  transpose (ggml-org#26585), null checks in queue command pools cleanup (ggml-org#27353)
- ggml: rope_set_offset on multiple backends, recurrent state rollback
- Vulkan coopmat1 SHMEM_STRIDE_PAD/APPLY_SLM_A_RESHAPE for Intel Xe
- server: LLAMA_SERVER_SLOTS_N_DIFF (ggml-org#27600), /metrics during llama_decode
  (ggml-org#27041), index.html no-cache (ggml-org#27006), make-release workflow
- model: MiniMax-M1/Text01 (ggml-org#27018), Kimi-K3 (ggml-org#26185), BailingMoE3 (ggml-org#26608),
  GraniteSWA (ggml-org#25505), GLM-4.5-Air MTP, DSV4 tensor split (-sm tensor)
- ui: Chat Conversation Tabbed navigation, settings refactor
- common: --models-dir loading MTP assistant models (ggml-org#24431),
  --load-mode replacing --mmap (ggml-org#26934), json.h abstraction (ggml-org#27511)
- vendor: cpp-httplib 0.53.1, BoringSSL 0.20260813.0, vendor/hash

CachyLLama features preserved through conflict resolution:
- Persistent SSD-backed KV cache (3-tier hot/warm/cold + system prompt cache)
- Per-user isolation (user_id, per-user concurrency cap, slot affinity)
- MoE expert residency + co-activation tracking
- CachyLLama Vulkan Lightning Indexer (108/108 on Strix Halo) + DSV4
  hyper-connection fused ops + DSV4 sparse FA + coopmat shaders
- FA quant-KV dequant-once + f16 contiguize (with host-RAM safety gate)
- DFlash framework + Laguna-S-2.1 model support
- DFlash d2t reduced-vocab draft support (upstream merge)
- Context checkpoint ring buffer + SWA skip + memory budget scaling
- Stable-prefix LCP gate + prompt_stable_prefix_tokens param
- conv_hash conversation-boundary detection
- All CachyLLama Vulkan shaders (concat_transpose, lightning_indexer,
  mmid_row_lists, flash_attn_top_k, dequant_f16_transpose)
- common::host_available_ram() utility
- llama-moe-residency + llama-moe-coact modules

Manual conflict resolution touches: src/models/dflash.cpp (DFlash d2t +
aux_norm), src/llama-kv-cache-dsv4.cpp (state snapshot fix), src/llama-
memory-recurrent.cpp (rs_idx bounds check), src/llama-model-saver.cpp
(DSV4 compress_ratios + swiglu_clamp sizing), ggml/src/ggml-vulkan/
{ggml-vulkan.cpp,vulkan-shaders-gen.cpp,vulkan-shaders/dequant_q8_0.
comp,vulkan-shaders/flash_attn.comp,vulkan-shaders/copy_transpose_02.
comp} (CachyLLama shader registration + FA scratch gate), ggml/src/
ggml-cuda/mmvq.cu (RDNA3_5 + GB10 enum), gguf-py/gguf/constants.py
(DFlash ENC_AUX_NORM + D2T tensors), tests/{CMakeLists.txt,test-backend-
ops.cpp,test-llama-archs.cpp,test-recurrent-state-rollback.cpp}
(test additions), tools/{CMakeLists.txt,server/*} (server_batch embd
support + spec_is_replay + user_id routing + MCP servers + CORS), and
docs/{AGENTS.md,README.md} (kept CachyLLama branding).

Verified: full build succeeds, test-backend-ops Vulkan LIGHTNING_INDEXER +
FLASH_ATTN pass on Strix Halo.

Based on a re-merge from the 20260824 (pristine pre-merge) branch after
a previous agent's merge attempt produced an unbuildable state from
-X ours that wiped shader float-typing and broke the dequant_q8_0 +
flash_attn shaders with redefinition errors.
therealkenc pushed a commit to therealkenc/llama.cpp that referenced this pull request Aug 24, 2026
* add common/json

* migrate common

* adapt jinja

* migrate server

* big wip

* migrate tests

* wip

* revert some excessive changes

* wip

* wip 2

* revert redundant changes

* fix server crash

* various fixes

* fix ci

* harden a bit

* clean up

* rm json-shim

* add some comments

* rm redundant decl
srossitto79 added a commit to srossitto79/llama.cpp that referenced this pull request Aug 25, 2026
Conflicts resolved:

- common/CMakeLists.txt: keep both the fork's jsonl.cpp/.h and upstream's
  new json.cpp/.h (common_json wrapper, ggml-org#27511).

- ggml/src/ggml-cuda/out-prod.cu: take upstream's removal of the redundant
  cublasSetStream (ggml-org#26574 binds the cuBLAS handle to its stream at creation).
  The fork's conditional lda for dequantized (quantized) src0 is unchanged;
  upstream's unconditional lda decl is dropped as it would redeclare it.

- ggml/src/ggml-metal/ggml-metal.metal: upstream split the monolithic
  source into kernels/*.metal (ggml-org#26561). The fork's kernels were ported into
  the new layout:
    * quantize_q3_K / quantize_q4_K / quantize_mxfp4 -> kernels/quantize.h
    * kernel_cpy_* q3_K/q4_K/mxfp4 instantiations    -> kernels/quantize.metal
    * adamw gclip, out_prod, out_prod_id, get_rows_back, repeat_back,
      cross_entropy_loss{,_back}                     -> kernels/misc.metal
    * rms_norm_back                                  -> kernels/norm.metal
    * soft_max_back                                  -> kernels/softmax.metal
  No CMake or enum changes are needed: kernel->library routing is built from
  each compiled library's functionNames.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tobocop2 added a commit to tobocop2/llama.cpp that referenced this pull request Aug 28, 2026
Upstream restructured the code the memory work sits on.

server: refactor sleep handling (ggml-org#27376) split slot reporting out of
SERVER_TASK_TYPE_METRICS into a new SERVER_TASK_TYPE_SLOT_GET task,
rewrote get_metrics around a cached snapshot served during sleep, and
extracted the props body into get_res_props(). memory_data stays on
server_task_result_metrics, the memory gauges keep their place on
/metrics, and endpoint_memory is back in get_res_props().

/metrics during sleep must equal the last awake scrape. The model is
unloaded by then, so update_cached_responses() now renders the memory
series while the model is still loaded and use_cached_metrics() appends
the cached string. The rendering moved into render_memory_metrics() and
the breakdown into server_context_impl::get_memory_data(), so the awake
and sleeping paths share one implementation.

common: add json.h abstraction (ggml-org#27511) replaced nlohmann::json with
common_json, which has no find(). The gauge loop uses contains() and
at() instead.
tobocop2 added a commit to tobocop2/llama.cpp that referenced this pull request Aug 28, 2026
Upstream restructured the code the memory work sits on.

server: refactor sleep handling (ggml-org#27376) split slot reporting out of
SERVER_TASK_TYPE_METRICS into a new SERVER_TASK_TYPE_SLOT_GET task,
rewrote get_metrics around a cached snapshot served during sleep, and
extracted the props body into get_res_props(). memory_data stays on
server_task_result_metrics, the memory gauges keep their place on
/metrics, and endpoint_memory is back in get_res_props().

/metrics during sleep must equal the last awake scrape. The model is
unloaded by then, so update_cached_responses() now renders the memory
series while the model is still loaded and use_cached_metrics() appends
the cached string. The rendering moved into render_memory_metrics() and
the breakdown into server_context_impl::get_memory_data(), so the awake
and sleeping paths share one implementation.

common: add json.h abstraction (ggml-org#27511) replaced nlohmann::json with
common_json, which has no find(). The gauge loop uses contains() and
at() instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation jinja parser Issues related to the jinja parser server testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants