diff --git a/docs/roadmap/performance-audit.md b/docs/roadmap/performance-audit.md new file mode 100644 index 0000000..45761b5 --- /dev/null +++ b/docs/roadmap/performance-audit.md @@ -0,0 +1,303 @@ +# Performance Audit + +## Scope and assessment + +Code-based review of ConfigKit at +`48619de024d794cf41e90b171b7fcbe609dcd437`, following the completed +[startup experiment](startup.md) and its [measurement record](startup-benchmark.md). +This audit covers the public facade and the loading, acquisition, composition, +inheritance, decoding, resolution, metadata, sensitivity, and inspection paths. +It identifies optimization candidates, not correctness findings or approved +changes to public behavior. No production changes or new benchmark measurements +were made for this audit, as requested. The host is currently busy; allocation +evidence will be more useful than short timing comparisons in follow-up work. + +The core composition algorithms already use mapping indexes, and schema decoding +already uses field-name indexes. The strongest remaining scaling opportunities +are auxiliary resolution, overlap, and sensitivity operations rather than the +recursive merge itself. Several smaller changes could also remove unnecessary +provenance work and repeated pointer formatting. + +Complexity below describes inspected algorithms, not measured runtime. Let +`N` be the number of relevant tree paths, `R` the number of resolution records, +`D` the number of changed subtrees, `K` the number of overrides, `E` the number +of sensitivity dependency edges, `S` the number of classified secret paths, +and `L` the path depth/text size. Distinguish input size from output size: +expanded trees, historical events, inheritance steps, and rendered YAML can +themselves grow substantially. An output-sensitive algorithm cannot avoid +producing the output required by the existing contract. Expected map lookup +costs and path processing costs also qualify any proposed linear bound. + +## Candidates + +### 1. Avoid materializing provenance for presence-only queries + +**Priority: high; small, isolated change.** + +Owner: [metadata.go](../../internal/engine/metadata.go), `Metadata.IsExplicit` +and `Metadata.Lookup`. + +`IsExplicit` calls `Lookup`, which clones `FieldInfo` and its origin and +materializes inheritance steps before returning the boolean. This adds work +proportional to provenance size to a query that only needs effective-path +validation and the stored `Explicit` field. Inherited values can make that +extra work particularly expensive. + +Use the same effective-path/shape validation as `Lookup`, followed by a direct +read of the private field's boolean. Keep `Lookup` detached and complete. +This removes record/trace construction rather than altering tree composition. +Preserve zero metadata, root, invalid/unknown paths, contextual numeric tokens, +explicit key/index distinctions, and inherited-input presence. Existing +presence and metadata tests should cover most of the risk. + +### 2. Reduce allocation in path comparisons and sorting + +**Priority: high; shared helper changes need careful semantic coverage.** + +Owners: [resolution.go](../../internal/engine/resolution.go), `pathContains`, +`appendUniquePath`, and `orderedResolutionRecords`; +[metadata.go](../../internal/engine/metadata.go), `matchesPathShape` and `Lookup`; +[overlay.go](../../internal/engine/overlay.go), `displacedOrigins`; +[override_apply.go](../../internal/engine/override_apply.go), +`affectedOverrideOrigins`; [document_compare.go](../../internal/engine/document_compare.go) +and [diagnostic.go](../../internal/engine/diagnostic.go), sorting comparators. + +`pathContains` constructs canonical pointer strings multiple times per test. +Many nested scans below repeatedly invoke it. Sorting comparators also repeatedly +format paths, making formatting/allocation work proportional to comparisons +rather than items. `Lookup` formats the same query for multiple lookups. + +First reuse an already formatted pointer within a function or decorate sortable +items with a pointer once. Consider an allocation-free containment comparison +over token spellings as a separate experiment. Complexity remains proportional +to path length, but repeated string allocations can disappear. + +Containment currently compares canonical pointer spellings, while metadata +shape matching additionally distinguishes explicit key/index shapes. A raw +segment-struct equality replacement would change behavior: parsed tokens and +explicit numeric keys/indices can spell the same pointer. Preserve each helper's +existing equivalence relation, invalid-path handling, root behavior, slash/tilde +escaping, empty keys, and lexical sort order. Precomputation must retain original +tie-breaking and must not introduce shared mutable path storage. + +### 3. Index resolution coverage and affected subtrees + +**Priority: high; larger, output-sensitive algorithm change.** + +Owners: [resolution.go](../../internal/engine/resolution.go), `runResolution` +and `coveringResolutionRecord`; +[load_metadata.go](../../internal/engine/load_metadata.go), +`applyResolutionMetadata`, `appendResolutionDescendantHistory`, +`coveringResolutionSnapshot`, and `refreshResolutionFields`. + +Each delta scans all records to find its most-specific covering record: +approximately `O(D * R * L)`. Metadata updates then scan and parse all field, +historical, or final-tree pointers for each delta: approximately +`O(D * N * L)`, even for disjoint single-leaf changes with small output. +Final-tree indexes are also rebuilt in both descendant-history and field-refresh +processing. Newly added fields perform further coverage scans. + +Build an operation-local path-prefix index for covering records and changed +subtrees. Walk relevant field/history/final paths once, finding covering deltas +and records through that index. Reuse a final-tree index within this operation +where its ownership and lifetime permit. Target work proportional to indexed +path material plus the actual emitted history, instead of every delta times +every field. A segment trie or a suitably ordered prefix traversal is a candidate, +not a reason to introduce a general indexing framework. + +Preserve most-specific record selection and equal-depth ties, previous origins, +removed descendants, newly introduced fields, null state, and deterministic +history ordering. Ancestor operations may legitimately emit many descendant +events. Do not collapse those events to claim an unconditional `O(N)` bound. + +### 4. Replace pairwise override overlap checks + +**Priority: medium; clear quadratic case, bounded current input size.** + +Owner: [override.go](../../internal/engine/override.go), `prepareOverrides`, +`overlappingOverride`, and `validateOverridePath`. + +For `K` accepted, disjoint overrides, overlap checking scans all prior accepted +paths: `O(K² * L)`. Validation also repeatedly linearly searches ordered mapping +entries, independently of the index used later when applying overrides. + +An incremental prefix index can detect prior ancestors and descendants in +approximately total path-processing work. Retain enough insertion-order +information to report the same first conflicting accepted override. Reuse a +read-only mapping lookup index during validation if wide mappings make that +worthwhile. The existing override/source limit bounds the exposure, so this is +less urgent than broadly exercised metadata work; retain the limit unchanged. + +Preserve duplicate-versus-overlap diagnostics, accepted-path bookkeeping, +original-record order, related source locations, mapping-only traversal, and +schema-version/directive handling. Sorting input records before validation +would not automatically preserve these semantics. + +### 5. Remove temporary sensitivity indexes and redundant parsing + +**Priority: high; promising narrowly scoped allocation experiment.** + +Owner: [secret.go](../../internal/engine/secret.go), `classifySecretPaths` +and `addSecretPath`; entry point: +[load_result.go](../../internal/engine/load_result.go), `publishLoadResult`. + +Classification builds temporary pointer-to-node maps for final input, +applications, and projections just to enumerate paths or read `Sensitive`. +It indexes the final tree initially and again among projections. Paths then +pass through `ParsePath`, including repeated contributions to an existing key. +These are linear passes, but allocate avoidable intermediate maps and paths. + +Walk trees directly into the local classification accumulator, formatting a +pointer once per visit and recording sensitivity in that visitor. Avoid +reparsing already known paths when equivalent behavior can be retained. +Evaluate same-tree pass reuse locally rather than unifying metadata indexes. + +Historical paths, removed/replaced secrets, static-default and dynamic-value +tags, inheritance contributors, and dependency-only paths remain necessary. +Reusing only the effective index would be incomplete. Preserve contextual path +handling and detached predicate arguments; the current pointer-keyed accumulator +can receive the same spelling through different historical shapes. Check that +any typed-path reuse preserves classification for those cases. + +### 6. Use indexed, incremental sensitivity propagation + +**Priority: high scaling opportunity; higher semantic risk than candidate 5.** + +Owner: [secret.go](../../internal/engine/secret.go), the fixed-point loop in +`classifySecretPaths` and `secretIntersectsPath`. + +Every pass scans all dependency/inheritance edges. Each ordinary dependency +test can scan all classified secrets and compare paths. With `P` propagation +passes, the dependency portion can approach `O(P * E * S * L)`; unfavorable +edge order can require many passes down a chain. Bidirectional inheritance +links add repeated work as well. The finite classified-path set makes the +procedure terminate, but does not make these repeated scans cheap. + +Consider a worklist of newly sensitive paths with reverse dependency adjacency +and a prefix-aware intersection index. Process newly activated relationships +instead of rescanning already satisfied edges. Aim for output-sensitive work +near the cost of path indexing, edges, and actual matches; broad ancestor +dependencies can inherently intersect many paths, so a blanket `O(N + E)` +promise would be unjustified. + +Preserve ordinary dependency direction, ancestor/descendant intersection, +inheritance's bidirectional propagation, cycles, historical-only paths, and +container redaction. Compare the resulting sensitive set and safe output, not +private iteration counts. Keep this separate from the simpler traversal change. + +### 7. Reduce repeated global work in inspection + +**Priority: medium; most valuable when consumers inspect many paths.** + +Owners: [explain.go](../../internal/engine/explain.go), `Inspection.Explain`; +[history.go](../../internal/engine/history.go), `coalescedResolutionSnapshots`; +[inheritance_history.go](../../internal/engine/inheritance_history.go), +`historyWithInheritance`; [document_walk.go](../../internal/engine/document_walk.go), +`documentAtPath`; [snapshot.go](../../internal/engine/snapshot.go). + +Each explanation clones whole baseline and decoded-input trees, replays all +applications, and coalesces the same resolution snapshots. Coalescing scans +previous groups for each snapshot and is `O(R² * L)` for independent records. +Each historical query also scans every inheritance operation. Document lookup +linearly scans each mapping on its path; querying every key in a wide mapping +can therefore add quadratic lookup work. These costs compound across queries, +even if each query asks about a small unrelated leaf. + +First consider preparing immutable coalesced replay descriptors once when an +inspection is constructed, and indexing relevant inheritance operations and +frozen-tree lookups. Coalescing can use an ancestor index rather than a group +scan. A later, separate experiment could replay only the queried branch and its +necessary dependencies instead of cloning whole trees. + +Preserve the first covering coalesced group, ordered reason/dependency unions, +normalization precedence, chronology, intermediate absence/replacement, typed +reconciliation, destination-local inheritance, and historical redaction. +Simply skipping applications marked irrelevant is not a proven safe algorithm: +their contributions can affect later inheritance/replay. Avoid eagerly retaining +every rendered explanation or tree-per-field; that exchanges query CPU for +potentially substantial startup and retained-memory costs. No new public batch +API or cache lifecycle is needed to investigate these internal opportunities. + +### 8. Replace growing-list provenance deduplication where worthwhile + +**Priority: medium; important for deep or contribution-heavy inheritance.** + +Owners: [inheritance_origin.go](../../internal/engine/inheritance_origin.go), +`projectInheritanceSteps`; [load_metadata.go](../../internal/engine/load_metadata.go), +`originContainedBy` and `originRepresentedByChange`; +[resolution.go](../../internal/engine/resolution.go), `appendUniquePath` +and `appendUniqueString`. + +Step materialization tests each proposed step against all steps already emitted, +reformatting multiple paths in each comparison. A chain of `T` distinct steps +can require `O(T²)` comparisons, before path/location costs. Origin containment +has pairwise location and contributor scans; repeated insertion into reason +and dependency lists also scans the accumulated list. + +Use local membership indexes alongside the ordered output slices when their +cardinality justifies the additional machinery. Keys must represent the actual +equality relation, including locations and trace structure where applicable; +hashes alone are insufficient without collision-safe equality. Preserve first +occurrence order, duplicate behavior, and detached returned records. Keep tiny +lists straightforward; do not impose global interners or flatten the existing +compact shared inheritance traces into eager per-value step lists. + +### 9. Reduce path construction in inheritance detection + +**Priority: medium; small common-path allocation candidate.** + +Owner: [inheritance_load.go](../../internal/engine/inheritance_load.go), +`containsInheritanceDirective`. + +The presence check uses a full `walkDocument` traversal that creates paths for +all descendants, and keeps traversing after finding `extends`. It only needs +to detect a mapping key. A direct boolean tree visitor could inspect keys and +sequence children without constructing paths and return immediately on a match. +The no-inheritance case still requires a linear scan, but with less allocation; +early matches can avoid the remainder of this detection pass. + +Detection remains separate from validation/expansion. It must still find keys +inside sequences and nested mappings; invalid inheritance syntax must reach +the existing diagnostics rather than being skipped. Do not infer permission +to change scope rules, traversal ordering during expansion, or resource budgets. + +## Reviewed areas without a priority algorithm replacement + +| Area | Assessment | +| --- | --- | +| Strict composition and recursive precedence (`compose.go`, `recursive_merge.go`, `mapping_index.go`, `load_request.go`) | Ordered entries already have lazily built per-node lookup maps, avoiding pairwise wide-map merge scans. Acquisition and tier assembly retain source/history evidence for distinct purposes. No wholesale merge rewrite recommended. | +| YAML parsing/writing (`internal/yaml`) and mapping-value construction (`value.go`) | Translation uses a duplicate-key map and presized containers. Adapter trees add linear allocation but protect the parser boundary. No parser replacement or custom serializer justified by this review. | +| Sources, imports, globs, filesystems, and resource contexts | Import identity/cycle lookup uses maps; directory enumeration is batched when supported. Glob traversal cost follows visited entries and matches, with deterministic sorting. Repeated directory/canonicalization work may merit future workload evidence, but caching could change acquisition observations and accounting. Preserve handle closure and passive returned contexts. | +| Schema, typed decode, projection, and typed cloning | Schema descriptors cache types within a load and index struct fields. Reflection and multiple linear projections/clones have costs, but provide defaults snapshots, codec validation, ownership, and callback auditing. Avoid removing these boundaries speculatively. A global schema cache is not the first recommendation. | +| Structural inheritance expansion | Siblings are indexed by name and resolved using dependency state rather than repeatedly searching for parents. Expanded output, destination-local processing, provenance, and retained-history accounting explain substantial work. Deep nested scopes may revisit subtrees, but this audit does not establish a safe general replacement for those passes. | +| Secrets resolution and explicit directory bootstrap | Environment lookups are already deduplicated within a load. Directory work is bounded, sorted, and staged before application. The classification candidates above do not call for changing bootstrap preflight or process-environment behavior. | +| Public facade and detached records | Root-owned wrappers and explicit conversions preserve the public/private boundary. Some records are copied in the engine and again during conversion; eliminating a copy needs a clear internal ownership handoff, not public aliases or borrowed mutable records. `Result.Metadata()` also clones the complete metadata before queries; consumers repeatedly obtaining it pay repeated full-copy costs. Treat redesign of that private handoff as a separate, lower-priority retained-memory/ownership experiment. | +| Effective YAML, diagnostics, examples, and release/development tooling | Deterministic rendering and diagnostics require ordering and output-sized work; path comparator allocation is covered above. Safe snapshots and YAML adaptation allocate separate trees. Maintained consumer examples and check tooling are not production library hot paths; no gate or tooling changes proposed. ReleaseKit internals are outside this repository audit. | + +## Recommended follow-up + +Start with candidate 1 or 9 as a small, reviewable optimization, then candidate +2's local formatting reuse and candidate 5's direct classification traversal. +These avoid work without demanding new architectural machinery. Next address +candidate 3 for many recorded changes and candidate 6 for large dependency +graphs. Candidates 4, 7, and 8 are useful follow-ups for consumers with many +overrides, inspection queries, or inherited contributions. + +This is a prioritized backlog, not a staged implementation plan or a requirement +to implement every candidate. Before each production change, confirm the proposed +algorithm against the existing [configuration contract](../config.md), relevant +tests, and [architecture policy](../policy/architecture.md). Preserve complete +load results, effective versus historical path shapes, provenance, ownership, +presence, secret isolation, and synchronous completion. Do not add caps, reject +previously valid inputs, change composition rules, or create performance promises +to simplify an optimization. + +For a selected change, add only missing behavioral regressions, run the relevant +package tests and everyday [development check](../development.md), and use race +checks where ownership/shared state changes warrant them. Follow-up measurements +can reuse existing startup, composition, underlay, inheritance, and history +benchmarks, with targeted fixtures for record count, disjoint overrides, +dependency chains, and repeated queries where coverage is missing. Prefer +allocations and bytes on the busy host; defer CPU improvement claims until +timing is representative. Benchmarks help decide whether the implementation's +complexity is worthwhile, but are not new release gates or test thresholds.