贡献 / #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 评论 · 去 GitHub 看

AMD / HIPNVIDIA / CUDAModels & quantsDocumentationWindows

说明

## 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.

本站相关内容

相关页面的快捷入口。