[bong-water-water-bong]

What this does

Stats gains four phase timers — ms_step, ms_batch, ms_draft, ms_commit — accumulated with steady_clock::now() around each phase of the draft loop, and the live report prints one split line per prompt plus one for the run under MTP_PHASE_SPLIT=1.

Measurement only: nothing here feeds back into a decision the loop makes. Emitted text, acceptance, per-depth tables and the drafter's stream are bit-identical to the same binary without the flag (p0: 44/80, chain 3.20, 90/65/35/30, TEXT IDENTICAL, 0 mismatches).

Why it is needed

This lane's cost model for the draft route was assembled by subtraction across tickets: "two passes per iteration" came from counting calls, and the price of each term was inferred from arms measured at different load. Two candidate changes have been weighed on that model — remove the plain step, or widen the batch — and both are decided by the difference, so the four terms are now measured in one window.

Measured (bonsai2-27b-pb16i, sealed M=16 pb16i ELFs, one npu-hold claim, amdxdna errors 0)

Prompt 0, 64 tokens, k=4, tau=0, 20 iterations, box load 53-57, plain decode 0.479 s/token:

phase	ms	share
step (the single-token pass)	11477.6	28.1 %
batch (verify, M=5)	18334.9	44.9 %
head (chain + lm head per draft)	7466.3	18.3 %
commit (commit_verify + drafter advance)	3554.7	8.7 %
total = draft phase	40833.4	2042 ms/iteration

The two decisions this settles

1. Folding the plain step into the verify batch cannot buy the 25 % it was priced at. A one-pass loop must draft from the previous committed row (npu/mtp/draft.h:246 is what produces the base hidden that :261 hands to MtpDrafter::draft(), and npu/mtp/mtp_head.cpp:367 chains from it). That first draft is then a self-regressive head step — depth-2 quality, 65 % here, instead of depth-1's 90 %. On measured terms:

2042 - 574 (step) + 87 (one extra batch row) + 81 (one extra head step) = 1636 ms/iteration
chain 1 + 0.65 + 0.35 + 0.30 + 0.25 = 2.55 tokens/pass
1636 / 2.55 = 642 ms/token   vs   2042 / 3.20 = 638 ms/token today

A dead heat, so the change is not carried in this PR. One earlier arm corroborates the direction: k=6 bought chain 3.25 for about 0.84 units of cost more than k=4 (depth 5 hit 20 %, depth 6 hit 0 %).

2. The batch, not the loop's shape, is where a draft-route win has to come from. Verifying five rows costs 917 ms/iteration while a pb16i launch runs its rows M-independently, so that 45 % is per-row host work. For the serve switch that means: the draft route's remaining lever is step_batch_all's per-row cost and the base step, not another loop rearrangement.

Cost of the flag

steady_clock::now() twice per iteration, ~40 ns against a 2 s iteration; the accumulators are four doubles in Stats. It prints nothing unless MTP_PHASE_SPLIT is set, so CI and the toy test are unaffected (ctest -R mtp_draft green, tools/copyright.py --check clean).

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 365400b)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Phase Timing Accuracy The phase timing logic in run function uses lap(tp) to accumulate time for each phase, but the timing starts only after the initial prompt processing and may not capture the full duration of each phase, especially for the first iteration. This could lead to underreporting of actual phase durations. // Phase timing: always accumulated (steady_clock::now() is ~20 ns against a ~0.5 s pass), printed by the // caller when it wants the split. Measurement only -- nothing here is read back into the loop's decisions. using Clock = std::chrono::steady_clock; auto lap = [](Clock::time_point& t0) { const auto t1 = Clock::now(); const double ms = std::chrono::duration<double, std::milli>(t1 - t0).count(); t0 = t1; return ms; }; Clock::time_point tp = Clock::now(); Phase Split Reporting The phase split reporting in mtp_acceptance.cpp aggregates timing data across prompts, but does not account for potential differences in workload or hardware state between prompts, which could skew the reported percentages. // The whole run's split, so the serve switch and any loop change are priced on one window's measurement. if (phase_split && tot_ms_step + tot_ms_batch + tot_ms_draft + tot_ms_commit > 0) { const double tot = tot_ms_step + tot_ms_batch + tot_ms_draft + tot_ms_commit; const double pc = 100.0 / tot; std::printf(" phase split (all prompts): step %.1f%% + batch %.1f%% + head %.1f%% + commit %.1f%%" " = %.1f s of draft phase; of that, base passes %.1f%%\n", tot_ms_step * pc, tot_ms_batch * pc, tot_ms_draft * pc, tot_ms_commit * pc, tot / 1000.0, (tot_ms_step + tot_ms_batch) * pc); }

[bong-water-water-bong]

VERDICT: READY at 14d982051

Measurement-only, and I checked that it is: the four accumulators are written and printed, and nothing reads them back —

grep for ms_step|ms_batch|ms_draft|ms_commit in npu/mtp/draft.h, excluding the declarations, phase_ms() and the += sites
  -> nothing

and the header parses and runs standalone, so the change is not entangled with the tool that prints it:

g++ -std=c++17 -fsyntax-only -I. (a TU that includes npu/mtp/draft.h)   -> OK
onebit::npu::mtp::Stats{}.phase_ms()                                    -> 0.000

The placement of the five lap() calls matches the phases the body claims — two ms_step (the loop pays two single-token passes per iteration), one ms_draft (the head's own chain), one ms_batch (verify batches, array + host), one ms_commit (snapshot restore + replay) — and tp is seeded once before the loop, so no phase double-counts its predecessor's tail.

The reason given for measuring all four in one window is the right one and worth keeping in the commit message: "two passes per iteration" only prices a loop change if the passes carry the cost, and subtracting one phase across two runs is exactly how a number ends up attributed to the wrong thing.

Two notes:

1. lap() is unconditionally accumulated with the justification "steady_clock::now() is ~20 ns against a ~0.5 s pass". That is true of a real pass, but MTP_PHASE_SPLIT also runs on the 16-token acceptance arms where a step can be ~10 ms — still negligible (4 clamps ≈ 80 ns), but if this ever lands in a hot path (per-token, not per-pass) it should be gated like NPU_PHASE_PROF is.
2. The claim "bit-identical to the same run without the flag" is the one thing I could not check without a device; it follows from nothing being read back, which is what I verified, so I am satisfied — but if you have the two acceptance logs, quoting the text/acceptance rows side by side is cheaper for the next reviewer than trusting the argument.

[github-actions]

Persistent review updated to latest commit 365400b