telemetry: split expert hit counter into pin vs ecache (LRU) (#336)#367
Closed
KingIcyCreamProjects wants to merge 1 commit into
Closed
telemetry: split expert hit counter into pin vs ecache (LRU) (#336)#367KingIcyCreamProjects wants to merge 1 commit into
KingIcyCreamProjects wants to merge 1 commit into
Conversation
m->hits counted a pin-tier hit and an LRU-ecache hit identically, which makes every cache-policy question unanswerable from logs: the pin tier is checked first and (with a warm usage profile) absorbs most hot experts, so the ecache may serve anywhere from ~0% to a large share of hits — prefill-pollution effects, eviction-policy A/Bs (JustVugg#223), and PILOT_REAL eviction pressure all need that split before any measurement means much. Two new counters (hit_pin, hit_ecache) bumped at the two Phase C lookup branches, surfaced in: - the periodic serve stats line: hit N% (lru M%) — pin share = N-M - run_replay summary: expert hit N% (pin X% + lru Y%) - prof_report: hit N% (X pin + Y lru / Z load) The serve-mux PROF protocol line is untouched (openai_server.py parses it positionally). Both reset sites clear the new counters; model_init's memset zeroes them at start. Zero cost: two increments on paths that already increment m->hits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
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.
What
Splits the single
m->hitsexpert-cache counter intohit_pin(HOT-STORE / pinned experts, never evicted) andhit_ecache(per-layer LRU). By constructionhit_pin + hit_ecache == hitsat every increment — the twom->hits++sites inmoe()each gain a 1:1 tier-specific bump, so the total is preserved with no double-count. Resolves #336.Why
Today
hitslumps together two very different events: a pin-tier hit (free, the expert is wired in RAM) and an ecache/LRU hit (subject to the eviction policy). That makes eviction-policy changes unmeasurable — you can't tell whether a policy "won" because it improved LRU reuse or because the pin tier simply absorbed everything.This directly unblocks the evaluation of #223 (cloxcache/SLRU): the maintainer's gating question there — "the lowest cap at which cloxcache starts to win" — is unanswerable while
hitsconflates the tiers. With the split, a cap sweep shows whether a flat A/B is "pin tier absorbed all the traffic" vs. a genuine LRU-policy floor.Surface
glm.conly, +19/−14. New fields onModel; snapshot/reset updated in lockstep at both counter-reset sites; windowed deltas added.emit_stream(lru%),[PROF](X pin + Y lru / Z load), andREPLAY(pin X% + lru Y%).hits. Cannot change model output (telemetry only).Notes for the reviewer
[PROF]protocol line is intentionally left untouched —openai_server.pyparses it positionally, so the pin/ecache breakdown is surfaced only in the human-facing report paths above.moe()lookup branches (both add to the pin/ecache lookup lines). No semantic conflict — this only reads/writes new counters, Nearly double the hit-rate #223 only changes victim selection — so whichever lands second is a trivial keep-both merge.