Repository navigation
perf(moss): reuse shared modulation slices - #2208
Conversation
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Block modulation interface src/model/diffusion/moss_image.hpp |
Block::forward accepts an array of six modulation tensors and uses each entry. |
Model modulation flow src/model/diffusion/moss_image.hpp, docs/moss_image.md |
Model::forward slices the shared modulation tensor, expands each slice into the graph when a graph is present, and passes the array to non-skipped blocks. The runner obtains its context with get_context(gf). The documentation describes the reuse and states that values and consuming arithmetic remain unchanged. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Refactor
Possibly related PRs
- leejet/stable-diffusion.cpp#2201: Changes the same modulation interface and model slicing flow, alongside additional changes not included here.
Suggested reviewers: leejet
Merge Risk: 
Minimal · up to f7877
-
No actionable merge-blocking risk is established for this change.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Title check
-
Explanation
The title "perf(moss): reuse shared modulation slices" names the main change: the model builds six shared modulation slices once per forward graph and reuses them across diffusion blocks. It is short, specific, and matches the changeset in moss_image.hpp and the docs note. It omits the benchmark context, which is not needed for a title.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
| <img src="../assets/moss/sft.png" /> | ||
|
|
||
| Use dimensions divisible by 16. The default discrete scheduler uses the upstream time shift of 5. Use `--flow-shift` to override it. The negative CFG branch skips DiT layer 22 (zero-based), matching the upstream pipeline. | ||
|
|
There was a problem hiding this comment.
The changes to the documentation should be removed, as they are unnecessary.
Summary
The model constructs the six shared modulation slices once per forward graph and reuses them across diffusion blocks. Their values and the arithmetic consuming them are unchanged. This application change works with the pinned ggml.
Applies independently to the current pinned ggml; no companion backend or submodule update is required.
Related Issue / Discussion
Prepared as an independent split of #2201 following the request for separate evaluation. This patch contains only the named application change. The previous combined application/backend speedup is not attributed to it.
Additional Information
Backend used for the companion measurements: leejet/ggml#16, at
2b1e3ee0d46213cbaa901d3f97008fc14013c9ff. The pinned-backend measurements do not include those changes.Functional compatibility with pinned ggml is separate from performance with the companion fusions. The two backend comparisons below are reported independently.
A synthetic 42-layer graph with 3x2 image latents and 11 text tokens reduced shared-modulation slices from 252 to 6 and total nodes from 2910 to 2418. This is a graph-structure result, not the node count of the measured CUDA image graph.
RTX 3090 (SM86), Windows, CUDA 12.4; MOSS-Image-SFT-Q8_0.gguf, Qwen3.5-9B-Q8_0.gguf, FLUX.2 VAE and external tokenizer; 1024x1024, 10 Euler steps, CFG 3, flow shift 5, seed 42, empty negative prompt and batch 1. Flash Attention and automatic placement enabled; SageAttention, CUDA Graphs, profiling and conditioning cache disabled.
Every case used a fresh server process, one warmup and one timed image. Each unmodified-application baseline was tested in two processes; each candidate in one. Native generation excludes startup and HTTP/PNG transfer. Denoising time is divided by ten. These samples do not establish statistical confidence; small timing differences are not claimed as speedups. Device-wide peak VRAM was sampled every 100 ms. Clocks and power settings were not changed.
All generated RGB pixels for this patch match the unmodified CUDA reference: zero changed pixels and zero maximum channel error. This establishes equality for the tested prompt/settings, not every model, GPU, seed, shape or offload configuration. No new Vulkan inference, multi-GPU, other-model or full regression testing is claimed. Local tests, model weights and binaries are excluded from the patch.
Prompt: A Victorian portrait of a closeup Scottish 60 years old man in a kilt, mountain nature and blue sky, sharp focus, DSLR, Fujifilm XT3, full body visible.
Application base:
95ff76230889898e1bf40058ba272a3699b50dc6. Pinned ggml:a5915eb0e47adef21c3f25b889a008925ea838c2. Companion backend, where used:2b1e3ee0d46213cbaa901d3f97008fc14013c9ff.Checklist
Published as an independent split of #2201. The branch was merge-checked against master
a85db96aa5ce2c72bd5aec326f6c5c857839a50a; the generation measurements above remain based on95ff76230889898e1bf40058ba272a3699b50dc6. No additional images were generated for publication.Summary by CodeRabbit