Speed up prefill on pre-Apple10 Macs - #159
Merged
Merged
Conversation
drumih
marked this pull request as draft
August 26, 2026 11:30
Brings this branch up to the reviewed state of the private tree. Four findings, all of which this branch previously carried unfixed. Reject a clipping window on the TensorOps path, not only on the fallback selection. The full-attention kernels start their key loop at zero and ignore `slidingWindow`, so a `.full`-labelled request carrying a window that actually clips produced wrong output. Production passes `slidingWindow == kvValidCount` for full layers and never clips, so selection there is unchanged; the guard now also requires `layerKind == .full`, which this tree was missing. Restrict the AGX interactivity mitigation to macOS at compile time. The `AGX_RELAX_CDM_CTXSTORE_TIMEOUT` variable is read by the AGX userspace driver and does not exist elsewhere. The operator override is unchanged: the overwrite argument stays 0, so exporting the variable as 0 still restores stock behaviour. Make the system-device source audit fail closed. It swallowed enumerator and file-read errors with `try?` and exempted by bare filename, so it could report green while auditing nothing. It now requires the enumerator, records walk and read failures, asserts them empty, and exempts by exact relative path. Route all three VisionRuntime command-buffer waits through the status-aware diagnostic helper. They stringified `buffer.error`, so a buffer that failed with `status == .error` and no error object was consumed as success. The fourth wait is inside a `defer` on the throw path and stays deliberately unchecked, which is now stated at the site. Each fix lands with a regression: a clipping-window fixture at the production 512/16/2 shape that reaches the real TensorOps pipeline, a compile-selected non-macOS negative test, positive assertions on the audit's failure arrays, and a call-site check on the VisionRuntime waits.
The previous commit staged the whole docs/ directory and swept in six untracked working files that were never meant to be published, plus a stray directory left by a failed sync whose name contained a newline. Removed: the four ISSUE_84 review and planning documents, the terminal 3D engine plan, and issue84_simulate_payloads.py. These are internal working notes; they contain local filesystem paths and unreleased planning material. They remain reachable in this branch's history at 04b36ce. Rewriting that history would discard the main merge made on top of it, so it is left for a deliberate decision rather than done here.
drumih
marked this pull request as ready for review
August 26, 2026 13:04
ulises-c
added a commit
to ulises-c/turbo-fieldfare
that referenced
this pull request
Aug 26, 2026
The pull request cited raw artifacts under benchmark-results/, which is gitignored, so none of the data backing the table was actually in the change. This moves the measurements into a durable experiment note alongside the conditions needed to read them correctly. Records both questions separately. The admission ladder is reported with its recorded run, and the note states plainly that the timing columns predate the merge of drumih#159, which rewrote prefill attention selection and the prefill command buffers, so those columns describe a superseded implementation and are marked for re-measurement. It also notes the rungs were captured across more than one commit with the baseline last, so they are not a matched set. Adds the KV memory model derived from KVCacheManager and shows it predicts observed peak Metal allocation at all five rungs to a constant 32 MB offset. That is the structural reason the memory columns should survive re-measurement, and the same formula gives the ~55 GiB figure behind the new --prefill guard. Records the retrieval results that do exist, at 1,543 and 14,043 prompt tokens, and states that nothing is measured between 57K and 254K, which is the range the 256K cap adds.
ulises-c
added a commit
to ulises-c/turbo-fieldfare
that referenced
this pull request
Aug 28, 2026
chore: sync fork-main with upstream through drumih#159
ulises-c
added a commit
to ulises-c/turbo-fieldfare
that referenced
this pull request
Aug 30, 2026
The recorded ladder measured admission but probed recall only at 1,543 and 14,043 tokens, so 57K-254K was unmeasured -- and a degraded long context returns HTTP 200 exactly like a healthy one. The recorded prefill columns also predate the merge of drumih#159, which rewrote the path they measure, and were taken across several commits with the 64K baseline last. One sweep answers both: every probe runs on a single binary and records pp_seconds and pp_tokens_per_second alongside the needle result, so the matched timing set falls out of the recall run instead of costing a second full prefill of the ladder. Each rung starts and stops its own server, and results append to JSONL after every probe so an interrupted sweep keeps what it measured.
ulises-c
added a commit
to ulises-c/turbo-fieldfare
that referenced
this pull request
Aug 30, 2026
Retrieval is measured across the ladder for the first time: 15/15 at depths 0.1/0.5/0.9 from 57,043 to 253,143 tokens, every probe with cached_tokens 0. Depth 0.5 at 256K puts the needle ~126,000 tokens from either end, outside the 1,024-token window of all 25 sliding-window layers, so it is reachable only through the 5 full-attention layers -- and it was recovered verbatim. The cap extends what the model can retrieve, not just what it will accept. Filler is repetitive by construction, so this is an upper bound on recall and evidence against catastrophic sliding-window failure, not a guarantee for arbitrary content. Recorded as such. The prefill columns are re-measured on one binary, three probes per rung, replacing numbers taken across several commits before drumih#159. Every rung is slower, median -6.5%. That is NOT called a regression: the old column is not an internally matched set, drumih#159 targeted pre-Apple10 Macs while this host is an M5 Max, and no A/B against the pre-drumih#159 commit has been run. The new table is a matched baseline for future comparison; the old one is kept as a dated lab record. Raw sweep output is committed under docs/experiments/data/ since benchmark-results/ is gitignored.
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
1 participant
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
M2 evidence
M4 is optional post-public reporter confirmation, not a merge gate: pipeline build capability is probed directly and failure retains the safe fallback.
Validation
Review fixes
Commit 04b36ce brings this branch up to the reviewed private state. It previously carried four findings unfixed:
Each lands with a regression test. Production selection is unchanged: full layers pass slidingWindow == kvValidCount, which never clips.