Skip to content

Pack Instant V into contiguous scratch before bidirectional attention - #178

Closed
hsliuustc0106 wants to merge 1 commit into
mainfrom
cursor/instant-v-pack-248b
Closed

hsliuustc0106 wants to merge 1 commit into
mainfrom
cursor/instant-v-pack-248b

Conversation

@hsliuustc0106

@hsliuustc0106 hsliuustc0106 commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Required human contributor answers

Answer every question before implementation and PR submission, including drafts.
Reuse explicit, still-applicable human answers with a traceable source reference;
ask the human for anything missing and wait. An agent must not invent answers.
Blank fields, placeholders, or an agent-written rationale do not satisfy this gate.
Share only information and source links appropriate for this PR's audience.

  1. What outcome are you trying to achieve?

    Answer: Lower StartLux Instant Public231 C1 latency by packing V so bidirectional attention reads contiguous rows, without regressing pooled p50 or p95. Source: owner Hongsheng approved tip1 only (pack V contiguous) in the task that opened this draft.

  2. What is the highest-leverage variable for that outcome?

    Answer: V row stride on cs1_attention_gated_bidirectional. Today V stays in the QKV row (ldv = attn_in = 10240 on Instant 4B). This tip passes ldv = Hk*Dh = 1024. Source: same owner tip approval.

  3. How will this work move that variable rather than create busywork?

    Answer: After Instant QKV, copy V into contiguous [T,Hk,Dh] scratch (same width as post-prep K) and call bidirectional attention with that stride. One layout change on the data path. Source: same owner tip approval.

  4. Who is the customer or user, and what do they actually need?

    Answer: StartLux Instant InferRSI. Owner Hongsheng needs this tip as a draft that can be remeasured on the same L20X protocol. They do not need tip2, a merge, or a ready-for-review flip. Source: same owner tip approval.

After answering the four questions: is a PR needed at all to achieve this outcome?

Human decision and rationale (or explicit source reference): Yes. Owner Hongsheng asked to open a draft PR from main for tip1 only, and not to merge.

For a bug fix: link actual observed failure evidence.
Include the observed result of a minimal reproduction, failing regression test,
or relevant trace. Label untested diagnoses as unverified. For a non-bugfix,
state why this is not applicable.

Evidence: Not a bug fix. This is a latency layout tip. The baseline below is the measured starting point. A wrong V stride is a hypothesized cost, not a verified functional failure. GPU output parity of this tip is unverified.

Purpose

Tip1 only: pack Instant V into contiguous scratch. Draft. Do not merge.

Rebased onto current origin/main a41e379453d4a8dc3cbe3f76c8754753c42230a1 (Clef #165 squash). No conflict. Pack-V semantics are unchanged. Tip2, scratch pre-size, Ignition, CUDA graphs, and kernel rebinds are not in this tip.

RSI phase: layout/contiguity (data path — not bare kernel swap).

Primary change: After Instant QKV (alongside attn prep), pack V into contiguous [T,Hk,Dh] scratch mirroring aq/ak, and call cs1_attention_gated_bidirectional with ldv = Hk*Dh (1024 for Instant 4B) instead of leaving V in attn_in with ldv = attn_in (10240).

Alternatives considered (not this tip):

  • (a) change QKV weight layout for contiguous V — larger ABI churn
  • (b) Instant CUDA graphs — incompatible with bidirectional
  • (c) cuBLASLt plan-cache / gemm warm|compact|shape — rejected standing
  • (d) scratch pre-size only — tip2, separate, not included

Causal full attention still reads V from the QKV row. No new CUDA symbol. Scratch still grows to the next multiple of 1024 tokens; this commit does not pre-size it. The contiguous V buffer is allocated with the rest of scratch (same width as post-prep K) and causal attention does not read it.

The optimize-remeasure-gate list of phase names is bring-up/scheduling, parallelism/comms, kernel, and launch/post. This tip keeps the owner name layout/contiguity. It is not a bare kernel swap. The PR stays draft until a separate owner yes after a conclusive remeasure.

Baseline (must match the tip record)

Measured C1 baseline (not the current git base):

  • runtime_revision baseline: 51f80ad681c8f7b1be9b22b5e700bef34e33534c
  • Public231 C1 pooled p50/p95: 7.338 / 51.816 ms; pass 693/693; warmup 5; C1; 3 rounds
  • device: L20X (nvidia-smi string); compute capability: 8.9
  • thinking: enabled:false; clock: post_to_body_json_and_schema_validation
  • 2048+ bucket: n=108, p50 50.346 / p95 71.942

Git base after rebase: a41e379453d4a8dc3cbe3f76c8754753c42230a1 (origin/main, Clef #165). No C1 number is claimed for that base or for the tip SHA.

Expected: dual gate — pooled p50 small–moderate improve; p95 and especially the 2048+ bucket clearer improve. Both p50 and p95 must not regress. This PR does not report a tip delta. C1 will be remeasured after this rebase. Remeasure is not in this commit.

Tip SHA: 56d4cc4c5fab5a193fd4b524b0bf626c43e0aaf6

Test Plan

System1-Omni Version / Commit: tip 56d4cc4c5fab5a193fd4b524b0bf626c43e0aaf6 on cursor/instant-v-pack-248b, git base a41e379453d4a8dc3cbe3f76c8754753c42230a1. Measured C1 baseline remains 51f80ad681c8f7b1be9b22b5e700bef34e33534c.

CPU, no GPU:

  • cargo test -p omni-qwen3-5-native --test v_pack --locked
  • cargo test -p omni-startlux-native --test contract --locked

Parent GPU smoke (not run here). Same protocol and the same recorded hardware as the baseline. runtime_revision must be the tip SHA above.

  1. Rebuild the Rust worker. No new CUDA symbol. An existing ABI 9 library that already exports cs1_attention_gated_bidirectional and cs1_copy2d is enough. Rebuilding that library is unnecessary for this tip.
  2. Serve Instant with thinking enabled: false. Same manifest, precision, request order, and cache policy as the baseline. Warmup 5, concurrency 1, three measured rounds. Clock post_to_body_json_and_schema_validation.
  3. Pool latency_ms from completed rows with null error. Recompute p50 and p95 from the pooled samples. Do not average the three round percentiles. Report the serving pass rate and the 2048+ bucket.
  4. Dual gate against 7.338 / 51.816 ms. Both p50 and p95 must not regress. Record device L20X (the nvidia-smi string) and compute capability 8.9 on the paste.
  5. Output parity is unverified. Run python benchmarks/bench.py compare against the baseline responses if decisions or probabilities can move. The pack is intended to copy the same V values.
  6. Do not stack tip2 (scratch pre-size) until this remeasure is conclusive. Do not merge.

Commands for the measured rounds are in recipe/startlux/latency.md.

Test Result

Post-rebase CPU checks on tip 56d4cc4c5fab5a193fd4b524b0bf626c43e0aaf6, rustc 1.99.1:

  • cargo test -p omni-qwen3-5-native --test v_pack --locked — 3 passed
  • cargo test -p omni-startlux-native --test contract --locked — 9 passed

Earlier, on the pre-rebase tip of the same diff: qwen lib 25 passed / 14 ignored (GPU), config, device_features, graph_loading, and image_decode passed, and clippy -D warnings on the two crates was clean. Those were not re-run after the rebase.

Not run: CUDA compile, GPU numerical parity, Public231 C1 remeasure. No latency delta is claimed.

Demo / evidence

N/A for a video. This is a data-layout change with no UI.

Unverified: Public231 C1 p50/p95 on the tip SHA, 2048+ bucket, serving pass rate, and bench.py compare versus the baseline outputs. The baseline paste above is the owner-supplied measurement of 51f80ad681c8f7b1be9b22b5e700bef34e33534c, not a result of this commit or of a41e379.

Controls to keep on the remeasure: warmup 5, concurrency 1, three rounds, thinking enabled: false, clock post_to_body_json_and_schema_validation, device L20X (nvidia-smi string), compute capability 8.9. A different device string or capability is a separate group. runtime_revision must be 56d4cc4c5fab5a193fd4b524b0bf626c43e0aaf6.

Self-review

Before marking this PR ready for review or requesting maintainer review, complete
the self-review checklist.
Keep the PR in draft while this work is incomplete.
For agent assistance, use the optional precheck-pr skill.

  • I have reviewed the full diff and addressed the issues I found.
  • I have checked that the change follows the project's architecture and stays focused on the stated purpose.
  • I have run the checks appropriate to this change and reported commands, results, and anything I could not verify above.
  • I have checked that the PR description, documentation, and any accuracy or performance claims match the implementation and available evidence.
Open in Web Open in Cursor 

RSI phase: layout/contiguity (data path — not bare kernel swap)

Primary change: After Instant QKV, pack V into contiguous [T,Hk,Dh]
scratch mirroring aq/ak and call cs1_attention_gated_bidirectional with
ldv = Hk*Dh (1024 on Instant 4B) instead of leaving V in attn_in with
ldv = attn_in (10240).

Alternatives considered (not this tip):
(a) change QKV weight layout for contiguous V — larger ABI churn
(b) Instant CUDA graphs — incompatible with bidirectional
(c) cuBLASLt plan-cache / gemm warm|compact|shape — rejected standing
(d) scratch pre-size only — tip2, separate, not included

Baseline:
- runtime_revision: 51f80ad (origin/main)
- Public231 C1 pooled p50/p95: 7.338 / 51.816 ms; pass 693/693; warmup 5; C1; 3 rounds
- device: L20X (nvidia-smi string); compute capability: 8.9
- thinking: enabled:false; clock: post_to_body_json_and_schema_validation
- 2048+ bucket: n=108, p50 50.346 / p95 71.942

Expected: dual gate — pooled p50 small–moderate improve; p95 and especially
2048+ bucket clearer improve. Both p50 and p95 must not regress.
Remeasure is not in this commit. Causal attention still uses the QKV stride.

Co-authored-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants