Repository navigation
perf: budget the scenarios added in #177 and #180, and compare the fastest processes of each build - #194
Merged
Conversation
…the measured noise
|
Coverage after merging perf/scenario-budgets into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This was referenced Oct 8, 2026
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
1 participant
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.
Part of #168.
The CI performance budgets covered 8 of the 93 scenarios. The 26 that #177 and #180 added (Map and Set values, object records, class instances, the deep path, push and insert, patch application, returned values, and the
current()searches) were validated in CI but had no budget, so their regressions went undetected. This PR budgets all 26, after measuring the noise between identical builds on GitHub runners. The measurements also showed why the existing gate failed on identical builds, as on #193, so the timing comparison changes as well.budgets.json(schema version 2) holds four groups:core, the eight existing scenarios and two memory cases, unchanged;collections;objects; andapply-return-search.node perf-testing/ci.mjs --group NAMEruns one group, all groups in turn without it, and the workflow runs one job per group in 6–8 minutes, where the 98 new timing cells would have made one job take about 17. The gate derives each group's cells from the scenario registry, so patch application runs with patches off only, and rejects unknown groups or scenarios and scenarios budgeted in two groups; a unit test checks that the workflow matrix lists every group. Each job runs the budget tests instead of all benchmark tool tests, which Node CI already runs, so the occasionalmemory.test.mjsfailure on GitHub does not now fail four jobs.map-update-10pct,set-update-10pct,object-update-10pct,class-wide-update,push-and-insert-reuse,apply-reverse,return-replace, andsearch-draft.perf-testing/README.mddescribes the groups, the comparison, and the measured noise.Noise floor
Before the new groups got budgets, a temporary workflow compared identical builds with
--self-controlon 30 GitHub runners, 10 per group: run 37817492628, whose branch has since been deleted. All 30 runs pass the final policy.collectionsmap-insert, auto-freeze)objectsclass-update, 3.4 µs, below the 500 ns floor)object-delete, auto-freeze)apply-return-searchsearch-current-shifted, patches)No cell exceeded 1.30 in any split, and only
object-deletewith auto-freeze exceeded 1.20. Smaller cases have no memory budget because the retained heap ofdeep-updatewith patches, 1.8 KiB per output, measured 13–36 KiB in 23 of 120 processes of both builds, and a retained-heap budget failed one of the 30 runs on it at 12.8x.Findings outside this PR
class-wide-updatewithout freezing (a class instance with 1,000 fields) ran 2.4–3.9 times slower in all 40 processes that ran its freeze-on cells first, and at most 1.08 times slower in the 60 others. Both processes of a pair run in the same order, so the budget compares like with like, but the effect itself may deserve a look.Verification
pnpm test:benchmarks: 35 tests pass; every commit passes the budget tests, the perf-testing format check, and lint.node perf-testing/ci.mjs --self-control --group core: all 48 decisions pass.main: the four jobs took 5.9–7.4 minutes, and all 206 decisions passed, at most 1.053 for timing, 1.140 for allocation, and 1.007 for retained heap.