Skip to content

Add "bytes read" metric to per-prompt statistics - #62

Open
laanwj wants to merge 1 commit into
sqliteai:mainfrom
laanwj:2026-09-bytes-read
Open

Add "bytes read" metric to per-prompt statistics#62
laanwj wants to merge 1 commit into
sqliteai:mainfrom
laanwj:2026-09-bytes-read

Conversation

@laanwj

@laanwj laanwj commented Sep 3, 2026

Copy link
Copy Markdown

Add a "bytes read" metric to per-prompt statistics. The "/stats" command in chat already shows this, but the stats for "waste run" didn't.

The "/stats" command already shows this, but the stats for "waste run" didn't.
@mfethe1

mfethe1 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Independent CPU/synthetic-container audit at 656af11ed49a83fce74792a3d58c139a57ae53b9 (base c66c7b3febf261590abea540e74602d8e7759d62) found two small but useful follow-ups:

  1. The stats format string contains GB read\]. Clang emits unknown escape sequence '\]'; removing that backslash removes the new warning. Base had no warnings under the same default flags.
  2. The new sum is callback/decode-only, whereas the existing engine stats include work outside those callbacks. Chunked prefill precedes the per-token bb baseline, so summing callback deltas cannot count those reads. Using the same label for these different scopes makes the measurement easy to misinterpret.

On a tiny synthetic container, callback bytes were 0 versus engine bytes 282624; adding the omitted term in the measurement control closed the difference, and a fixed-seed repeat reproduced it. That is not a general omission percentage: this fixture fits entirely in cache, so it cannot establish the magnitude under decode-time cache pressure. Whole-run accounting also needs explicit before/after boundaries rather than assuming an absolute counter is per-request.

Project make check stayed 60 passed / 0 failed / 14 skipped on both base and PR.

Would you prefer the deliberately narrower label decode GB read, or a whole-request metric with a before/after counter boundary and regression coverage? I can contribute a focused companion patch once the intended metric is settled, rather than create a competing implementation.

@mfethe1

mfethe1 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Concrete minimal option for the scope question: keep the existing callback sum, make its narrow scope explicit, and correct the unit label (the divisor is 1ULL << 30, so GiB, not decimal GB). This avoids changing accounting semantics while you decide whether a whole-request metric belongs in a separate change.

Applied locally on this PR's unchanged head 656af11ed49a83fce74792a3d58c139a57ae53b9:

diff --git a/cli/main.c b/cli/main.c
index b19d2e7..ae916f6 100644
--- a/cli/main.c
+++ b/cli/main.c
@@ -876,7 +876,7 @@ static int run_segs(waste_ctx *c, const opts *o, const seg *segs, int ns,
     if (show_stats && s.n) {
         const double sec = s.ms / 1000.0;
         fprintf(stderr, "\n[%u tokens, %.2f s, %.2f tok/s | experts %llu hit / "
-                        "%llu miss = %.0f%% | %.2f GB read\]\n",
+                        "%llu miss = %.0f%% | %.2f decode GiB read]\n",
                 s.n, sec, s.n / sec,
                 (unsigned long long)s.hit, (unsigned long long)s.miss,
                 100.0 * (double)s.hit / (double)(s.hit + s.miss ? s.hit + s.miss : 1),

Verification on macOS CPU: fresh default make -j2 succeeds with zero compiler warnings; make check gives 55 passed / 0 failed / 18 skipped in this environment. The skip set includes missing uv and external tokenizer/model fixtures; this is not the previous environment's 60-pass run and not real-checkpoint validation. The edit changes only the printed label and removes the invalid C escape; it does not change the accumulated counter, fix omitted prefill reads, or claim whole-request accounting.

No competing branch/PR was pushed. This is an independently built, tested suggestion for the author to incorporate if callback/decode scope is intended. AI-assisted contribution.

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