Record algorithmic efficiency audit findings
This commit is contained in:
147
docs/roadmap/audit/02-efficiency.md
Normal file
147
docs/roadmap/audit/02-efficiency.md
Normal file
@@ -0,0 +1,147 @@
|
||||
# Algorithmic Efficiency Audit
|
||||
|
||||
Reviewed 2026-11-04 against commit `0d27252`.
|
||||
|
||||
## Scope And Method
|
||||
|
||||
Reviewed PromptKit-owned configuration assembly, provider-value conversion and
|
||||
copying, rendering and requirement analysis, inventory selection, prepared-run
|
||||
ownership, validation orchestration, repair formatting, retry coordination,
|
||||
observations, and capacity management. Findings below follow from implementation
|
||||
and operation counts; no benchmarks or live-provider calls were needed.
|
||||
|
||||
The public facade delegates to internal components. ConfigKit supplies the final
|
||||
configuration; PromptKit constructs immutable runtime indexes and captured
|
||||
templates. Execution uses operation-local validation plans, shared backend
|
||||
capacity, and a generation coordinator. Inspection reads the captured snapshot.
|
||||
ConfigKit and ReleaseKit internals are outside this repository audit.
|
||||
|
||||
### Deliberate Choices Preserved
|
||||
|
||||
Checked current internal documentation and relevant retired plans in Git history,
|
||||
including the performance plan at `8a5c0de`, configuration adoption at `5f7b194`,
|
||||
and collections at `89af2a1`. This audit does not reopen:
|
||||
|
||||
- lexical inventory ordering, recursive provider-map overlay semantics, or
|
||||
conservative requirement-discovery semantics;
|
||||
- captured parsing, per-render artifact-text reuse, operation-local schema
|
||||
compilation, frozen retry payloads, and linked-list FIFO permits with
|
||||
constant-time cancellation removal;
|
||||
- the existing JSON depth/work bounds, output/diagnostic limits, or copies
|
||||
deliberately retained to protect public and injected-collaborator ownership;
|
||||
- synchronous filesystem/template/schema operations or the selected retry and
|
||||
repair budgets.
|
||||
|
||||
The opportunities below concern implementation work that those choices do not
|
||||
require. Complexity statements use expected constant-time Go map operations and
|
||||
treat identifier comparison/hashing cost separately. No new public performance
|
||||
guarantee, cache, caller limit, or release gate is proposed.
|
||||
|
||||
## 1. Condition Evidence Unions Can Require Quadratic Work
|
||||
|
||||
**Priority:** P2. Requirement-inspection path; most relevant to deeply nested
|
||||
conditions rather than ordinary small templates.
|
||||
|
||||
**Code:** [evaluateCommand](../../../internal/prompt/requirements.go:142) and
|
||||
[mergeConditionEvaluations](../../../internal/prompt/requirements.go:318).
|
||||
|
||||
Each comparison recursively evaluates its operands, then allocates fresh
|
||||
`missingVariables` and `contentInputs` maps and copies both operands' accumulated
|
||||
evidence. A left-heavy expression with distinct missing caller keys, such as
|
||||
successively nesting `eq (eq .v0 .v1) .v2`, carries an increasingly large missing
|
||||
key set through each ancestor. Template parsing accepts these expressions;
|
||||
inspection can encounter them even when their eventual execution would fail or
|
||||
the inspection result is partial.
|
||||
|
||||
For an expression of size n, successive unions copy approximately
|
||||
2 + 3 + ... + n entries: **Theta(n²) total set work and cumulative allocation**.
|
||||
The final unique evidence has only O(n) entries. This is separate from the
|
||||
previously fixed repeated syntax-subtree scan: visiting each syntax node once
|
||||
does not eliminate repeated copies of the evidence produced by those nodes.
|
||||
|
||||
**Opportunity:** Transfer ownership of temporary sets and merge the smaller set
|
||||
into the larger one, rather than copying both at every comparison. For this
|
||||
expression family that is O(n); small-to-large merging gives O(n log n) aggregate
|
||||
set-insertion work across general expression trees. Another option is an
|
||||
operation-local evidence accumulator that records each discovery directly,
|
||||
while keeping only condition truth/limitation state on recursive return values.
|
||||
Choose the simpler approach after checking temporary-set ownership. Preserve
|
||||
current grammar, classifications, deduplication, sorted output, cancellation,
|
||||
and partial-result behavior; interpreting more expressions is not necessary.
|
||||
|
||||
## 2. Recursive Provider Overlay Repeatedly Clones Descendant Subtrees
|
||||
|
||||
**Priority:** P3. Construction-time improvement with an existing depth bound.
|
||||
|
||||
**Code:** [overlayProviderMap](../../../internal/configuration/targets.go:131).
|
||||
|
||||
The helper deep-copies the entire lower map before considering overrides. When
|
||||
both values at a key are maps, it recursively invokes itself on a descendant
|
||||
that was already copied, and copies that whole descendant again. The first copy
|
||||
is then replaced and discarded. All descendants on an overlapping path can be
|
||||
copied once per ancestor.
|
||||
|
||||
Let N be the lower tree's node count and D its overlapping map depth. Node-copy
|
||||
work can reach **O(ND)**. For a narrow chain it is the triangular sum
|
||||
D + (D-1) + ... + 1. A wide K-entry leaf below D overlapping ancestors is cloned
|
||||
roughly D times instead of once. `jsonvalue.CopyMap` also sorts map keys, so the
|
||||
same wide leaf's sorting work is repeated at those ancestors.
|
||||
|
||||
The existing maximum container depth of 100 is important: this is **not an
|
||||
unbounded quadratic-in-N claim under today's fixed limits**. It is an avoidable
|
||||
depth-dependent multiplier in traversal and cumulative allocations within those
|
||||
limits, multiplied again when multiple profiles overlay the same backend. That
|
||||
bound and the recursive-map/value-replacement semantics remain selected choices.
|
||||
|
||||
**Opportunity:** Clone the lower tree once, then overlay into the owned clone
|
||||
without re-cloning existing descendant maps. Copy incoming replacement values
|
||||
where needed to preserve isolation from configuration inputs. Alternatively,
|
||||
construct the merged output in one traversal. Either removes the repeated
|
||||
ancestor copies: expected O(N + M) merge traversal for lower/upper trees of sizes
|
||||
N and M, apart from the retained validation/sorting costs. Preserve null and
|
||||
array replacement, empty-map behavior, input ownership, and existing JSON
|
||||
validation/bounds; do not replace ConfigKit's configuration machinery.
|
||||
|
||||
## 3. Filtered Listings Copy And Scan The Complete Inventory
|
||||
|
||||
**Priority:** P3. Scalability opportunity for engines with many namespaces.
|
||||
|
||||
**Code:** [filterIdentityList](../../../internal/engine/engine.go:306),
|
||||
[ListPrompts](../../../internal/engine/engine.go:270), and the
|
||||
[configuration inventory methods](../../../internal/configuration/listing.go:14).
|
||||
|
||||
A named-namespace request first obtains a detached copy of all N identities and
|
||||
then scans it to build its k matching results. Even an absent namespace costs
|
||||
**Theta(N) time and O(N) temporary storage**, rather than work proportional to
|
||||
the selected inventory. Listing C namespaces separately costs Theta(CN); when
|
||||
each has a bounded number of entries, that becomes quadratic in total inventory
|
||||
size despite only O(N) total returned entries.
|
||||
|
||||
The collection implementation plan at `89af2a1` expressly allowed namespace
|
||||
buckets or sorted ranges to avoid repeated full scans; it did not require the
|
||||
present whole-inventory-copy algorithm. The lexical ordering and caller-owned
|
||||
result contracts do not prevent indexed selection.
|
||||
|
||||
**Opportunity:** Capture per-kind namespace buckets or ranges alongside existing
|
||||
construction-time inventories, then copy only the selected entries. Buckets
|
||||
allow expected O(1 + k) selection/copy; binary search over sorted full IDs can
|
||||
support named-prefix ranges in O(log N + k). Keep unqualified and all-ID modes
|
||||
correct without assuming unqualified entries form one contiguous lexical range.
|
||||
Resolve filters before requesting the inventory, and retain exact namespace
|
||||
matching, full IDs, ordering, context behavior, and independent returned slices.
|
||||
An all-ID request must still spend O(N) to return N owned entries; that is not a
|
||||
defect. No public pagination API or mutable cache is needed.
|
||||
|
||||
## Review Outcome
|
||||
|
||||
These three opportunities are supported by direct operation-count analysis;
|
||||
they are not measured latency claims. The largest asymptotic issue is growing
|
||||
condition-evidence copying. The overlay finding is explicitly bounded, and the
|
||||
listing finding matters as independent namespace count grows.
|
||||
|
||||
No additional ordinary correctness defect was established in this pass. The
|
||||
findings in [the correctness audit](01-correctness.md) remain separate and were
|
||||
not duplicated here. No production code, tests, dependencies, or established
|
||||
behavior were changed, so the successful race-suite/vet evidence recorded there
|
||||
still applies to the unchanged implementation; those checks were not rerun just
|
||||
for this documentation change.
|
||||
Reference in New Issue
Block a user