diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index a3fe0a7..46ef053 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -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 diff --git a/registry.go b/registry.go index 1bed211..7d12b54 100644 --- a/registry.go +++ b/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 } diff --git a/registry_test.go b/registry_test.go index 4867e5f..7b38a24 100644 --- a/registry_test.go +++ b/registry_test.go @@ -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 {