Record conversation algorithm efficiency opportunities
This commit is contained in:
@@ -11,7 +11,7 @@ The implementation retains the intended boundaries: immutable parent-linked
|
|||||||
history, ordinary preparation and execution for continuations, a separate
|
history, ordinary preparation and execution for continuations, a separate
|
||||||
sequential coordinator, and explicitly selected observer-based groups. No runtime
|
sequential coordinator, and explicitly selected observer-based groups. No runtime
|
||||||
correctness defect or inadvertently strengthened restriction was identified.
|
correctness defect or inadvertently strengthened restriction was identified.
|
||||||
The remaining finding concerns the durable public documentation in Stage 13.
|
The initial finding concerns the durable public documentation in Stage 13.
|
||||||
|
|
||||||
Validation completed with the locally installed Go 1.27.3 toolchain,
|
Validation completed with the locally installed Go 1.27.3 toolchain,
|
||||||
`GOTOOLCHAIN=local`, `GOWORK=off`, and a writable temporary build cache:
|
`GOTOOLCHAIN=local`, `GOWORK=off`, and a writable temporary build cache:
|
||||||
@@ -20,6 +20,27 @@ Validation completed with the locally installed Go 1.27.3 toolchain,
|
|||||||
- `go vet ./...` passed.
|
- `go vet ./...` passed.
|
||||||
- `git diff --check` passed.
|
- `git diff --check` passed.
|
||||||
|
|
||||||
|
### Algorithmic Follow-Up
|
||||||
|
|
||||||
|
A subsequent static audit of all 13 stages, at the same implementation baseline,
|
||||||
|
identified the two efficiency opportunities below. No additional ordinary
|
||||||
|
correctness bug or avoidable quadratic-time algorithm was found. No real-world
|
||||||
|
performance data was collected.
|
||||||
|
|
||||||
|
The planned algorithms remain fixed: suffix-only immutable capture, iterative
|
||||||
|
history flattening, sequential coordination, synchronous observer delivery, and
|
||||||
|
map-based incremental group aggregation. The findings address extra allocation
|
||||||
|
and copying around those choices, not replacements for them. Complexity statements
|
||||||
|
describe implementation costs, not new public performance guarantees.
|
||||||
|
|
||||||
|
For orientation, capture is linear in the new turn's message count; flattening is
|
||||||
|
linear in retained message count; coordinator bookkeeping is constant per turn,
|
||||||
|
excluding application callbacks and ordinary execution. Group updates are
|
||||||
|
expected constant time per event for fixed-length run identities, with linear
|
||||||
|
membership storage and linear full snapshots. An observer dispatch is linear in
|
||||||
|
its selected callbacks. These implementations do not rescan all prior turns or
|
||||||
|
group members on each incremental update.
|
||||||
|
|
||||||
## 1. Complete The Public Exchange Capture And Continuation Contract
|
## 1. Complete The Public Exchange Capture And Continuation Contract
|
||||||
|
|
||||||
**Priority:** P2. **Status:** Open. **Plan stage:** 13.
|
**Priority:** P2. **Status:** Open. **Plan stage:** 13.
|
||||||
@@ -63,3 +84,79 @@ private storage/allocation promises while documenting the behavior.
|
|||||||
prepared-execution, ownership, and repair tests and implementation. Validate the
|
prepared-execution, ownership, and repair tests and implementation. Validate the
|
||||||
affected documentation links and diff. This is documentation remediation; it
|
affected documentation links and diff. This is documentation remediation; it
|
||||||
does not require changing runtime behavior or duplicating existing tests.
|
does not require changing runtime behavior or duplicating existing tests.
|
||||||
|
|
||||||
|
## 2. Avoid Intermediate History Copies During Assembly And Inspection
|
||||||
|
|
||||||
|
**Priority:** P3. **Status:** For consideration. **Plan stages:** 2 and 5.
|
||||||
|
|
||||||
|
**Locations:** `completePreparationWithStructuredOutput` in
|
||||||
|
[the runner](../../internal/usecase/runner.go), `Exchange.Messages` in
|
||||||
|
[history storage](../../internal/domain/exchange.go), and
|
||||||
|
[`Exchange.Details`](../../exchange.go) with `fromDomainRenderedMessages` in
|
||||||
|
[facade conversion](../../convert.go).
|
||||||
|
|
||||||
|
Let H be the captured history's message count and S the newly rendered and
|
||||||
|
caller-appended message count. Continuation preparation currently:
|
||||||
|
|
||||||
|
1. allocates and copies S messages into the new suffix;
|
||||||
|
2. materializes H history messages into a detached intermediate slice; and
|
||||||
|
3. allocates the final H + S slice and copies both intermediate slices again.
|
||||||
|
|
||||||
|
Thus assembly copies 2H + 2S message entries, rather than the H + S needed to
|
||||||
|
populate the final execution-owned buffer. Each pass also clones non-nil
|
||||||
|
cache-control structs. The public `Details` path similarly materializes a
|
||||||
|
detached domain history and then traverses it again to allocate public messages
|
||||||
|
and clone cache controls for a second time. Strings share immutable content;
|
||||||
|
these are message-header and cache-struct copies, not duplicate text-byte buffers.
|
||||||
|
|
||||||
|
**Complexity:** Current and proposed assembly are both Θ(H + S) in time and
|
||||||
|
auxiliary space; details remain Θ(H). The opportunity removes an extra linear
|
||||||
|
pass and temporary allocations, not an asymptotic quadratic defect. Successive
|
||||||
|
full-history requests still entail work proportional to the sum of their history
|
||||||
|
lengths. This audit does not propose truncation or a different
|
||||||
|
conversation protocol.
|
||||||
|
|
||||||
|
**Potential refactoring:** Add an internal fill/flatten helper that writes history
|
||||||
|
and the new message parts directly into one correctly sized final domain slice.
|
||||||
|
For inspection, consider an internal indexed traversal/projection helper that
|
||||||
|
fills the public destination without first constructing a complete domain slice.
|
||||||
|
Keep the immutable parent-linked representation, separate public types, and
|
||||||
|
detachment of every exposed mutable cache-control value. Prioritize the simple
|
||||||
|
assembly change; avoid adding a general projection framework solely to eliminate
|
||||||
|
the inspection intermediate.
|
||||||
|
|
||||||
|
**Verification:** Reuse history ordering, cache-control mutation, branching,
|
||||||
|
prepared-execution, and mutating-client tests. Review the copy/allocation paths
|
||||||
|
statically. Do not add public allocation guarantees or exact-allocation tests.
|
||||||
|
|
||||||
|
## 3. Size Observer Storage For Selected Callbacks
|
||||||
|
|
||||||
|
**Priority:** P3. **Status:** For consideration. **Plan stage:** 9.
|
||||||
|
|
||||||
|
**Location:** [`Compose`](../../internal/observation/compose.go).
|
||||||
|
|
||||||
|
`Compose` allocates `selected` with capacity equal to every supplied observer
|
||||||
|
slot before filtering nil callbacks. The returned closure retains that backing
|
||||||
|
array even when only a small subset of slots survives. An all-nil input also
|
||||||
|
allocates the full temporary array before returning nil.
|
||||||
|
|
||||||
|
Let K be the number of supplied slots and A the number of non-nil callbacks,
|
||||||
|
including intentionally repeated callbacks. Construction is Θ(K) time with
|
||||||
|
Θ(K) allocated storage, and a nonempty composition retains Θ(K) storage.
|
||||||
|
Dispatch is already Θ(A) and does not need changing. Selected-function storage
|
||||||
|
can instead be Θ(A), with constant storage for A = 0, while preserving Θ(K)
|
||||||
|
construction time.
|
||||||
|
|
||||||
|
**Potential refactoring:** Count non-nil callbacks first, then allocate exactly A
|
||||||
|
slots and fill them in order. This trades a second linear scan for avoiding
|
||||||
|
storage proportional to disabled slots. Alternatively, append into an initially
|
||||||
|
nil slice for a simpler one-pass filter with amortized Θ(K) construction and
|
||||||
|
Θ(A) retained storage, accepting growth allocations for dense inputs. A
|
||||||
|
zero/single-callback fast path can also avoid an unnecessary fan-out closure.
|
||||||
|
The usual coordinator supplies only two slots, so its benefit is small; this is
|
||||||
|
a low-priority general-helper scalability opportunity, not a release blocker.
|
||||||
|
|
||||||
|
**Verification:** Preserve copied function values, nil skipping, duplicate
|
||||||
|
delivery, supplied order, and ordinary panic propagation. Existing composition
|
||||||
|
and prepared-observer ownership tests protect those contracts; no benchmark or
|
||||||
|
new performance gate is required.
|
||||||
|
|||||||
Reference in New Issue
Block a user