Skip to content

Extract shared realtime timing helpers#387

Merged
jarcherNV merged 3 commits into
mainfrom
dev/jarcher/realtime-timing
Jul 20, 2026
Merged

Extract shared realtime timing helpers#387
jarcherNV merged 3 commits into
mainfrom
dev/jarcher/realtime-timing

Conversation

@jarcherNV

Copy link
Copy Markdown
Collaborator

Extract shared realtime timing helpers

Move the OmniDreams realtime timing records, trace context, video-model
stage tracing, and input-to-present profile window into
flashdreams.serving.realtime.timing. Add shared duration and rolling
summary helpers, optional decode/cache stage timings, and migrate the
OmniDreams interactive demo and tests to consume the shared module while
keeping the old import path as a compatibility shim.

@greptile-apps

greptile-apps Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extracts OmniDreams-specific realtime timing types and helpers into a new canonical module at flashdreams.serving.realtime.timing, then migrates all call sites to import from the shared location while keeping the old omnidreams.interactive_drive.runtime.timing path as a thin compatibility re-export shim.

  • New canonical module (flashdreams/serving/realtime/timing.py) consolidates ChunkTimes, FrameTimes, ChunkPrediction, TraceContext, TraceSink, VideoModelTimings (extended with optional decode/cache_update fields), ChunkHistory, RollingChunkTimingSummary, InputToPresentProfileWindow, and stage-duration summary helpers.
  • Compatibility shim (integrations/omnidreams/.../runtime/timing.py) is replaced with a flat re-export plus __all__, preserving all previously public symbols without breaking existing imports.
  • emit_video_model_timing_ranges is a new shared helper that centralises the condition → model → cache_update → decode → merge trace emission, including is not None guards on optional event IDs that fix the zero-valued-event dependency bug identified in prior review threads.

Confidence Score: 5/5

Safe to merge; the refactoring is a clean extract with no logic changes to existing production paths.

All production behaviour is preserved: the global e2e profiling accumulators are replaced by an equivalent InputToPresentProfileWindow object, the trace-event dependency chain in emit_video_model_timing_ranges uses explicit is not None guards fixing the zero-valued-ID corruption from prior review threads, and VideoModelTimings gains only optional fields with defaults keeping all existing constructors valid. The compatibility shim correctly re-exports every previously public symbol. No correctness regressions were found.

The new canonical module at flashdreams/serving/realtime/timing.py could benefit from an all declaration to match the shim and make the public API explicit, but this does not affect correctness.

Important Files Changed

Filename Overview
flashdreams/flashdreams/serving/realtime/timing.py New canonical shared timing module; logic is sound and the is-not-None guards on optional event IDs correctly address previous review findings. Missing all is a minor completeness gap.
flashdreams/tests/test_realtime_timing.py Comprehensive tests covering zero-valued event IDs, optional subranges, percentile math, and profile-window rollover; all assertions match expected behavior.
integrations/omnidreams/omnidreams/interactive_drive/runtime/timing.py Correctly converted to a pure re-export shim with all; preserves all previously public symbols for backwards compatibility.
integrations/omnidreams/omnidreams/interactive_drive/runtime/loop.py Global profiling state replaced with InputToPresentProfileWindow; behavior equivalent to old manual accumulator pattern, interval_s re-read from env var on each call as before.
integrations/omnidreams/omnidreams/interactive_drive/video_model/chunk_pipeline.py Trace emission delegated to emit_video_model_timing_ranges; final_event_id correctly replaces the old frame_merge event variable, preserving downstream dependency edges.
integrations/omnidreams/omnidreams/interactive_drive/types.py VideoModelTimings removed from types.py and imported from the canonical module; existing callers constructing with 6 positional fields remain compatible due to the 4 new optional-with-default fields.
integrations/omnidreams/tests/interactive_drive/test_chunk_pipeline.py Import paths updated to canonical module; no logic changes.
integrations/omnidreams/tests/interactive_drive/test_latency_loop.py Import paths updated to canonical module; no logic changes.
integrations/omnidreams/tests/interactive_drive/test_latency_timing.py Single import path update only.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph before["Before (all in omnidreams)"]
        OT["omnidreams.interactive_drive\n.runtime.timing\n(full implementation)"]
        OTypes["omnidreams.interactive_drive\n.types\n(VideoModelTimings here)"]
        Loop1["runtime/loop.py\n(manual global accumulators)"]
        Pipeline1["chunk_pipeline.py\n(inline trace ranges)"]
        App1["app.py / cli.py"]
        OT --> Loop1
        OT --> Pipeline1
        OT --> App1
        OTypes --> Pipeline1
    end

    subgraph after["After (shared canonical module)"]
        FT["flashdreams.serving.realtime\n.timing\n(canonical module)"]
        Shim["omnidreams.interactive_drive\n.runtime.timing\n(re-export shim)"]
        Loop2["runtime/loop.py\n(InputToPresentProfileWindow)"]
        Pipeline2["chunk_pipeline.py\n(emit_video_model_timing_ranges)"]
        App2["app.py / cli.py"]
        FT --> Shim
        FT --> Loop2
        FT --> Pipeline2
        FT --> App2
        Shim -. "backwards-compat" .-> Loop2
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    subgraph before["Before (all in omnidreams)"]
        OT["omnidreams.interactive_drive\n.runtime.timing\n(full implementation)"]
        OTypes["omnidreams.interactive_drive\n.types\n(VideoModelTimings here)"]
        Loop1["runtime/loop.py\n(manual global accumulators)"]
        Pipeline1["chunk_pipeline.py\n(inline trace ranges)"]
        App1["app.py / cli.py"]
        OT --> Loop1
        OT --> Pipeline1
        OT --> App1
        OTypes --> Pipeline1
    end

    subgraph after["After (shared canonical module)"]
        FT["flashdreams.serving.realtime\n.timing\n(canonical module)"]
        Shim["omnidreams.interactive_drive\n.runtime.timing\n(re-export shim)"]
        Loop2["runtime/loop.py\n(InputToPresentProfileWindow)"]
        Pipeline2["chunk_pipeline.py\n(emit_video_model_timing_ranges)"]
        App2["app.py / cli.py"]
        FT --> Shim
        FT --> Loop2
        FT --> Pipeline2
        FT --> App2
        Shim -. "backwards-compat" .-> Loop2
    end
Loading

Reviews (4): Last reviewed commit: "Add coverage for ChunkHistory iterator c..." | Re-trigger Greptile

Comment thread flashdreams/flashdreams/serving/realtime/timing.py Outdated
Comment thread flashdreams/flashdreams/serving/realtime/timing.py Outdated
Comment thread flashdreams/flashdreams/serving/realtime/timing.py Outdated
Comment thread flashdreams/flashdreams/serving/realtime/timing.py
@gtong-nv

Copy link
Copy Markdown
Collaborator

I think we might need to revisit this profiling logic later. Currently only the omnidream-interactive demo uses this logic. Technically other interactive demo can re-use it, but the interface is not very clear.

@gtong-nv gtong-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Base automatically changed from dev/jarcher/media-refactor to main July 20, 2026 21:22
@jarcherNV

Copy link
Copy Markdown
Collaborator Author

I think we might need to revisit this profiling logic later. Currently only the omnidream-interactive demo uses this logic. Technically other interactive demo can re-use it, but the interface is not very clear.

Yeah I imagine @ArielG-NV may want to update our timing metrics as part of his benchmark work this week, so this one may go through a bunch of updates.

@jarcherNV
jarcherNV enabled auto-merge July 20, 2026 21:25
@jarcherNV
jarcherNV disabled auto-merge July 20, 2026 21:25
Move the OmniDreams realtime timing records, trace context, video-model
stage tracing, and input-to-present profile window into
flashdreams.serving.realtime.timing. Add shared duration and rolling
summary helpers, optional decode/cache stage timings, and migrate the
OmniDreams interactive demo and tests to consume the shared module while
keeping the old import path as a compatibility shim.
Preserve zero-valued trace event IDs when wiring optional cache and decode
stage dependencies, update the ChunkHistory iterator annotation, and remove
a dead profile-window count guard. Add regression coverage for trace sinks
that return event ID 0 for optional timing stages.
Add a realtime timing test that verifies ChunkHistory.__iter__ returns an
Iterator and yields appended chunks in order, making the iterator contract
from the review feedback explicit in test coverage.
@jarcherNV
jarcherNV force-pushed the dev/jarcher/realtime-timing branch from e3507a8 to a562201 Compare July 20, 2026 21:31
@ArielG-NV

ArielG-NV commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

I am unsure on if I would want to use the current tooling here for timing or to eventually replace it all. Unlikely I use it in its current state though.

Eventually, either we will add to what we have or replace it so that any timing metrics we collect give us an annotated nvtx timeline for agentic development and profiling sanity reasons.

We can still merge the changes though with/without the existing code - if I won't use it, it will just be code I strip out naturally when I propose the benchmark/regression-test changes.

@jarcherNV
jarcherNV added this pull request to the merge queue Jul 20, 2026
Merged via the queue into main with commit 7478155 Jul 20, 2026
8 checks passed
@jarcherNV
jarcherNV deleted the dev/jarcher/realtime-timing branch July 20, 2026 22:15
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.

3 participants