diff --git a/docs/roadmap/performance.md b/docs/roadmap/performance.md new file mode 100644 index 0000000..b34bbda --- /dev/null +++ b/docs/roadmap/performance.md @@ -0,0 +1,265 @@ +# Computational efficiency roadmap + +Status: proposed; implementation has not begun. + +Date: 2026-10-09. + +## Purpose and authority + +Reduce repeated work and avoidable allocations in CommandKit's command dispatch, +registration, and inspection reports. Favor straightforward changes with clear +benefits, and use measurements to decide whether more complex optimizations are +worth maintaining. + +This document owns the proposed improvements, priorities, and completion +evidence. The [architecture policy](../policy/architecture.md), accepted +[ADRs](../adr/), [Go declarations](../../app.go), and +[CLI reference](../cli.md) continue to govern behavior. Follow the +[agent policy](../policy/agents.md) for proportionate implementation and the +[testing policy](../policy/testing.md) for validation. These proposals do not +establish new public performance or concurrency guarantees. + +## Reviewed baseline + +The efficiency review examined commit `54cfbd8` with a clean worktree. Its +findings come from source inspection; no benchmarks or allocation profiles were +run. Complexity observations identify scaling costs, not measured bottlenecks +or promised speedups. Recheck the affected implementation before starting work. + +| ID | Observed work | Proposed result | Priority | +| --- | --- | --- | --- | +| PF-01 | PromptKit text reports are built even for YAML requests. | Render only the requested format and avoid redundant report buffers. | First pass | +| PF-02 | Selected-command lookup scans a map; each ancestor is rediscovered from the root. | Direct declaration lookup and ancestry traversal proportional to path depth. | First pass | +| PF-03 | Every invocation translates and sorts metadata; help and usage construct another backend tree. | Reuse immutable metadata and the invocation's prepared tree. | Measure and improve after the first pass | +| PF-04 | Aggregate YAML framing splits every line and copies the complete native document. | Inspect only the opening lines; assess eliminating the complete framed copy. | Prefix scanning in the first pass; copying afterward | +| PF-05 | Each registration copies the existing top-level registry. | Prepare new entries separately and install a validated batch atomically. | Lower priority | +| PF-06 | Scope ordering, lookup, and selection repeat; selector duplicate checks scan previous entries. | Prepare stable scope metadata once and use direct duplicate checks. | Lower priority | + +The implemented package inventory remains owned by the +[internal overview](../internal/overview.md). The target is internal improvement +within the existing root facade and Cobra adapter; no additional public package +or consumer registration API is proposed. + +## Target behavior and ownership + +Optimizations preserve the existing contracts rather than changing operator or +consumer workflows: + +- Keep Cobra/pflag objects and operations inside the internal adapter, following + [ADR 0003](../adr/0003-public-facade.md). +- Keep parsed values, collections, and presence independent between invocations. + [ADR 0002](../adr/0002-cobra-backend.md) permits safe reuse; rebuilding backend + storage is an implementation choice. Caller-owned state retains its lifecycle. +- Preserve registration validation, reserved command ownership, and complete + batch validation before changing the registry, as documented in + [command declarations](../../command.go) and [registration](../../registry.go). +- Keep metadata construction, help, and version independent of provider + acquisition under the architecture policy. +- Preserve owning-library rendering and confidentiality semantics, aggregate + document order, and acquisition of all selected views before `config show` + output under [ADR 0007](../adr/0007-native-yaml-reports.md). +- Preserve cancellation, diagnostic delivery, outcome classification, and + reporting of partial output and write failures under + [ADR 0005](../adr/0005-invocation-outcomes.md). + +Registration and command metadata belong to the root implementation. Backend +indexes and invocation machinery belong to `internal/cobra`. Report formatting +and scope preparation remain with the corresponding root handlers. Acquisition, +configuration resolution, and provider lifetime remain with the existing +integration contracts and consuming applications. + +## Proposed improvements + +### PF-01: Render only the requested PromptKit format + +In [PromptKit handlers](../../promptkit.go), `runPromptKitPrompt` calls +`formatPromptText` before `writePromptKitReport` chooses a format. +`runPromptKitProfile` similarly calls `formatProfileText` unconditionally. The +profile text formatter serializes `ExtraParams` as nested YAML; a YAML request +then serializes those parameters again as part of the structured report. Text +delivery also copies an already completed report into another builder. + +Choose the requested formatter before rendering. Use a straightforward buffer or +completed report representation that avoids the redundant text-buffer copy. +Continue using the existing structured report representation and YAML encoder; +the optimization concerns when rendering occurs and how its result is delivered. + +Completion evidence should show unchanged text and YAML report semantics, +including `ExtraParams`, diagnostics, and delivery failures, with eliminated +unused text construction for YAML requests. Measure prompt input collections and +profile parameter maps large enough to expose the redundant work. + +### PF-02: Index command declarations and ancestry + +In the [Cobra adapter](../../internal/cobra/parse.go), `declarationFor` scans the +declaration-to-backend map to find a selected backend command. `pathFor` calls +`parentOf` from the root for every ancestor. For N command nodes and selected +depth D, repeated parent searches can take O(N × D) work; a chain can produce +quadratic work. + +Record a backend-to-declaration mapping during construction. Build parent links +alongside it, or traverse Cobra's existing parent relationships internally. +Prefer an implementation that reconstructs only the selected path rather than +storing a separate complete path for every node. Declaration lookup should be +constant time, with path construction proportional to D after tree preparation. + +Exercise shallow, wide, and deeply nested declarations through dispatch, help, +and usage. Preserve exact registered-path matching and error classification. +This improvement removes redundant searches; constructing the complete backend +tree still has a separate cost addressed by PF-03. + +### PF-03: Reduce command-tree reconstruction + +[Metadata translation](../../parser.go) copies and sorts the registered tree on +every invocation, including flag and operand declarations. The adapter then +builds backend commands and flag sets for all nodes. Help and usage rendering +build another complete backend tree after parsing has already built one. + +Pursue two improvements: + +1. Retain the invocation's prepared backend tree inside the adapter and use it + for parsing, help, and usage. The root coordinates these operations without + receiving Cobra/pflag objects. Keep the internal coordination API narrow. +2. Prepare or lazily cache immutable bridge metadata and sorted child order per + `App`. Refresh that derived state after successful registration. Failed + registration must leave the previously registered commands usable, including + their help and dispatch behavior. + +Metadata caching principally benefits repeated `Execute` calls. Avoiding a +second backend construction also benefits a single CLI help or usage request. +Measure both cases separately, including the cost of app construction and +registration for consumers that execute only once. + +Reusing mutable backend trees across different invocations is a later option, +conditional on measured benefit and a clear account of state reset and value +ownership. The first target can retain fresh backend parse state for each +invocation. No public freeze step, backend selector, or concurrent-dispatch +guarantee is needed for these improvements. + +### PF-04: Reduce aggregate YAML scanning and copying + +In [configuration display](../../show.go), `hasYAMLDirective` and +`frameEffectiveYAML` split the complete document into lines before examining its +opening content. These splits scan all bytes and allocate a slice entry for each +line, even when the framing decision can be made immediately. Framing then +allocates a complete copy alongside the native buffer. + +First, scan the preamble with byte offsets, stopping when the directive or marker +decision is complete. Share the resulting framing analysis where useful so +directive detection and scope-label placement do not independently scan the same +preamble. Avoid a document-wide line index and unnecessary string conversions. + +Then evaluate delivering separators, labels, and native byte segments directly, +retaining one native render buffer without a second complete framed copy. Keep +the design simple enough to preserve failure handling for multiple writes. + +Preserve native bytes, directive placement, inline document content and block +scalars, scope-label escaping, and boundary newlines according to the CLI +contract. Keep single-scope native delivery direct. Aggregate acquisition remains +complete before output; the current code buffers one rendered document at a time, +not every scope's rendered bytes simultaneously. + +Measure documents with many short lines as well as large values. The initial +target removes avoidable scans and allocations; complete rendering and delivery +will still require work proportional to output size. + +### PF-05: Avoid repeated registry copying + +[Registration](../../registry.go) copies all existing top-level entries for each +`Register` call. Registering R top-level commands individually therefore produces +O(R²) cumulative map-copy work, apart from validation and declaration cloning. +This is a construction cost, not work performed by a registered handler. + +Validate declarations, check collisions against both existing and pending +entries, and prepare owned new entries in temporary storage. Install them only +after the complete batch succeeds. Preserve declaration ownership and atomic +validation without copying every existing map entry. + +Compare individual and batch registration at representative sizes. Batch +registration is already available to consumers; adopting it is an optional +consumer optimization, not a new requirement. Coordinate this work with PF-03's +metadata refresh so registration has one clear completion point. + +### PF-06: Prepare stable configuration scope metadata + +[Scope selection](../../selectors.go) sorts scope declarations and repeatedly +searches the scope slice. Aggregate selection and handler acquisition each look +up every scope by name, producing quadratic scope-search work. Selection also +occurs during preflight and again in the handler. In +[suite normalization](../../config.go), selector duplicate checking compares each +selector with all previous selectors; PromptKit already uses name and short-name +maps for the equivalent check. + +Prepare stable scope order and a name lookup during suite registration, keeping +that derived data private. Use direct name and short-name checks for selector +duplicates. Assess retaining the selected scopes in invocation-owned input after +preflight if repeated selection remains significant; keep selector applicability +and per-invocation contexts fresh. + +Measure the normal small scope set before adopting additional indexes for +selector applicability. Preserve scope order, declared operation availability, +and scope-aware selector validation under +[ADR 0009](../adr/0009-configuration-scopes.md). Do not invoke acquisition while +preparing this metadata. + +## Recommended sequence and measurement + +1. Establish focused before-change measurements for PF-01, PF-02, and PF-04, + then implement conditional formatting, direct command lookup/ancestry, and + YAML prefix scanning as independently reviewable changes. +2. Measure complete dispatch paths and implement PF-03's reuse within an + invocation and immutable metadata preparation where the benefit justifies + the coordination and retained memory. +3. Evaluate PF-04's segmented delivery, PF-05, and PF-06 using the first-pass + evidence and expected consumer sizes. Record a deferral if a proposed change + adds complexity without a worthwhile benefit. +4. Consider backend reuse across invocations only if the remaining measured cost + warrants it. + +Use Go benchmarks with controlled, offline fixtures when implementation work is +authorized. Keep external loading and service latency out of isolated CommandKit +measurements, and identify when an integration measurement includes provider +work. Include: + +| Area | Representative comparisons | +| --- | --- | +| Dispatch | Small and larger registries; wide and deep trees; few and many flags; successful commands, help, and usage errors. | +| Lifecycle | Construction, registration, and first execution versus repeated execution on an existing `App`. | +| Reports | Prompt text/YAML; profile text/YAML with varied parameter maps; aggregate configuration documents with varied size and line count. | +| Registration and scopes | Individual/batch registration; small and larger scope and selector sets. | + +Record elapsed cost, bytes allocated, and allocations per operation with the Go +version, machine, fixture dimensions, and benchmark scope. Use repeated samples +for comparisons; profile CPU or allocation behavior when it helps explain a +remaining cost. Account for memory retained by caches as well as allocation +reductions. Avoid combining every fixture dimension into a large benchmark +matrix without a specific question. + +Choose behavioral validation according to each change's risks and existing +coverage. Important cases include fresh presence and collections across runs, +registration after earlier execution, failed registration, help without provider +loading, YAML framing, cancellation, and partial output. Behavioral tests should +protect the existing contracts; benchmark results assess efficiency without +locking in private data structures or exact helper-call sequences. + +These measurements guide implementation choices. They do not establish fixed +timing thresholds, allocation budgets, or additional release gates. + +## Completion and documentation + +For each PF item, record whether it was implemented, deferred, or narrowed, +alongside its change reference, relevant behavioral validation, and measurement +results. An unmeasured improvement must be described as such. Completion means +the selected changes preserve the existing contracts and have evidence supporting +their claimed benefit; it does not require implementing every speculative reuse +option. + +Update [internal documentation](../internal/overview.md) when implemented +mechanics change. Update public GoDoc or the CLI reference only when their owning +contracts need clarification. Keep proposal status and performance evidence in +this roadmap or a subsequent implementation plan. + +Provider caching, consumer loader redesign, removing copies required for value +ownership, and replacing Cobra are outside this target. They require their own +concrete motivation and review rather than following automatically from a +performance optimization.