Record performance audit and optimization candidates
This commit is contained in:
303
docs/roadmap/performance-audit.md
Normal file
303
docs/roadmap/performance-audit.md
Normal file
@@ -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.
|
||||
Reference in New Issue
Block a user