Fixed for decoder dl speed and mobile warnings on low memory devices.
This commit is contained in:
1 parent
2643a446d0
commit
78e99fc415
7 files changed
+208
-32
No files matched your search
@@ -617,3 +617,88 @@ warning thresholds).
|
||||
mobile crashes in practice, consider loosening the confirm thresholds; if
|
||||
onnxruntime-web ever gains a streaming/incremental session-creation API,
|
||||
drop the patch and re-measure before touching the thresholds again.
|
||||
**Amendment (2026-07-29, D-028):** "a little wall-clock time" undersold the
|
||||
cost — see D-028. The sequential patch no longer applies unconditionally;
|
||||
`options.sequentialSessionLoad` (set by `App.tsx`, true only when
|
||||
`shouldWarnBeforeLoad` is true for the load) now gates it.
|
||||
|
||||
## D-028 — Sequential session loading gated to actual mobile-crash risk, not unconditional
|
||||
**Status:** Accepted (2026-07-29)
|
||||
Reported symptom: `encoder_model_q4.onnx` (Whisper Medium) downloaded far
|
||||
slower after D-027 landed than before it — reproduced on desktop too, ruling
|
||||
out phone CPU/network as the cause. Measured in Chrome DevTools: ~1.6 MB/s
|
||||
sustained on a connection that otherwise performs fine, downloading from
|
||||
Hugging Face's Xet-backed CDN (`us.aws.cdn.hf.co/xet-bridge-us/...`).
|
||||
**Cause.** D-027's `constructSessions` patch made the encoder and decoder
|
||||
downloads always sequential — one HTTP request in flight at a time — to keep
|
||||
both weight buffers from being resident together. On a long-RTT path (this
|
||||
CDN is US-hosted), a single HTTP/2 stream typically can't fill the available
|
||||
bandwidth; two concurrent requests (the pre-D-027 `Promise.all`) filled more
|
||||
of it. Serializing the downloads dropped every load back to one flow's
|
||||
throughput ceiling — not "a little" wall-clock cost as D-027 claimed, but
|
||||
multiple times slower on exactly this kind of connection, and paid by every
|
||||
user regardless of device.
|
||||
**Fix.** The sequential/parallel choice is now conditional, not baked into
|
||||
the patch: `constructSessions` branches on a new `options.sequentialSessionLoad`
|
||||
flag (`patches/@huggingface%2Ftransformers@4.2.0.patch`) — parallel
|
||||
`Promise.all` (the original, fast behavior) when falsy/absent, the D-027
|
||||
for-loop when true. `App.tsx` computes this per load via `isMobileLoadRisk`
|
||||
(extracted from `confirmRiskyMobileLoad`'s existing hint/`estimatePeakLoadBytes`
|
||||
logic — the same `shouldWarnBeforeLoad` check the crash-warning dialog uses)
|
||||
and threads it through a new `sequentialSessionLoad` field on the worker's
|
||||
`LoadMessage` (`protocol.ts`) into the `pipeline()` call
|
||||
(`inference.worker.ts`). Crucially this is evaluated independently of the
|
||||
confirm dialog's answer — even a load the user clicked "Continue anyway" on
|
||||
still loads sequentially, since the memory risk that motivated D-027 doesn't
|
||||
go away just because the user accepted it.
|
||||
**Consequence:** desktop/laptop loads (and any mobile load under the
|
||||
size/`deviceMemory` risk threshold) get back the full concurrent-download
|
||||
speed that predates D-027; only loads that would already trigger the mobile
|
||||
crash warning pay the sequential-loading wall-clock cost, which is exactly
|
||||
where D-027's OOM fix is actually needed.
|
||||
**Revisit:** if Hugging Face's Xet CDN behavior changes, or if a future model
|
||||
in the catalog is large enough to risk mobile OOM at a size where the
|
||||
confirm-dialog threshold doesn't yet trigger, re-check that the two
|
||||
thresholds (`shouldWarnBeforeLoad`'s and this gate's) are meant to stay
|
||||
identical.
|
||||
|
||||
## D-029 — Mobile crash warning was silently disabled on every phone but the lowest-end
|
||||
**Status:** Accepted (2026-07-29)
|
||||
Reported symptom: the D-027 mobile crash `window.confirm()` never appeared
|
||||
for Whisper Medium (or any model), even on devices it's explicitly meant to
|
||||
protect.
|
||||
**Cause.** `shouldWarnBeforeLoad`'s `deviceMemory`-scaled branch used
|
||||
`SAFE_TAB_FRACTION_OF_DEVICE_MEMORY = 0.5`. Checked against the actual
|
||||
catalog (`estimatePeakLoadBytes`) for every `navigator.deviceMemory` value
|
||||
Chromium can report:
|
||||
|
||||
| deviceMemory | "safe" budget (old 0.5) | Medium (556 MB) | Large v3 Turbo (762 MB) |
|
||||
|---|---|---|---|
|
||||
| 1 | 500 MB | warns | warns |
|
||||
| 2 | 1 GB | **silent** | **silent** |
|
||||
| 4 | 2 GB | **silent** | **silent** |
|
||||
| 8 | 4 GB | **silent** | **silent** |
|
||||
|
||||
`deviceMemory` of 2 or more is ordinary today, not just flagship-tier — so
|
||||
in practice the warning only ever fired on the oldest/cheapest Android
|
||||
phones and on iOS (which lacks the API entirely and always used the flat
|
||||
400 MB fallback, D-027). The exact mid/low-range phones the original crash
|
||||
report was about — the whole reason D-027 exists — cleared the threshold and
|
||||
got no warning at all.
|
||||
**Fix:** `SAFE_TAB_FRACTION_OF_DEVICE_MEMORY` lowered to 0.1
|
||||
(`src/lib/mobileMemoryGuard.ts`). Re-checking the same table: deviceMemory
|
||||
2 and 4 now warn for Medium+ (200 MB / 400 MB budgets, both under 556 MB);
|
||||
deviceMemory 6-8 (600-800 MB budgets) still don't, treating genuine
|
||||
high-end devices as having real headroom. `tests/mobileMemoryGuard.test.ts`
|
||||
gained a regression case pinning deviceMemory 2/4 to `true` for Medium's
|
||||
peak — the exact gap this decision closes.
|
||||
**Alternatives rejected:** dropping the `deviceMemory` scaling entirely in
|
||||
favor of always using the flat 400 MB fallback (simpler, but throws away the
|
||||
one signal that lets genuinely high-RAM devices skip an unnecessary
|
||||
confirm); a hard absolute cap on top of the fraction (more robust long-term,
|
||||
but two knobs to calibrate against no real data beats one — revisit if a
|
||||
future model's peak lands awkwardly relative to the 0.1 fraction).
|
||||
**Revisit:** this is still a guess, same as D-027's original 0.5 was — there
|
||||
is no real crash-telemetry feedback loop. If field reports show 0.1 is still
|
||||
too generous (or too aggressive) for some device class, adjust the constant
|
||||
rather than the shape of the heuristic.
|
||||
Reference in new issue
Block a user