Skip to content

PhaseLog::Close's pre-lock wait is charged to nobody, so it inflates the closing record's UNCOVERED time — the quantity the coverage floor reads #1719

Description

@localai-bot

PhaseLog::Open reads a clock BEFORE it takes the process-wide mutex — const std::chrono::steady_clock::time_point entered = std::chrono::steady_clock::now(); in src/vllm/multimodal/render_phase_log.cpp — and charges that lock wait to whatever encloses the gap before the new record. PhaseLog::Close has no such read. It takes the mutex and only then stamps r.end, so the wall between a scope's last statement and its recorded end is instrument time that NO record and NO table absorbs: it is not in Record::instrument_seconds, not in PhaseLog::Instrument(), and therefore not in unaccounted_seconds's explanation.

It is not cosmetic. That interval lies inside the CLOSING record's own duration and outside every child of it, which is exactly the quantity covered >= min_coverage * leaf_seconds reads — the floor #1439, #1470, #1494 and #1536 have spent months arguing about. A Close that waits on a contended mutex therefore makes the coverage floor see an un-named phase, and the number a reader would subtract to see through that does not include it. #1668 item 4 landed instrument_seconds precisely so that residue could be separated into "the render did not name this" and "the instrument spent this"; this wait falls into neither.

Found by the fresh review of PR #1711, as a by-product of attempting the per-site charge gate in the sibling issue — it is the reason that attempt could not be made honest, because contention deliberately staged through PhaseLog::Records() lands mostly in this uncharged wait rather than in the site under test.

NOT FIXED IN FLOW. It is a production change to a shared instrument that four render paths run through, and it owes its own red-first evidence and its own mutation. It also interacts with the charge clamp PR #1711 lands: a Close that charges a pre-lock interval charges it to the record that is about to end, so the per-target high-water mark seeded at o.start is what keeps the charge inside [start, end] and the conservation invariant instrument_seconds <= duration_seconds intact. Doing it before that clamp existed would have broken the invariant again.

Owned by .agents/specs/ltx25-phase-instrument.md ## Owed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions