perf(ci): surface keyed-list allocation in the /perf comment#702
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Surfaces allocation metrics for the keyed-list /perf leg (StressPerf.KeyedList) in the rendered PR comment by adding an allocation sub-table to the keyed-list section, using the existing allocation metric spec and the same paired-Δ 95% CI machinery already used elsewhere in the perf comment.
Changes:
- Extend
Format-PerfKeyedListSectionto append an Allocation (keyed-list) markdown table (Alloc bytes/render, Gen0 GC / 1k renders) when allocation metrics are present. - Add PowerShell unit tests to verify the keyed allocation sub-table renders/omits correctly and remains distinct from the StocksGrid allocation table.
- Update the perf CI README to document the new keyed allocation sub-table and its intent.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/stress_perf/ci/PerfLib.ps1 | Adds a keyed-list allocation sub-table to the keyed-list section, gated on alloc metrics being present. |
| tests/stress_perf/ci/PerfLib.Tests.ps1 | Adds assertions covering rendering, ordering, omission gating, and distinction from the StocksGrid allocation table. |
| tests/stress_perf/ci/README.md | Documents that keyed-list output includes its own allocation sub-table and when it appears. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The keyed-list leg (StressPerf.KeyedList) already captures the full alloc set in metrics.json (allocBytesPerRender, gen0PerKRenders) via the shared PerfTracker, but Format-PerfKeyedListSection rendered only the four headline metrics, so the keyed-DIFF allocation signal was invisible. Allocation on the keyed (ReconcileKeyed -> ReconcileKeyedMiddle / LIS) path scales with reorder volume in a way the positional StocksGrid allocation table can never isolate; that signal is exactly what keyed-diff alloc-reduction work needs measured. Render an "Allocation (keyed-list)" sub-table inside the keyed section, reusing the existing $PerfAllocMetricSpec over the keyed aggregates with the identical paired-delta 95% CI machinery as the headline + StocksGrid alloc tables. Gated on the keyed leg reporting alloc metrics (n/a for a legacy head). Pure render reuse, no workload or metrics.json change. Tests: +8 assertions in PerfLib.Tests.ps1 (sub-table renders, bytes/render + Gen0 rows, alloc-down reads improvement, ordering after the headline table, omitted when keyed aggregates lack alloc, and distinct from the StocksGrid "Allocation (Reactor)" table in a full comment). PerfLib.Tests 226 green. Docs: ci/README.md keyed-list bullet + the Format-PerfKeyedListSection doc-comment and tests/stress_perf/METHODOLOGY.md keyed-list section synced to mention the allocation sub-table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b3ffc52 to
debacca
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The keyed-list
/perfleg (StressPerf.KeyedList) already captures the full allocation set inmetrics.json(allocBytesPerRender,gen0PerKRenders) via the sharedPerfTracker, butFormat-PerfKeyedListSectionrendered only the four headline metrics — so the keyed-diff allocation signal was invisible in the comment.This adds an Allocation (keyed-list) sub-table inside the keyed-list section, reusing the existing
$PerfAllocMetricSpecover the keyed aggregates with the identical paired-Δ 95% CI machinery as the headline and StocksGrid allocation tables.Allocation on the keyed (
ReconcileKeyed→ReconcileKeyedMiddle/ LIS) path scales with reorder volume in a way the positional StocksGrid allocation table can never isolate — the exact signal keyed-diff alloc-reduction work (e.g. #657) needs measured.Why it's safe
metrics.jsonchange. The keyed aggregates already carryAllocBytesPerRender/Gen0PerKRenders(+Spread/Samples) viaMeasure-PerfRuns, exactly like the StocksGrid leg.$hasAllocguard.tests/stress_perf/ci/**, 0src/Reactor(Class-B harness-only).Rendered output
Below the keyed headline metrics, the section now shows (real magnitudes from a
StressPerf.KeyedList@50% run, alloc reduced ~20%):Tests
+8 assertions in
PerfLib.Tests.ps1: sub-table renders,bytes/render+Gen0rows present, alloc-down reads improvement, ordering after the headline table, omitted when keyed aggregates lack alloc, and distinct from the StocksGridAllocation (Reactor)table in a full comment.PerfLib.Tests226 green;RunPerfBenchmark.Tests39 green.Part of the "make
/perfvaluable" harness roadmap (follows PR1–PR4 + #700). Class-B harness-only.