Pull requests / #826
#826 verify: the shared-expert stream fork starts off on HIP (fixes the 0.1.38 -> 0.1.39 decode regression, #816)
closed · @pugolini · 0 Kommentare · Auf GitHub
AMD / HIPNVIDIA / CUDAModels & quantsDocumentationWindows
Beschreibung
## What this changes The shared expert is forked onto a second stream and joined back per layer (`sh_fork` in `record_window`). That overlap pays off on CUDA, where cfd3b72 added it. On HIP, the cross-stream event dependency costs more than the shared expert takes to run, so this makes the fork start **off** on HIP and `STRATA_SH_STREAM=1` turns it back on. **CUDA's default is unchanged.** This is the same shape as `mtp.cpp`'s `STRATA_MTP_PREFILL_SYNC` (#382): the env var always wins, only the default differs per backend. It also closes a small gap in the join path: the second `cudaStreamWaitEvent` tested `!prof_on_` instead of whether the fork actually ran, so it waited on an event that had never been recorded whenever the fork was off. Fixes #816 (same regression on a gfx1030, RX 6800 on Windows, where `STRATA_SH_STREAM=0` already removes it). No issue/PR touching the fork's default for HIP existed when I opened this. The regression window lines up with the root cause: `sh_stream_env` appears 0 times in v0.1.38 (`99f3dbd`) and twice in v0.1.39 (`6f32ec0`), and `cfd3b72` is an ancestor of `6f32ec0` but not of `99f3dbd`. Two independent HIP reports match that window — #816 on a gfx1030, and a [comment on #646](https://github.com/Niko1221/Strata/pull/646) from @Burntgogi, who measured an RX 7900 GRE (`gfx1100`) decoding slower from `99f3dbd` to `6f32ec0` with compiler, libraries, model and profile fixed. Neither of them attributed it to the fork; that is what the A/B below adds. ## Measurements Radeon AI PRO R9700 (gfx1201), ROCm 6.4.3, Qwen3.8-Flash-Next 176B MoE IQ3_XXS, `--kv int8 --kv-resident 32768 --resident-experts`, temperature 0, same machine, same binary per case, same prompt. Prompt is **48,067 tokens** so the KV stream path runs; 3 measured runs each, all three reported. | Configuration | decode t/s (median) | range | | --- | --- | --- | | stock 0.1.39 (fork on, the current default) | **43.40** | 38.90 – 44.60 | | **this change (fork off by default)** | **62.90** | 55.40 – 63.80 | | `STRATA_SH_STREAM=1` (force fork on) | 43.50 | 39.10 – 45.20 | | `STRATA_SH_STREAM=0` (force fork off) | 63.00 | 55.90 – 64.10 | The forcing cases bracket the change: `=1` reproduces stock 0.1.39 (43.50 against 43.40) and `=0` reproduces the new default (63.00 against 62.90), so the switch is still in charge. The list below is the raw run output. Run 1 of each case is slower because it pays the first cold read; runs 2-3 are at 98.5%/99.0% expert cache hit, so the gap is not a cold-cache artefact. ``` === AB-STOCK-DEFAULT (binario=0139stock, env=DEFAULT) === sha en uso: 25416cee5c483c2f <- stock 0.1.39 env real: STRATA_SH_STREAM=<ausente> run 1: prompt=48067 gen=256 decode=38.9 t/s cache=96.5% wall=41.9s run 2: prompt=48067 gen=256 decode=43.4 t/s cache=98.5% wall=6.2s run 3: prompt=48067 gen=256 decode=44.6 t/s cache=99.0% wall=6.0s MEDIANA=43.40 t/s min=38.90 max=44.60 n=3 === AB-PATCH-DEFAULT (binario=0139patch, env=DEFAULT) === sha en uso: 16316e44fec79285 <- this change env real: STRATA_SH_STREAM=<ausente> run 1: prompt=48067 gen=256 decode=55.4 t/s cache=96.5% wall=39.7s run 2: prompt=48067 gen=256 decode=62.9 t/s cache=98.5% wall=4.3s run 3: prompt=48067 gen=256 decode=63.8 t/s cache=99.0% wall=4.3s MEDIANA=62.90 t/s min=55.40 max=63.80 n=3 === AB-PATCH-SH1 (binario=0139patch, env=SH1) === env real: STRATA_SH_STREAM=1 run 1: prompt=48067 gen=256 decode=39.1 t/s cache=96.5% wall=41.7s run 2: prompt=48067 gen=256 decode=43.5 t/s cache=98.5% wall=6.1s run 3: prompt=48067 gen=256 decode=45.2 t/s cache=99.0% wall=5.9s MEDIANA=43.50 t/s min=39.10 max=45.20 n=3 === AB-PATCH-SH0 (binario=0139patch, env=SH0) === env real: STRATA_SH_STREAM=0 run 1: prompt=48067 gen=256 decode=55.9 t/s cache=96.5% wall=39.5s run 2: prompt=48067 gen=256 decode=63.0 t/s cache=98.5% wall=4.3s run 3: prompt=48067 gen=256 decode=64.1 t/s cache=99.0% wall=4.2s MEDIANA=63.00 t/s min=55.90 max=64.10 n=3 ``` ## Independent confirmation on the same architecture After I opened this, @biosynthart posted [a second gfx1201 measurement in #816](https://github.com/Niko1221/Strata/issues/816#issuecomment-5987496242) — same architecture, different card, different model quantisation, engine built from source with `-DSTRATA_ENABLE_HIP=ON -DSTRATA_ENABLE_CUDA=OFF -DCMAKE_HIP_ARCHITECTURES=gfx1201`: | Engine | IQ3_S | IQ3_XXS | | --- | --- | --- | | 0.1.38 | 74.1 | 68.9 | | 0.1.39 (defaults) | 37.0 | 44.6 | | 0.1.39 + `STRATA_SH_STREAM=0` | **78.3** | **71.9** | Their IQ3_XXS fork-on figure (44.6) lands on mine (43.40), and their conclusion matches mine: `STRATA_SH_STREAM=0` does not just recover 0.1.38, it lands slightly **above** it on both quantisations. They also hold constant the expert cache sizing (13,217 slots / 25.09 GiB), the decode cache hit rate (94-98%) and the per-run draft acceptance counts (174/244 on both arms), and find prefill unchanged — which isolates the regression to the decode verify path, exactly where `sh_fork` lives. I did not run their configuration and they did not run mine; the agreement is between two independent setups. ## Output is unchanged The fork is a scheduling change, so the answers must be identical. Four prompts (explanatory, code, technical summary, free paragraph), temperature 0, same model, stock against this change, default env in both (no `STRATA_SH_STREAM`): ``` IDENTICO sha=c90c0d634af13016 len=546 IDENTICO sha=2e47bb14718944bb len=565 IDENTICO sha=f535875ace5f7839 len=804 IDENTICO sha=ad0681b573302507 len=1031 === RESULTADO: 4 identicos, 0 distintos de 4 === ``` ## Limitations, honestly - **My own measurement is one gfx1201 card.** @biosynthart's gfx1201 numbers above are an independent reproduction on the same architecture, and #816 covers a gfx1030 on Windows, but I did not run either of their configurations and they did not run mine. - **Short prompts barely show it.** At a 256-token output on a short prompt the same comparison is 65.4 t/s (fork off) against 62.7 t/s (fork on) — about 4%. The ~45% only appears once the KV stream path is in play. That is why this stayed invisible: it looks like noise on a small prompt. - **I did not profile why** the second stream costs this much on HIP. The numbers are consistent with the cross-stream event dependency serialising what CUDA overlaps, but that is a hypothesis, not a measurement. - **Only the env-var-absent case is a real default change.** `=0` and `=1` keep working exactly as before on both backends. - I have no CUDA card, so **CUDA is untested here** — but the `#else` branch returns exactly what the current code returns (`!e || e[0] != '0'`), so the CUDA default is bit-for-bit the old behaviour. - Per `docs/COMMUNITY_BENCHMARKS.md` I kept this separate from a results-only submission; this PR is the engine change only. ## Testing - Builds clean on HIP (gfx1201, ROCm 6.4.3, CMake 4.4.3, Release); the only warnings are the pre-existing `hipError_t` `-Wunused-result` ones in `on_device.hpp` and `verify.cpp`, unchanged by this patch. - Stock 0.1.39 rebuilt from `6f32ec0` for the A/B has sha256 `25416cee5c483c2f`, identical to the binary I was running, so the control is the real release build. - Ran the decode A/B above and the 4-prompt output-parity check. - `sycl/` is untouched: its `verify.cpp` has no `sh_cs_`/`sh_fork`. - `src/pack` has no diff, so existing packs stay compatible.
Mehr auf der Site
Links zu Install, Modellen, Releases.