Skip to content

fix(engine): restore discarded warmup timing samples - #5204

Merged
jrusso1020 merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/discarded-warmup-timing
Oct 8, 2026
Merged

jrusso1020 merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/discarded-warmup-timing

Conversation

@user-github-me

Copy link
Copy Markdown
Contributor

discardWarmupCapture() restores scalar performance counters but shallow-copies capturePerf.frameMs. The inner capture appends to that shared array, so discarded samples remain in the p50/p95/p99 summary on both success and failure.

Copy the timing samples by value before capture. Extend the existing restoration tests to use real sample arrays and verify the full performance summary, including an injected capture that replaces sample contents.

Refs #4435 and the accounting review on #2489. This fixes the exported helper's accounting contract. The helper still has no production call sites; this does not fix blank worker-boundary frames or add a warm-up capture.

Validation:

  • Four restoration regressions fail on main with leaked or replaced samples.
  • Warm-up, percentile, frame-capture and parallel-coordinator suites: 129 tests passed.
  • Engine typecheck/build, repository lint, changed-file formatting and applicable pre-commit gates passed.
  • No capture-mode, seek, compositor timestamp or rendering changes.

@jrusso1020 jrusso1020 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.

Good catch. The snapshot copied capturePerf shallowly, so the inner capture pushed its timing into the same frameMs array, and the discarded sample stayed in the percentile summary. Copying the array by value fixes that. The new tests compare the full perf summary on both the success and failure paths, so they would catch a regression. Small and correct, thanks.

— Rames

@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 8, 2026
Merged via the queue into heygen-com:main with commit 971e137 Oct 8, 2026
88 checks passed
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