perf(loading): Phase 2 groundwork — benchmark harness, aggregate load profiler, baseline#10429
perf(loading): Phase 2 groundwork — benchmark harness, aggregate load profiler, baseline#10429davidfirst wants to merge 8 commits into
Conversation
…ase 2 baseline phase 2 of the component-loading redesign is gated on a committed benchmark harness and a recorded baseline. scripts/bench-component-loading.js measures wall-time (median of warm runs) and peak RSS for bit status/list/show/graph on this workspace. baseline recorded in the redesign doc's section 4; status (11s) and graph (20s, 2gb) are the hotspots the rest of phase 2 targets.
BIT_LOAD_PROFILE=1 accumulates every span's self-time into a process-wide per-stage table dumped at exit, giving the whole-command aggregate that the per-component debug-load can't. used to record where bit status/graph spend their load time for Phase 2 prioritization. zero cost when the env var is unset.
…tatus cost is structural)
Code Review by Qodo
1. Bench inherits server env
|
PR Summary by Qodoperf(loading): add benchmark harness + opt-in aggregate load-stage profiler WalkthroughsDescription• Add a rerunnable benchmark script measuring warm wall-time and peak RSS for load-heavy Bit commands. • Add BIT_LOAD_PROFILE aggregate per-stage self-time profiling, dumped once at process exit. • Record Phase-2 baseline metrics and profiling conclusions to guide redesign priorities. Diagramgraph TD
A["Developer"] --> B["bench-component-loading.js"] --> C["bit CLI commands"] --> D["load-trace spans"]
D --> E["BIT_LOAD_PROFILE=1"] --> F["stderr profile table"]
B --> G["/usr/bin/time RSS"]
B --> H["redesign doc baseline"]
High-Level AssessmentThe following are alternative approaches to this PR: 1. Implement aggregation via existing SpanEmitter wiring
2. Emit machine-readable profile output (JSON) instead of stderr-only text
Recommendation: Current approach is appropriate for Phase-2 groundwork: it’s opt-in (env-gated), requires no additional CLI wiring, and has near-zero overhead when disabled. If the profiler becomes long-lived tooling (or needs CI consumption), consider migrating aggregation behind the existing SpanEmitter with optional JSON output, to avoid module-level process hooks and to standardize profile export. File ChangesEnhancement (2)
Documentation (1)
Other (1)
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
Code review by qodo was updated up to the latest commit 2397ec9 |
|
Code review by qodo was updated up to the latest commit d746290 |
…ded no-trace spans
|
Code review by qodo was updated up to the latest commit 7077e40 |
|
Code review by qodo was updated up to the latest commit 7a31ae7 |
| function runOnce(bin, args) { | ||
| const fullArgs = timeAvailable ? [timeFlag, bin, ...args] : args; | ||
| const fullBin = timeAvailable ? TIME_BIN : bin; | ||
| const start = process.hrtime.bigint(); | ||
| const res = spawnSync(fullBin, fullArgs, { | ||
| stdio: ['ignore', 'ignore', 'pipe'], | ||
| encoding: 'utf8', | ||
| maxBuffer: 1024 * 1024 * 64, | ||
| }); | ||
| const wallMs = Number(process.hrtime.bigint() - start) / 1e6; | ||
| const ok = res.status === 0; | ||
| const rssBytes = timeAvailable ? parsePeakRssBytes(res.stderr) : undefined; | ||
| return { wallMs, rssBytes, ok, stderr: res.stderr }; |
There was a problem hiding this comment.
2. Bench inherits server env 🐞 Bug ≡ Correctness
The benchmark harness spawns bit without normalizing environment variables, so if BIT_CLI_SERVER* is set it may run via the long-running bit-server path and measure a different execution path/latency, producing non-comparable baselines.
Agent Prompt
## Issue description
The harness uses `spawnSync()` without an `env` override, so the spawned `bit` process inherits the caller’s environment. If the caller has `BIT_CLI_SERVER` / `BIT_CLI_SERVER_TTY` / `BIT_CLI_SERVER_PTY` enabled, Bit will route through the background bit-server, changing startup/load behavior and invalidating the benchmark’s reproducibility.
## Issue Context
This repo supports a long-running background `bit-server` mode selected by env vars, and `runBit()` chooses that mode based on `shouldUseBitServer()`.
## Fix Focus Areas
- scripts/bench-component-loading.js[89-101]
## Suggested fix
- Pass an explicit `env` to `spawnSync` that disables server/daemon execution for benchmark runs, e.g.:
- `BIT_CLI_SERVER=0`
- `BIT_CLI_SERVER_TTY=0`
- `BIT_CLI_SERVER_PTY=0`
- (optionally) `BIT_DAEMON=0`
- Consider printing a note if any of these env vars were set in the parent, to make misconfiguration obvious.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit e61a311 |
2 participants
Groundwork for Phase 2 of the component-loading redesign (
scopes/workspace/workspace/component-loading-redesign.md). Tooling + measurement only — no change to load behavior.scripts/bench-component-loading.js— benchmark harness: wall-time (median of warm runs) + peak RSS forbit status/list/show/graph. Re-runnable at every phase boundary.BIT_LOAD_PROFILE=1— opt-in aggregate per-stage load profiler in@teambit/harmony.modules.load-trace; sums each stage's self-time across a whole command (the per-command aggregate that the per-componentdebug-loadcan't give). Zero cost when unset.Key finding that corrects the doc's pre-profiling assumptions: the dependency FS cache works (warm
bit status= 635 cache hits, 0 misses).status's dominant warm cost (~7s) is dependency-object materialization on cache hit — structural, belongs to the staged-loading phase, not a quick fix.bit graph's dominant load cost is file-content reads. This reorders the Phase-2 plan around evidence.