Skip to content

chore: extract PerfSampler from AutoPilot for AltTester reuse - #8826

Merged
popuz merged 16 commits into
devfrom
feat/test-automation/autopilot-alttester
May 22, 2026
Merged

chore: extract PerfSampler from AutoPilot for AltTester reuse#8826
popuz merged 16 commits into
devfrom
feat/test-automation/autopilot-alttester

Conversation

@popuz

@popuz popuz commented May 19, 2026

Copy link
Copy Markdown
Collaborator

What does this PR change?

Extracts per-frame CPU/GPU sampling out of AutoPilot into a reusable PerfSampler so the same code path can produce perf summaries for both:

  1. The existing standalone --autopilot-csv / --autopilot-summary CLI flow (byte-identical output).
  2. AltTester-driven InWorld fixtures in explorer-automation, which call PerfSampler.Begin / End over the AltTester bridge to bracket a test fixture instead of a 90 s StandAtSpawnAsync loop.

Changes

  • New PerformanceAndDiagnostics/AutoPilot/PerfSampler.csConfigure(IProfiler) / Begin(csvPath, summaryPath) / End() static API. Owns the per-frame PlayerLoop hook, the CSV writer, the 8-line summary writer, and the PercentWorst aggregation. Same IProfiler.LastFrameTimeValueNs / LastGpuFrameTimeValueNs signal source as before — no new ProfilerRecorder instances, no analytics-pipeline dependency.
  • Refactored AutoPilot.csRunAsync now brackets StandAtSpawnAsync(90) with PerfSampler.Begin / End. Application.Quit stays in finally and is only reachable from the standalone path; AltTester-driven sampling never quits the player.
  • ProfilingPlugin.cs — calls PerfSampler.Configure(profiler) unconditionally (outside the AUTOPILOT flag gate) so the InWorld path works without --autopilot.

Compatibility

CSV columns, line endings (CRLF), encoding (UTF-8 no BOM), and the eight summary lines (CPU average, CPU 1% worst, CPU 0.1% worst, CPU worst, GPU average, GPU 1% worst, GPU 0.1% worst, GPU worst) are byte-identical to the pre-refactor output. The Part B consumer in explorer-automation parses the same file with one regex regardless of which path produced it.

Test Instructions

Standalone path — same as before this PR:

<Player>.exe --autopilot-csv perf.csv --autopilot-summary perf-summary.txt <wallet flags>

Wait ~3 min (load + 90 s stand). Player exits. perf-summary.txt should contain the same 8 lines as a pre-refactor build.

AltTester-driven path is exercised by the companion PR — no manual steps needed here:

Quality Checklist

  • Standalone --autopilot-summary output verified byte-identical to pre-refactor.
  • PerfSampler.Configure placement reviewed — outside the AUTOPILOT gate so the InWorld path works.
  • No new using directives for analytics namespaces in PerfSampler.cs (signal source is IProfiler only).

  Move per-frame CPU/GPU sampling, CSV writer, 8-line summary writer, and
  PercentWorst out of AutoPilot.RunAsync into a new static PerfSampler
  (Configure/Begin/End). AutoPilot now brackets StandAtSpawnAsync with
  PerfSampler.Begin/End and keeps Application.Quit in finally.
  ProfilingPlugin calls PerfSampler.Configure(profiler) unconditionally,
  so AltTester-driven InWorld fixtures can drive Begin/End via
  AltDriver.CallStaticMethod without --autopilot. CSV columns, line
  endings (CRLF), encoding (UTF-8 no BOM), and the eight summary lines
  are byte-identical to the pre-refactor output.
@popuz popuz self-assigned this May 19, 2026
@popuz popuz added the force-build Used to trigger a build on draft PR label May 19, 2026
@github-actions

github-actions Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

badge

New build in progress, come back later!

@popuz popuz added force-build Used to trigger a build on draft PR and removed force-build Used to trigger a build on draft PR labels May 19, 2026
  PercentWorst computed k as (int)(times.Count * fraction), which floors
  to 0 whenever Count * fraction < 1 — e.g. the 0.1% bucket on any window
  shorter than 1000 samples. Take(0).Average() then threw
  InvalidOperationException, killing the whole summary write.

  Standalone AutoPilot never hit this because StandAtSpawnAsync yields
  ~90s of samples on a dev machine. Fixture-level InWorld windows on the
  T4 chassis (~15-20s, low FPS) regularly land below 1000 samples and
  crashed the entire perf-summary writer.

  Floor k to 1 so 0.1% worst on small windows simply reports the worst
  single frame instead of crashing. Output is byte-identical for any
  window with >= 1000 samples (k = (int)(N * 0.001f) >= 1 already).
  Also guard the empty-list case defensively.
popuz added a commit to decentraland/explorer-automation that referenced this pull request May 20, 2026
  Bump cloud_build_url default to pr-22430-3a0b8d5, which carries the
  PerfSampler.PercentWorst guard against tiny sample windows
  (decentraland/unity-explorer#8826).

  Improve AttachPerf diagnostics: log ex.ToString() instead of just
  ex.Message so AltTester's wrapped TargetInvocationException reveals the
  real Player-side stack in the Allure report. Best-effort attach
  perf.csv on the End-failure path — PerfSampler.End disposes the CSV
  before WriteSummary runs, so a summary-side crash still leaves a
  complete CSV on disk that can be inspected post-mortem.
@popuz popuz added the no QA needed Used to tag pull requests that does not require QA validation label May 20, 2026
@popuz popuz changed the title feat(perf): extract PerfSampler from AutoPilot for AltTester reuse feat: extract PerfSampler from AutoPilot for AltTester reuse May 20, 2026
@popuz popuz changed the title feat: extract PerfSampler from AutoPilot for AltTester reuse chore: extract PerfSampler from AutoPilot for AltTester reuse May 20, 2026
@popuz popuz removed the force-build Used to trigger a build on draft PR label May 20, 2026
@popuz
popuz marked this pull request as ready for review May 20, 2026 10:57
@popuz
popuz requested review from a team as code owners May 20, 2026 10:57
@github-actions
github-actions Bot requested review from mikhail-dcl and pravusjif May 20, 2026 10:57
@popuz popuz removed the no QA needed Used to tag pull requests that does not require QA validation label May 20, 2026
@popuz popuz changed the title chore: extract PerfSampler from AutoPilot for AltTester reuse chore: extract Sampler from AutoPilot for AltTester reuse May 20, 2026
@popuz popuz changed the title chore: extract Sampler from AutoPilot for AltTester reuse chore: extract PerfSampler from AutoPilot for AltTester reuse May 20, 2026
@popuz
popuz requested review from m3taphysics and removed request for mikhail-dcl May 20, 2026 10:59
@popuz popuz added the perf_test Run Performance Tests from the PR label May 20, 2026
@popuz
popuz marked this pull request as draft May 20, 2026 12:44
@popuz popuz removed the perf_test Run Performance Tests from the PR label May 20, 2026
@popuz
popuz marked this pull request as ready for review May 20, 2026 12:44
@claude

This comment has been minimized.

@m3taphysics

Copy link
Copy Markdown
Contributor

PR #8826, run #26192309600

Builds: Windows change, Windows baseline, macOS change, macOS baseline

Framework 13 i7

Metric Change Baseline Delta Improvement
Samples 2701 2700
CPU average 33.3 ms 33.3 ms -0.0 ms 0.0%
CPU 1% worst 33.7 ms 33.9 ms -0.2 ms 0.5%
CPU 0.1% worst 36.1 ms 37.6 ms -1.5 ms 3.9% 🟢
GPU average 8.2 ms 7.7 ms 0.5 ms -6.8% 🔴
GPU 1% worst 19.4 ms 19.3 ms 0.1 ms -0.6%
GPU 0.1% worst 25.7 ms 24.1 ms 1.6 ms -6.4% 🔴

@popuz popuz added the no QA needed Used to tag pull requests that does not require QA validation label May 21, 2026
@popuz
popuz removed the request for review from Ludmilafantaniella May 21, 2026 08:15
@claude

This comment has been minimized.

@github-actions github-actions Bot removed the no QA needed Used to tag pull requests that does not require QA validation label May 21, 2026
@claude

This comment has been minimized.

@m3taphysics

Copy link
Copy Markdown
Contributor

PR #8826, run #26217949913

Builds: Windows change, Windows baseline, macOS change, macOS baseline

Framework 13 i7

Metric Change Baseline Delta Improvement
Samples 2701 2700
CPU average 33.3 ms 33.3 ms 0.0 ms -0.0%
CPU 1% worst 33.6 ms 33.6 ms 0.0 ms -0.1%
CPU 0.1% worst 35.1 ms 35.6 ms -0.5 ms 1.4%
GPU average 7.6 ms 7.7 ms -0.1 ms 1.4%
GPU 1% worst 18.4 ms 20.0 ms -1.6 ms 8.0% 🟢
GPU 0.1% worst 24.7 ms 25.1 ms -0.4 ms 1.7%

@pravusjif pravusjif left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we extend some virtual input simulation for Alttester tests in the future, we can have automatized waypoint-based paths in-world, measuring performance thanks to this initiative, great job!

@claude

This comment has been minimized.

@popuz popuz added the no QA needed Used to tag pull requests that does not require QA validation label May 21, 2026
@popuz

popuz commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

since we have proper performance report - it means that behavior wasn't broken and it acts as a simple smoke test for run of explorer. Thus - no QA needed
image

@claude

claude Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @popuz's task in 1m 59s —— View job


PR Re-review — chore: extract PerfSampler from AutoPilot for AltTester reuse

  • Read project documentation
  • Read changed files (current state)
  • Check if previous issues are resolved
  • Post review findings

Root-cause check

This PR extracts per-frame CPU/GPU sampling out of AutoPilot into a reusable static PerfSampler so the same code path can be driven by both the standalone --autopilot-csv CLI flow and AltTester-driven InWorld fixtures. The diff addresses the cause directly — no symptom-masking patterns found.


Progress since last review

# Issue Status
1 Dead profiler field in AutoPilot.cs ✅ Fixed
2 Inverted error message / incorrect summary guard in AutoPilot.cs ✅ Fixed — summaryFile is now optional via TryGetValue with no throw
3 Debug.LogWarning instead of ReportHub in PerfSampler.cs ✅ Fixed
4 Missing exception handling in SampleLoopAsync ✅ Fixed
5 LINQ violation in PerfSampler.cs ❌ Not fixed

Blocking Issue

LINQ violation — PerfSampler.cs lines 7, 154, 160, 162, 168, 185 — CLAUDE.md rule: "Do not use LINQ — it allocates too much memory."

OrderByDescending, Take, and Average in PercentWorst (line 185) plus Average() and Max() in WriteSummary (lines 154, 160, 162, 168) all allocate. using System.Linq (line 7) is still present. See inline comment for the exact fix.

On the "it only runs in CI" argument: the code lives in Explorer/ and ships in every Player build. The standalone --autopilot-csv path runs in production binaries. CLAUDE.md has no carve-out for call frequency or deployment context.


REVIEW_RESULT: FAIL ❌
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Introduces a new static class with a UniTaskVoid fire-and-forget sampling loop, modifies plugin initialization wiring in ProfilingPlugin.InjectToWorld, and changes runtime perf-data collection behaviour that ships in the Unity player.
QA_REQUIRED: YES

@github-actions github-actions Bot removed the no QA needed Used to tag pull requests that does not require QA validation label May 21, 2026
@m3taphysics

Copy link
Copy Markdown
Contributor

PR #8826, run #26233457658

Builds: Windows change, Windows baseline, macOS change, macOS baseline

Framework 13 i7

Metric Change Baseline Delta Improvement
Samples 2701 2700
CPU average 33.3 ms 33.3 ms -0.0 ms 0.0%
CPU 1% worst 33.5 ms 33.6 ms -0.1 ms 0.3%
CPU 0.1% worst 33.9 ms 35.3 ms -1.3 ms 3.8% 🟢
GPU average 7.5 ms 7.9 ms -0.3 ms 4.3% 🟢
GPU 1% worst 20.4 ms 18.4 ms 1.9 ms -10.5% 🔴
GPU 0.1% worst 25.5 ms 24.7 ms 0.8 ms -3.4% 🔴

@popuz
popuz merged commit 71ac3b4 into dev May 22, 2026
19 of 20 checks passed
@popuz
popuz deleted the feat/test-automation/autopilot-alttester branch May 22, 2026 09:02
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