Register validated commands without registry copies
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
# Performance implementation plan
|
||||
|
||||
Status: implementation in progress; Stages 1–5 are complete, Stages 6–11 have not started.
|
||||
Status: implementation in progress; Stages 1–6 are complete, Stages 7–11 have not started.
|
||||
|
||||
Date: 2026-10-09.
|
||||
|
||||
@@ -494,7 +494,7 @@ and reusing the opening analysis for framing and aggregate directive handling.
|
||||
|
||||
## Stage 6: install validated registrations without copying the registry
|
||||
|
||||
**Status:** not started. **Prerequisite:** Stage 5.
|
||||
**Status:** complete. **Prerequisite:** Stage 5.
|
||||
|
||||
**Files:** [registry.go](../../registry.go),
|
||||
[registry_test.go](../../registry_test.go), registration benchmarks, and evidence.
|
||||
@@ -519,7 +519,33 @@ and reusing the opening analysis for framing and aggregate directive handling.
|
||||
**Completion:** registration copies only pending declarations and preserves
|
||||
all-or-nothing validation without rebuilding the existing registry map.
|
||||
|
||||
**Evidence:** pending.
|
||||
**Evidence:** `GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none
|
||||
GOSUMDB=off go test -mod=readonly -count=1 .` passed. Existing tests cover
|
||||
atomic rollback, reserved paths, callback lifecycle, and ownership. Added a
|
||||
duplicate-within-batch case and confirmed that it leaves the registry unchanged.
|
||||
|
||||
Measurements used Go 1.26.8, linux/amd64, Linux 6.17.0-41-generic, Intel Core
|
||||
i5-7260U CPU @ 2.20 GHz, based on source revision `3f8841f` with this stage's
|
||||
changes in the working tree. The five-sample command passed:
|
||||
`GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none GOSUMDB=off go test -mod=readonly -run '^$' -bench '^BenchmarkPerformanceRegistration$' -benchmem -benchtime=200ms -count=5 .`.
|
||||
Raw output is in `/tmp/commandkit-performance-registration-after-stage6.txt`.
|
||||
|
||||
At 8 declarations, individual registration measured 4,873–5,401 ns/op,
|
||||
3,152 B/op, and 45 allocations; batch registration measured 4,404–10,155 ns/op,
|
||||
3,152 B/op, and 45 allocations (the timing range includes noisy outliers).
|
||||
At 128 declarations, individual registration measured 55,274–64,107 ns/op,
|
||||
29,399–29,400 B/op, and 293 allocations, compared with the Stage 1 baseline
|
||||
414,618–492,261 ns/op, 405,011–405,018 B/op, and 784 allocations. Batch
|
||||
registration measured 47,867–53,858 ns/op, 35,969–35,970 B/op, and 296
|
||||
allocations, compared with baseline 33,985–36,207 ns/op, 23,013–23,014 B/op,
|
||||
and 288 allocations. A slice-only pending check reduced the batch allocation
|
||||
cost to about 30.5 KB but was slower (about 61–66 μs) because it scans pending
|
||||
names quadratically; the map-based pending index is retained for linear-time
|
||||
duplicate validation. The batch regression remains measurable and reflects
|
||||
growing the existing map plus maintaining pending entries; restoring a full
|
||||
registry rebuild would contradict this stage's selected change. Individual
|
||||
registration has a large allocation and latency reduction. No correctness
|
||||
issues remain open.
|
||||
|
||||
## Stage 7: cache immutable bridge metadata per app
|
||||
|
||||
|
||||
22
registry.go
22
registry.go
@@ -9,19 +9,27 @@ func (a *App) Register(commands ...Command) error {
|
||||
return fmt.Errorf("cannot register commands on a nil app")
|
||||
}
|
||||
|
||||
updated := make(map[string]*commandNode, len(a.commands)+len(commands))
|
||||
for name, node := range a.commands {
|
||||
updated[name] = node
|
||||
}
|
||||
pending := make(map[string]*commandNode, len(commands))
|
||||
for _, command := range commands {
|
||||
if err := validateCommandDeclaration(command); err != nil {
|
||||
return err
|
||||
}
|
||||
if _, exists := updated[command.name]; exists {
|
||||
if _, exists := a.commands[command.name]; exists {
|
||||
return fmt.Errorf("command path %q is already registered", command.name)
|
||||
}
|
||||
updated[command.name] = &commandNode{command: cloneCommand(command)}
|
||||
if _, exists := pending[command.name]; exists {
|
||||
return fmt.Errorf("command path %q is already registered", command.name)
|
||||
}
|
||||
pending[command.name] = &commandNode{command: cloneCommand(command)}
|
||||
}
|
||||
if len(pending) == 0 {
|
||||
return nil
|
||||
}
|
||||
if a.commands == nil {
|
||||
a.commands = make(map[string]*commandNode, len(pending))
|
||||
}
|
||||
for name, node := range pending {
|
||||
a.commands[name] = node
|
||||
}
|
||||
a.commands = updated
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -159,6 +159,12 @@ func TestRegisterBatchIsAtomicAndRejectsReservedPaths(t *testing.T) {
|
||||
if _, exists := app.commands["added"]; exists {
|
||||
t.Fatal("failed batch attached a partial registration")
|
||||
}
|
||||
if err := app.Register(added, added); err == nil {
|
||||
t.Fatal("Register accepted a duplicate within a batch")
|
||||
}
|
||||
if _, exists := app.commands["added"]; exists {
|
||||
t.Fatal("failed duplicate batch attached a registration")
|
||||
}
|
||||
for _, name := range []string{"help", "version"} {
|
||||
reserved, err := NewCommand(CommandSpec[struct{}]{Name: name})
|
||||
if err != nil {
|
||||
|
||||
Reference in New Issue
Block a user