Repository navigation
Bench: Add the docgen perf engine framework and the compodoc engine - #35636
Conversation
2cf56c8 to
25afc98
Compare
4a2eeb4 to
fd4e8c8
Compare
📝 WalkthroughWalkthroughThe change introduces a typed docgen performance suite with configurable repetition profiles, reusable benchmark engines, synthetic Angular project generation, Compodoc execution and documentation metrics, strict child-process handling, aggregation utilities, tests, and the Compodoc dependency. ChangesCompodoc benchmark suite
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CompodocEngine
participant AngularGenerator
participant CompodocCLI
participant DocumentationCounter
CompodocEngine->>AngularGenerator: generateAngularProject(scenario)
CompodocEngine->>CompodocCLI: run cold documentation generation
CompodocCLI-->>CompodocEngine: documentation.json and RSS metrics
CompodocEngine->>CompodocEngine: modify one component and run warm measurement
CompodocEngine->>DocumentationCounter: countDocumentation(documentation.json)
DocumentationCounter-->>CompodocEngine: members and opaqueTypes
✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/bench/docgen-perf/engines/compodoc.ts`:
- Around line 57-70: Update the RSS polling logic around the setInterval
callback and peakRssKb to track whether any valid RSS sample was collected and
whether polling errors persist. Before reporting the benchmark result, detect an
all-error or all-zero measurement and surface it as unavailable or unreliable
instead of returning peakRssMb: 0 as a valid metric, while preserving normal
reporting when valid samples exist.
- Around line 40-94: Update runCompodocOnce to enforce a timeout for the spawned
Compodoc child: terminate the process when the limit is exceeded, clear the
polling interval and timeout, and reject the promise with a descriptive timeout
error. Ensure normal close and spawn-error paths clean up the timeout without
changing their existing results.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bba9c5d6-6a5a-4516-a09f-4d7f94da38a7
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
scripts/bench/docgen-perf/aggregate.test.tsscripts/bench/docgen-perf/aggregate.tsscripts/bench/docgen-perf/config.tsscripts/bench/docgen-perf/engine.tsscripts/bench/docgen-perf/engines/compodoc-doc.test.tsscripts/bench/docgen-perf/engines/compodoc-doc.tsscripts/bench/docgen-perf/engines/compodoc.tsscripts/bench/docgen-perf/generators/angular.tsscripts/bench/docgen-perf/spawn.tsscripts/bench/docgen-perf/types.tsscripts/bench/docgen-shared/engine-ids.tsscripts/package.json
fd4e8c8 to
dc0af52
Compare
The orchestrator settles on the child's close event, so a compodoc that hangs - which it can, on some inputs - stalled the whole suite with no way out. The run is now killed after ten minutes and reported as a timeout, so the engine fails on its own rather than taking the suite with it. Reported by CodeRabbit on #35636.
dc0af52 to
c11bb76
Compare
The orchestrator settles on the child's close event, so a compodoc that hangs - which it can, on some inputs - stalled the whole suite with no way out. The run is now killed after ten minutes and reported as a timeout, so the engine fails on its own rather than taking the suite with it. Reported by CodeRabbit on #35636.
c11bb76 to
1322ce3
Compare
Package BenchmarksCommit: No significant changes detected, all good. 👏 |
f5cb06a to
6992914
Compare
This is the base of the per-engine perf suite: the metric types, the fixed measurement profiles, the aggregation that turns repetitions into medians, the child-process spawn helper, and the BenchEngine base class every engine implements. It comes with the first engine, compodoc, which measures Angular. Compodoc runs as a fresh CLI process, so cold extraction and whole-project scan are the same measurement and peak memory is sampled from outside the child. The engine also reads the documentation.json compodoc writes, and reports two counts: how many members it documented, and how many of those carry a type it never resolved. The second count matters because compodoc never expands a named type. Chain a type through twenty files and it documents every member at the same speed, having looked through none of them, so a member count alone would read as identical to an engine that expanded all of them. @compodoc/compodoc is pinned at exactly 2.0.0, the version the Angular docgen baselines capture against. Its cost moves between versions. The profile and the engine id list carry Angular only. React and Vue arrive with their engines in the next PR. There is no CLI entry point yet. registry.ts, cli.ts and run.ts land in the PR stacked on top, so this cannot be run end to end; the two test files cover the aggregation and the documentation.json parsing.
Collapse coldMembers/warmMembers/coldOpaqueTypes into one members() hook
returning MemberCounts, and give BenchEngine an assemble() that turns a
scenario's repetitions into its ScenarioResult.
Both engines implemented the three count hooks as pass-through one-liners, and
the orchestrator had to call all three plus aggregate() and pick the designated
repetition itself - which meant naming a sample shape it cannot know, so the
generic parameter was erased and re-asserted with a cast. assemble() keeps
Sample bound inside the class, so the cast has nowhere to live, and picking the
designated repetition happens in one place instead of two. Sample is now
constrained to { coldMs: number }, which is what designatedRep has always
required and what the erased generic hid.
Pin N at 6 rather than 5. The two sides of a control pair alternate which runs
first by repetition parity, so an odd N hands one side the first slot once more
than the other, and the cold median then lands on a repetition from the majority
slot - turning the ordering effect the alternation exists to cancel into a
systematic bias on the headline ratio.
Report compodoc's peak RSS as n/a when nothing was sampled. `ps` is POSIX-only
and a run can finish inside one poll interval, and the initial 0 was reported as
a measured peak on either path. oneShotMetrics now requires every repetition to
carry a reading before it reports a mean.
Also: read the compodoc RSS poll interval from its constant rather than back out
of the params bag, and say in the "retained metrics missing" error that a series
shorter than two saves reaches it too.
The orchestrator settles on the child's close event, so a compodoc that hangs - which it can, on some inputs - stalled the whole suite with no way out. The run is now killed after ten minutes and reported as a timeout, so the engine fails on its own rather than taking the suite with it. Reported by CodeRabbit on #35636.
The generator was a sequence of writes, so the only way to see what compodoc is pointed at was to run it and look at a directory. angularProjectFiles returns the project as path -> contents instead, and generateAngularProject writes that map. The bookkeeping of which paths are components stops being tracked separately from the writes that create them. The new test snapshots that map: the tree, the fake @angular/core that keeps it hermetic, the tsconfig include that decides the measured set, and a component at zero and four extra props. Generated output is byte-identical.
6992914 to
3892cd0
Compare
The orchestrator settles on the child's close event, so a compodoc that hangs - which it can, on some inputs - stalled the whole suite with no way out. The run is now killed after ten minutes and reported as a timeout, so the engine fails on its own rather than taking the suite with it. Reported by CodeRabbit on #35636.
The orchestrator settles on the child's close event, so a compodoc that hangs - which it can, on some inputs - stalled the whole suite with no way out. The run is now killed after ten minutes and reported as a timeout, so the engine fails on its own rather than taking the suite with it. Reported by CodeRabbit on #35636.
The orchestrator settles on the child's close event, so a compodoc that hangs - which it can, on some inputs - stalled the whole suite with no way out. The run is now killed after ten minutes and reported as a timeout, so the engine fails on its own rather than taking the suite with it. Reported by CodeRabbit on #35636.
The orchestrator settles on the child's close event, so a compodoc that hangs - which it can, on some inputs - stalled the whole suite with no way out. The run is now killed after ten minutes and reported as a timeout, so the engine fails on its own rather than taking the suite with it. Reported by CodeRabbit on storybookjs#35636.
What I did
This is the base of the new per-engine docgen performance suite, plus its first engine.
The framework: metric types, the measurement profiles, the aggregation that turns N repetitions into medians, the child-process spawn helper, and the
BenchEnginebase class every engine implements.The engine: Compodoc, which is what Storybook shells out to for Angular docgen. It runs as a fresh CLI process, so cold extraction and whole-project scan are the same measurement, and peak memory is sampled from outside the child.
Heads-up: there is no CLI entry point yet.
registry.ts,cli.tsandrun.tsall land later, so you cannot run this end to end here.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
aggregate.test.tsandcompodoc-doc.test.ts, on top of part 2's tests — 4 files, 43 tests.Manual testing
Expect 4 test files and 43 tests passing.
compodoc-doc.test.tscounts against the real Compodoc captures already committed incode/lib/docgen-harness, so it checks the counter against output the tool actually produced, not against a hand-written fixture.There is no runnable benchmark at this point in the stack — that first becomes possible in part 4.
Documentation
Checklist for Maintainers
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsqa:neededorqa:skipbuildSummary by CodeRabbit
New Features
Tests