Skip to content

Refine benchmark reports and fix FFmpeg stderr draining - #405

Open
jarcherNV wants to merge 4 commits into
mainfrom
dev/jarcher/benchmarkfixes
Open

Refine benchmark reports and fix FFmpeg stderr draining#405
jarcherNV wants to merge 4 commits into
mainfrom
dev/jarcher/benchmarkfixes

Conversation

@jarcherNV

Copy link
Copy Markdown
Collaborator

Fold derived benchmark summary metrics into their base metric rows so the HTML report shows the median-focused view developers expect. Constrain the scenario table layout and collapse long command strings behind a details control to avoid wide commands crowding adjacent columns.

Drain FFmpeg stderr concurrently while streaming video frames to stdin to avoid subprocess pipe-buffer deadlocks during MP4 writing.

Fold derived benchmark summary metrics into their base metric rows so the
HTML report shows the median-focused view developers expect. Constrain the
scenario table layout and collapse long command strings behind a details
control to avoid wide commands crowding adjacent columns.

Drain FFmpeg stderr concurrently while streaming video frames to stdin to
avoid subprocess pipe-buffer deadlocks during MP4 writing.
@greptile-apps

greptile-apps Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Refines benchmark report presentation and prevents FFmpeg stderr pipe deadlocks.

  • Adds parser provenance to benchmark metric metadata and uses it to fold only genuine perf-summary metrics.
  • Preserves base metric medians when derived summary metrics share their base name.
  • Simplifies metric tables to a median-focused view and constrains scenario table command rendering.
  • Drains FFmpeg stderr concurrently while video frames stream to stdin.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
flashdreams/flashdreams/infra/runner_io.py Concurrently drains FFmpeg stderr during frame streaming and joins the drain thread before processing diagnostics.
flashdreams/tools/benchmarks/harness.py Persists per-metric record type, parser, and source provenance in scenario manifests.
flashdreams/tools/benchmarks/metrics.py Labels perf-summary and profile-e2e records with explicit parser provenance.
flashdreams/tools/benchmarks/report.py Uses provenance-gated metric folding, retains existing base statistics, and presents a constrained median-focused report layout.
flashdreams/tests/test_benchmark_harness.py Covers parser provenance, safe metric folding, base-median preservation, suffix-like custom metrics, and report layout.
flashdreams/tests/test_runner_io.py Verifies that stderr draining begins before FFmpeg frame writes proceed.

Sequence Diagram

sequenceDiagram
  participant Runner
  participant FFmpeg
  participant DrainThread
  Runner->>FFmpeg: Stream video frames through stdin
  FFmpeg-->>DrainThread: Emit stderr diagnostics
  DrainThread->>DrainThread: Accumulate stderr chunks
  Runner->>FFmpeg: Close stdin and wait
  Runner->>DrainThread: Join
  DrainThread-->>Runner: Complete stderr output
Loading

Reviews (4): Last reviewed commit: "Avoid overwriting base benchmark metrics" | Re-trigger Greptile

Comment thread flashdreams/tools/benchmarks/report.py
Record metric-summary provenance in benchmark manifests and use it when
rendering HTML reports, so derived perf-summary metrics are only folded when
they came from parsed log-summary records.

This keeps median-focused report output for known benchmark perf summaries
while preserving custom metric names such as latency_median_s from stats JSON.
Record parser provenance for log-derived benchmark metrics and carry it into
metric_summary_metadata so the HTML report can distinguish generated perf
summary metrics from arbitrary profile metrics.

Only fold suffix-shaped metrics when they came from the perf-summary parser,
preserving profile e2e metric names such as latency_median_s in reports.
Comment thread flashdreams/tools/benchmarks/report.py
Make report metric folding explicitly skip existing base metric rows so a
perf-summary-derived value such as generate_s_median_s cannot replace the
median computed from generate_s samples.

Add regression coverage for conflicting base and derived medians, and apply
ruff formatting expected by the CPU CI pre-commit run.
@jarcherNV
jarcherNV enabled auto-merge August 1, 2026 21:22
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.

1 participant