From 3ad355927c1b81c6ec61b1c6fce521600a18933a Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 9 Oct 2026 01:34:24 +0000 Subject: [PATCH] Cache immutable command bridge metadata --- app.go | 7 ++- docs/roadmap/implementation.md | 36 ++++++++++++++-- parser.go | 7 +++ parser_test.go | 78 ++++++++++++++++++++++++++++++++++ registry.go | 1 + 5 files changed, 125 insertions(+), 4 deletions(-) diff --git a/app.go b/app.go index 135b246..d58379d 100644 --- a/app.go +++ b/app.go @@ -1,11 +1,16 @@ package commandkit -import "fmt" +import ( + "fmt" + + internalcobra "gitea.maximumdirect.net/eric/CommandKit/internal/cobra" +) // App owns one command registry and its application identity. type App struct { identity Identity commands map[string]*commandNode + commandTree *internalcobra.Command configRegistered bool } diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 46ef053..7fe234c 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Performance implementation plan -Status: implementation in progress; Stages 1–6 are complete, Stages 7–11 have not started. +Status: implementation in progress; Stages 1–7 are complete, Stages 8–11 have not started. Date: 2026-10-09. @@ -549,7 +549,7 @@ issues remain open. ## Stage 7: cache immutable bridge metadata per app -**Status:** not started. **Prerequisite:** Stage 6. +**Status:** complete. **Prerequisite:** Stage 6. **Files:** [app.go](../../app.go), [parser.go](../../parser.go), registry invalidation, root parser/registry tests, dispatch benchmarks, and evidence. @@ -581,7 +581,37 @@ invocation still creates fresh backend values at this stage. Use **Completion:** unchanged registrations reuse translated metadata; successful registration is visible to later parsing and help. -**Evidence:** pending. +**Evidence:** `GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none +GOSUMDB=off go test -mod=readonly -count=1 .` passed. The new behavioral test +builds help metadata, registers a command and both standard command families, +then confirms later help and parsing see them without invoking provider +callbacks. It also checks visibility after failed and empty registration. +Source review confirms the tree is built lazily, contains only neutral +declarations/flag/operand metadata, and is cleared only after successful +nonempty registration. Root and child ordering is performed by the uncached +builder once per metadata generation. + +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 `5816dd3` 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 '^BenchmarkPerformanceDispatch$' -benchmem -benchtime=200ms -count=5 .`. +Raw output is in `/tmp/commandkit-performance-dispatch-after-stage7.txt`. + +Repeated small success changed from 31,066–41,041 ns/op, 33,209–33,210 B/op, +and 287 allocations to 26,008–31,223 ns/op, 29,408–29,409 B/op, and 267 +allocations. Wide success changed from 218,073–253,010 ns/op, 269,063–269,070 +B/op, and 1,898 allocations to 182,431–194,243 ns/op, 247,626–247,635 B/op, +and 1,756 allocations. Deep success changed from 119,958–139,625 ns/op, +139,787–139,791 B/op, and 1,074 allocations to 103,208–124,879 ns/op, +124,240–124,242 B/op, and 879 allocations. Help and usage paths also reduced +allocations and bytes. Cold construction plus first execution remained similar +for the small app (38,152–50,235 ns/op, 38,275–38,276 B/op, 339 allocations +baseline; 39,158–47,365 ns/op, 38,243 B/op, 338 allocations now). The wide +cold case changed from 251,945–315,276 ns/op, 291,822–291,827 B/op, and 2,184 +allocations to 271,403–321,746 ns/op, 305,046–305,051 B/op, and 2,194 +allocations. Its timing range overlaps baseline; the extra cold storage is the +retained neutral tree. The measured tradeoff is lower repeated-execution cost +with one tree allocation retained per app metadata generation. ## Stage 8: introduce an invocation-owned adapter session diff --git a/parser.go b/parser.go index 8a2d24a..f411e6e 100644 --- a/parser.go +++ b/parser.go @@ -17,6 +17,13 @@ func (a *App) bridgeTree() *internalcobra.Command { if a == nil { return nil } + if a.commandTree == nil { + a.commandTree = a.buildBridgeTree() + } + return a.commandTree +} + +func (a *App) buildBridgeTree() *internalcobra.Command { root := &internalcobra.Command{Name: a.identity.Name} names := make([]string, 0, len(a.commands)) for name, node := range a.commands { diff --git a/parser_test.go b/parser_test.go index 8f49598..c0cb725 100644 --- a/parser_test.go +++ b/parser_test.go @@ -1,11 +1,15 @@ package commandkit import ( + "bytes" "context" + "io" "reflect" + "strings" "testing" internalcobra "gitea.maximumdirect.net/eric/CommandKit/internal/cobra" + "gitea.maximumdirect.net/eric/promptkit" ) func TestAppParseUsesRegisteredBindingsWithTheAdapter(t *testing.T) { @@ -48,3 +52,77 @@ func TestAppParseUsesRegisteredBindingsWithTheAdapter(t *testing.T) { t.Fatal("accepted a child flag before the complete command path") } } + +func TestBridgeTreeRefreshesAfterSuccessfulRegistration(t *testing.T) { + app, err := New(Identity{Name: "example"}) + if err != nil { + t.Fatal(err) + } + var output bytes.Buffer + streams := Streams{Out: &output, Err: io.Discard} + renderRootHelp := func() string { + t.Helper() + output.Reset() + outcome := app.Execute(context.Background(), []string{"--help"}, streams) + if outcome.Kind != OutcomeSuccess { + t.Fatalf("root help outcome = %#v", outcome) + } + return output.String() + } + initial := renderRootHelp() + if strings.Contains(initial, "custom") || strings.Contains(initial, "config") || strings.Contains(initial, "promptkit") { + t.Fatalf("initial help unexpectedly lists future registrations: %q", initial) + } + + custom, err := NewCommand(CommandSpec[struct{}]{Name: "custom", Run: func(Invocation, struct{}) Outcome { return Success() }}) + if err != nil { + t.Fatal(err) + } + if err := app.Register(custom); err != nil { + t.Fatal(err) + } + providerCalls := 0 + if err := RegisterConfig(app, ConfigSuite[struct{}]{Scopes: []ConfigScope[struct{}]{{ + Name: "app", + Operations: []ConfigOperation{ConfigShow}, + Acquire: func(context.Context, ConfigRequest, struct{}) (Acquired[ConfigView], error) { + providerCalls++ + return Acquired[ConfigView]{}, nil + }, + }}}); err != nil { + t.Fatal(err) + } + if err := RegisterPromptKit(app, PromptKitSuite[struct{}]{InspectPrompt: func(context.Context, Selection, struct{}, string, string) (Acquired[*promptkit.PromptInspection], error) { + providerCalls++ + return Acquired[*promptkit.PromptInspection]{}, nil + }}); err != nil { + t.Fatal(err) + } + updated := renderRootHelp() + for _, name := range []string{"custom", "config", "promptkit"} { + if !strings.Contains(updated, name) { + t.Errorf("updated help omits %q: %q", name, updated) + } + } + if providerCalls != 0 { + t.Fatalf("help invoked inspection or configuration callbacks %d times", providerCalls) + } + + late, err := NewCommand(CommandSpec[struct{}]{Name: "late"}) + if err != nil { + t.Fatal(err) + } + if err := app.Register(late, custom); err == nil { + t.Fatal("Register accepted a batch with an existing-name collision") + } + if err := app.Register(); err != nil { + t.Fatalf("empty registration failed: %v", err) + } + final := renderRootHelp() + if !strings.Contains(final, "custom") || !strings.Contains(final, "config") || !strings.Contains(final, "promptkit") || strings.Contains(final, "late") { + t.Fatalf("help after failed and empty registration = %q", final) + } + if outcome := app.Execute(context.Background(), []string{"custom"}, streams); outcome.Kind != OutcomeSuccess { + t.Fatalf("new command did not parse after registration: %#v", outcome) + } +} diff --git a/registry.go b/registry.go index 7d12b54..3dd5a98 100644 --- a/registry.go +++ b/registry.go @@ -31,5 +31,6 @@ func (a *App) Register(commands ...Command) error { for name, node := range pending { a.commands[name] = node } + a.commandTree = nil return nil }