Plan CommandKit performance improvements
This commit is contained in:
265
docs/roadmap/performance.md
Normal file
265
docs/roadmap/performance.md
Normal file
@@ -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.
|
||||
Reference in New Issue
Block a user