Repository navigation
Conversation
The built-in whisper.cpp engine hard-coded flash_attn=false, while upstream has enabled it by default since v1.8.0. Add shouldUseFlashAttn(backend) and use it at both addon call sites (the main transcription pipeline and the voice-clone reference-clip transcriber). Enabled for metal, coreml and cuda, where upstream benchmarks and a local Metal A/B show lower encode/decode time. Left off, i.e. unchanged behaviour, for cpu (no data), vulkan (the shipped ggml predates the flash-attention shader out-of-bounds fix, llama.cpp #29988) and custom addons (unknown build).
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The built-in whisper.cpp engine passed
flash_attn: falseon every call, while upstream has enabled flash attention (FA) by default since v1.8.0 (ggml-org/whisper.cpp#3441). This turns it on for the backends where there is evidence that it helps, and leaves every other backend exactly as it was.What changes
shouldUseFlashAttn(backend)tomain/helpers/engines/transcribeShared.ts:trueformetal,coreml,cuda;falseforcpu,vulkan,custom.builtinEngine.ts(main transcription pipeline) andvoiceClone/referenceTranscriber.ts(reference-clip transcription).scripts/test-engine-units.ts. The expectation table is typedRecord<WhisperBackend, boolean>, so adding a backend later fails to compile until someone decides its FA setting.The addon already reads
flash_attnfrom JS (its init log printsflash attn = 0/1), so no addon rebuild is needed.scripts/longgap/run*.tsstill hard-codeflash_attn: false; they are experiment harnesses and I left them alone so their numbers stay comparable.Why not simply
trueeverywhereDeviceLostcrash, included in whisper.cpp v1.9.5). Enabling FA there could crash Vulkan users.Opening these up later is a one-line change in
shouldUseFlashAttn(return true;for blanket-on), ideally after an A/B with the matching addon.Testing
Why enable it, from upstream's
scripts/bench-all-gg.txt(same device and commit, FA off vs on, small to large models): Apple Silicon encode -15 to -33 %, decode -9 to -22 %; NVIDIA encode -53 to -70 %, decode -6 to -24 %, but only on Blackwell (RTX 5090, DGX Spark). It is not a free win everywhere: one row (M1 Pro, tiny) shows decode 7.6 % slower with FA, and encoder/decoder times in that file do not convert directly to end-to-end time.Local A/B on an Apple M1 Pro, using the
addon.node/addon.coreml.nodethe app ships, called the way the app calls them (process.dlopen+promisify) with the app'swhisperParams. FA off and on were run in alternating processes; the first call in each process was discarded as Metal pipeline warm-up and the rest are medians. The machine was shared and under load, so treat timings as indicative. English audio, language set toen.Output is not bit-identical with FA (float accumulation order changes), so a few decoding forks and segment boundaries move. On the 4-minute clip, tokens that line up between the two transcripts shift by a median of 50 ms (p90 400 ms, max 950 ms). To put that in context, running the same clip on CPU instead of Metal, both with FA off, gives the same spread (edit distance 6, median 50 ms, p90 410 ms, max 950 ms), and Metal with FA on matched the CPU transcript word for word. Repeating the same setting in separate processes gave identical output in every repeat I made. This is one clip and one model family, so it shows FA stays within normal backend noise there, not that it is lossless in general.
CoreML:
addon.coreml.nodewithggml-tinyand its encoder.mlmodelc, 11 s clip. It loads and runs withflash attn = 1, output identical to FA off, encode time unchanged (encoder is on the ANE). Decode went 44-55 ms -> 38-40 ms, which is too small a model to mean anything.npm run test:engines: 982 passed, 0 failed (976 before, +6 new); the sherpa and Qwen scaffold scripts pass.tsc --noEmitclean for renderer, main and automation; prettier clean.truefails exactly thecpu,vulkanandcustomcases.whisperParams:line now showsflash_attn: trueon Metal, CoreML and CUDA.Not run: