312 lines
20 KiB
Markdown
312 lines
20 KiB
Markdown
# 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.
|
|
|
|
## Target state and selected scope
|
|
|
|
The implementation target is the same public configuration model with less
|
|
avoidable auxiliary work: presence-only queries do not materialize provenance;
|
|
path comparisons and sorts avoid repeated formatting; resolution coverage,
|
|
override overlaps, and sensitivity propagation use local path indexes;
|
|
classification collects paths directly; and frozen inspection reuses prepared
|
|
resolution/inheritance lookup data. Inheritance detection does not construct
|
|
semantic paths merely to detect a keyword. Ordered provenance collections use
|
|
membership indexes where their equality rules are straightforward.
|
|
|
|
The [implementation plan](implementation.md) selects bounded revisions from
|
|
all nine candidates below. Whole-tree explanation replay remains in place;
|
|
branch-local replay, changing the public metadata-copy handoff, global schema
|
|
caching, and indexing complex contributor trace equality remain later
|
|
experiments. These have larger ownership, memory, or equivalence risks than the
|
|
selected changes. No new consumer-facing complexity bound or performance
|
|
threshold is part of the target. The candidate descriptions retain the broader
|
|
audit context; the plan owns the selected implementation details and sequence.
|
|
|
|
## 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.
|
|
|
|
Reusing an already formatted pointer within a function and decorating sortable
|
|
items with a pointer once avoid repeated formatting. Allocation-free containment
|
|
comparison over token spellings is also in the selected scope. Cost 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.
|
|
|
|
The selected target prepares immutable coalesced replay descriptors once when an
|
|
inspection is constructed and indexes relevant inheritance history operations
|
|
and frozen-tree lookups. Coalescing uses ancestor lookup rather than a group
|
|
scan. Replaying only the queried branch and its necessary dependencies remains
|
|
a deferred experiment; whole-tree replay remains in the selected target.
|
|
|
|
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. |
|
|
|
|
## Acceptance and boundaries
|
|
|
|
The selected revisions preserve the existing [configuration contract](../config.md)
|
|
and [architecture policy](../policy/architecture.md): complete load results,
|
|
effective versus historical path shapes, provenance, ownership, presence,
|
|
secret isolation, and synchronous completion remain unchanged. New caps,
|
|
input restrictions, composition rules, or performance promises are not part of
|
|
this work. The plan owns implementation sequencing, validation, and evidence.
|
|
|
|
Success is removal of the identified redundant work and broad pairwise scans
|
|
without disproportionate machinery or semantic regressions. Output-sized work,
|
|
deterministic ordering, and required ownership copies remain legitimate costs.
|
|
Benchmarks may inform the tradeoff, especially allocations on the busy host,
|
|
but are not new release gates or test thresholds. Unmeasured improvements
|
|
remain algorithmic expectations rather than quantified runtime claims.
|