From a7960e5e94dacb2c754015e427ba21b563feb111 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 9 Oct 2026 01:42:03 +0000 Subject: [PATCH] Reuse adapter sessions during execution --- builtins.go | 26 +++++-- docs/roadmap/implementation.md | 60 ++++++++++++++++- execute.go | 119 ++++++++++++++++++++++++++++----- execute_external_test.go | 103 ++++++++++++++++++++++++++-- internal/cobra/parse.go | 10 +++ parser_test.go | 42 ++++++++++++ 6 files changed, 328 insertions(+), 32 deletions(-) diff --git a/builtins.go b/builtins.go index c68e679..13f3b62 100644 --- a/builtins.go +++ b/builtins.go @@ -38,11 +38,7 @@ func (a *App) installBuiltins() error { func(value string) (string, error) { return value, nil }, func(input *helpInput, value Supplied[[]string]) { input.Targets = value })}, Run: func(invocation Invocation, input helpInput) Outcome { - target, err := a.helpCommandTarget(input.Targets.Value) - if err != nil { - return Usage(err) - } - return a.renderHelpOutcome(target, invocation.Streams) + return a.renderHelpTargets(input.Targets.Value, invocation.Streams) }, }) if err != nil { @@ -92,12 +88,30 @@ func parseVersionFormat(value string) (string, error) { } func (a *App) renderHelpOutcome(path []string, streams Streams) Outcome { + session := internalcobra.Prepare(a.bridgeTree(), internalcobra.ParseOptions{}) + return a.renderHelpOutcomeWithSession(path, streams, session) +} + +func (a *App) renderHelpTargets(targets []string, streams Streams) Outcome { + session := internalcobra.Prepare(a.bridgeTree(), internalcobra.ParseOptions{}) + return a.renderHelpTargetsWithSession(targets, streams, session) +} + +func (a *App) renderHelpTargetsWithSession(targets []string, streams Streams, session *internalcobra.Session) Outcome { + target, err := a.helpCommandTarget(targets) + if err != nil { + return Usage(err) + } + return a.renderHelpOutcomeWithSession(target, streams, session) +} + +func (a *App) renderHelpOutcomeWithSession(path []string, streams Streams, session *internalcobra.Session) Outcome { if len(path) > 0 { if _, ok := a.commandAt(path); !ok { return Usage(fmt.Errorf("unknown help path %q", strings.Join(path, " "))) } } - content, failure := internalcobra.RenderHelp(a.bridgeTree(), path) + content, failure := session.RenderHelp(path) if failure != nil { return Failure(errors.New(failure.Message)) } diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index dc17b82..24bec89 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -673,7 +673,7 @@ measures repeated parse plus help on the same fresh-per-session backend state. ## Stage 9: reuse the adapter session throughout root execution -**Status:** not started. **Prerequisite:** Stage 8. +**Status:** complete. **Prerequisite:** Stage 8. **Files:** [execute.go](../../execute.go), [builtins.go](../../builtins.go), private parser coordination, [execute_external_test.go](../../execute_external_test.go), @@ -719,7 +719,63 @@ to confirm that signatures and backend isolation are unchanged. **Completion:** every normal `Execute` path uses the selected OD-1 lifecycle and all existing public handler and presentation semantics remain intact. -**Evidence:** pending. +**Evidence:** `GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none +GOSUMDB=off go test -mod=readonly -count=1 ./internal/cobra .` passed. Existing +help, parser, outcome, and cancellation coverage continues to pass. Added checks +for a successful flag-bearing invocation followed by local help with an empty +value, explicit false and omitted values, distinct contexts/streams across +executions, handler-returned usage after successful registration, and help plus +usage against metadata registered after a session was prepared. The built-in +help handler and top-level dispatcher share target resolution/rendering. Source +review confirms `Execute` prepares one session from the cached neutral tree; +presentation reuses it when the tree pointer is unchanged and prepares against +new metadata only after successful registration. Parsing, input factories, +checks, and handlers are not rerun for presentation. Public signatures and the +Cobra facade boundary are 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 `a852301` 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-stage9.txt`; +focused wide and cold repeats are in +`/tmp/commandkit-performance-dispatch-stage9-repeat.txt` and +`/tmp/commandkit-performance-dispatch-stage9-cold-repeat.txt`. + +Compared with the Stage 1 baseline, small repeated success measured +38,671–44,179 ns/op, 29,576–29,578 B/op, and 269 allocations (baseline: +31,066–41,041 ns/op, 33,209–33,210 B/op, 287 allocations). The timing ranges +overlap; bytes fell by about 3.6 KB and allocations by 18. Local help measured +85,120–91,091 ns/op, 41,428–41,430 B/op, and 509 allocations (baseline: +103,022–116,183 ns/op, 78,176–78,177 B/op, 810 allocations). Explicit help +measured 89,431–116,591 ns/op, 41,917–41,918 B/op, and 516 allocations +(baseline: 101,998–111,895 ns/op, 78,152–78,153 B/op, 815 allocations). +Parser usage measured 28,521–46,876 ns/op, 30,128–30,129 B/op, and 292 +allocations (baseline: 62,596–74,475 ns/op, 66,363–66,364 B/op, 591 +allocations). Handler usage measured 29,563–53,319 ns/op, 29,976–29,977 +B/op, and 284 allocations (baseline: 61,878–72,264 ns/op, 66,211–66,212 +B/op, 583 allocations). + +The focused wide repeat measured success at 178,004–194,710 ns/op, 247,796– +247,806 B/op, and 1,758 allocations (baseline: 218,073–253,010 ns/op, +269,063–269,070 B/op, 1,898 allocations). Wide help measured 227,764–243,610 +ns/op, 259,685–259,689 B/op, and 1,998 allocations (baseline: 491,172– +622,188 ns/op, 549,923–549,940 B/op, 4,033 allocations). Deep success +measured 151,780–172,540 ns/op, 124,410–124,412 B/op, and 881 allocations +(baseline: 119,958–139,625 ns/op, 139,787–139,791 B/op, 1,074 allocations), +so this run was slower while reducing bytes and allocations. Deep help measured +253,765–324,263 ns/op, 157,797–157,808 B/op, and 1,238 allocations; its +focused Stage 1 baseline was 300,155–326,094 ns/op, 294,882–294,896 B/op, and +2,408 allocations. + +Cold small construction plus first execution measured 37,934–42,626 ns/op, +38,411–38,412 B/op, and 340 allocations (baseline: 38,152–50,235 ns/op, +38,275–38,276 B/op, 339 allocations). Cold wide measured 267,018–335,726 +ns/op, 305,212–305,220 B/op, and 2,196 allocations (baseline: 251,945– +315,276 ns/op, 291,822–291,827 B/op, 2,184 allocations). Both timing ranges +overlap their baselines; retained session coordination adds a small cold +allocation/byte cost. Results show lower help/usage work and lower repeated +allocation costs, with timing variation on repeated success and deep dispatch. ## Stage 10: prepare configuration scope metadata once diff --git a/execute.go b/execute.go index c645bec..b0f1f8e 100644 --- a/execute.go +++ b/execute.go @@ -19,34 +19,111 @@ func (a *App) Execute(ctx context.Context, args []string, streams Streams) Outco return a.present(Failure(errors.New("invocation context must not be nil")), nil, streams) } streams = normalizeStreams(streams) - parsed := a.parse(args, internalcobra.ParseOptions{ - Context: ctx, - Out: streams.Out, - Err: streams.Err, - }) + current := newExecution(a, ctx, streams) + parsed := current.parse(args) if parsed.Failure != nil { - return a.present(Usage(errors.New(parsed.Failure.Message)), parsed.Path, streams) + return current.present(Usage(errors.New(parsed.Failure.Message)), parsed.Path) } command, ok := a.commandAt(parsed.Path) if !ok || command.payload == nil { - return a.present(a.renderHelpOutcome(parsed.Path, streams), parsed.Path, streams) + return current.present(current.renderHelp(parsed.Path), parsed.Path) } if parsed.HelpRequested || !command.payload.hasRun() { if _, err := command.payload.prepare(parsed.FlagValues, parsed.Operands, false); err != nil { - return a.present(Usage(err), parsed.Path, streams) + return current.present(Usage(err), parsed.Path) } - return a.present(a.renderHelpOutcome(parsed.Path, streams), parsed.Path, streams) + return current.present(current.renderHelp(parsed.Path), parsed.Path) } buildInput, err := command.payload.prepare(parsed.FlagValues, parsed.Operands, true) if err != nil { - return a.present(Usage(err), parsed.Path, streams) + return current.present(Usage(err), parsed.Path) } input := buildInput() if err := command.payload.checkInput(input); err != nil { - return a.present(Usage(err), parsed.Path, streams) + return current.present(Usage(err), parsed.Path) + } + if command.name == "help" { + help, ok := input.(*helpInput) + if !ok { + return current.present(Failure(errors.New("built-in help input has an unexpected type")), parsed.Path) + } + return current.present(current.renderHelpTargets(help.Targets.Value), parsed.Path) } outcome := command.payload.run(Invocation{Context: ctx, Streams: streams}, input) - return a.present(outcome, parsed.Path, streams) + return current.present(outcome, parsed.Path) +} + +type execution struct { + app *App + ctx context.Context + streams Streams + options internalcobra.ParseOptions + metadata *internalcobra.Command + session *internalcobra.Session +} + +func newExecution(app *App, ctx context.Context, streams Streams) *execution { + current := &execution{ + app: app, + ctx: ctx, + streams: streams, + options: internalcobra.ParseOptions{Context: ctx, Out: streams.Out, Err: streams.Err}, + } + if app != nil { + current.metadata = app.bridgeTree() + current.session = internalcobra.Prepare(current.metadata, current.options) + } + return current +} + +func (current *execution) parse(args []string) internalcobra.Result { + if current.app == nil { + return internalcobra.Result{Failure: &internalcobra.Failure{Message: "cannot parse commands on a nil app"}} + } + return current.session.Parse(args) +} + +func (current *execution) presentationSession() *internalcobra.Session { + if current.app == nil { + return nil + } + metadata := current.app.bridgeTree() + if current.session == nil || current.session.Metadata() != metadata { + current.metadata = metadata + current.session = internalcobra.Prepare(metadata, current.options) + } + return current.session +} + +func (current *execution) renderHelp(path []string) Outcome { + if current.app == nil { + return Failure(errors.New("cannot render help on a nil app")) + } + return current.app.renderHelpOutcomeWithSession(path, current.streams, current.presentationSession()) +} + +func (current *execution) renderHelpTargets(targets []string) Outcome { + if current.app == nil { + return Failure(errors.New("cannot render help on a nil app")) + } + return current.app.renderHelpTargetsWithSession(targets, current.streams, current.presentationSession()) +} + +func (current *execution) usageLine(path []string) string { + if current.app == nil { + return "Usage: command [command]" + } + session := current.presentationSession() + if session != nil { + if usage, failure := session.UsageLine(path); failure == nil { + return "Usage: " + usage + } + } + return current.app.usageFallback() +} + +func (current *execution) present(outcome Outcome, path []string) Outcome { + return current.app.presentWithUsageLine(outcome, path, current.streams, current.usageLine) } func normalizeStreams(streams Streams) Streams { @@ -88,6 +165,10 @@ func (a *App) commandAt(path []string) (Command, bool) { } func (a *App) present(outcome Outcome, path []string, streams Streams) Outcome { + return a.presentWithUsageLine(outcome, path, streams, a.usageLine) +} + +func (a *App) presentWithUsageLine(outcome Outcome, path []string, streams Streams, usageLine func([]string) string) Outcome { if outcome.Kind == "" && outcome.Status == 0 && outcome.Domain == "" && outcome.Cause == nil && outcome.DeliveryError == nil && !outcome.ReportIncomplete && len(outcome.Diagnostics) == 0 { @@ -105,7 +186,7 @@ func (a *App) present(outcome Outcome, path []string, streams Streams) Outcome { diagnostics = append(diagnostics, MessageDiagnostic(configkit.SeverityError, outcome.Cause.Error())) } if outcome.Kind == OutcomeUsage { - diagnostics = append(diagnostics, MessageDiagnostic(configkit.SeverityError, a.usageLine(path))) + diagnostics = append(diagnostics, MessageDiagnostic(configkit.SeverityError, usageLine(path))) } outcome.Diagnostics = diagnostics if len(diagnostics) == 0 { @@ -137,15 +218,19 @@ func (a *App) present(outcome Outcome, path []string, streams Streams) Outcome { } func (a *App) usageLine(path []string) string { - name := "command" - if a != nil && a.identity.Name != "" { - name = a.identity.Name - } if a != nil { if usage, failure := internalcobra.UsageLine(a.bridgeTree(), path); failure == nil { return "Usage: " + usage } } + return a.usageFallback() +} + +func (a *App) usageFallback() string { + name := "command" + if a != nil && a.identity.Name != "" { + name = a.identity.Name + } return fmt.Sprintf("Usage: %s [command]", name) } diff --git a/execute_external_test.go b/execute_external_test.go index 325ea20..a818a22 100644 --- a/execute_external_test.go +++ b/execute_external_test.go @@ -21,8 +21,9 @@ func TestExecuteRunsTypedCommandsWithFreshInvocationState(t *testing.T) { var inputs []input var contexts []context.Context var streams []commandkit.Streams - ctx := context.WithValue(context.Background(), struct{}{}, "invocation") - var out, diagnostic bytes.Buffer + firstContext := context.WithValue(context.Background(), struct{}{}, "first") + secondContext := context.WithValue(context.Background(), struct{}{}, "second") + var firstOut, secondOut, firstDiagnostic, secondDiagnostic bytes.Buffer command, err := commandkit.NewCommand(commandkit.CommandSpec[input]{ Name: "run", Flags: []commandkit.Flag[input]{ @@ -43,9 +44,9 @@ func TestExecuteRunsTypedCommandsWithFreshInvocationState(t *testing.T) { if err != nil { t.Fatal(err) } - first := app.Execute(ctx, []string{"run", "--name=first", "--id=a,b", "--id=c"}, commandkit.Streams{Out: &out, Err: &diagnostic}) - second := app.Execute(ctx, []string{"run"}, commandkit.Streams{Out: &out, Err: &diagnostic}) - third := app.Execute(ctx, []string{"run"}, commandkit.Streams{}) + first := app.Execute(firstContext, []string{"run", "--name=first", "--id=a,b", "--id=c"}, commandkit.Streams{Out: &firstOut, Err: &firstDiagnostic}) + second := app.Execute(secondContext, []string{"run"}, commandkit.Streams{Out: &secondOut, Err: &secondDiagnostic}) + third := app.Execute(context.Background(), []string{"run"}, commandkit.Streams{}) if first.Kind != commandkit.OutcomeSuccess || second.Kind != commandkit.OutcomeSuccess || third.Kind != commandkit.OutcomeSuccess { t.Fatalf("outcomes = %#v, %#v, and %#v", first, second, third) } @@ -58,8 +59,96 @@ func TestExecuteRunsTypedCommandsWithFreshInvocationState(t *testing.T) { if streams[2].Out != io.Discard || streams[2].Err != io.Discard { t.Fatalf("nil streams were not replaced with io.Discard: %#v", streams[2]) } - if contexts[0] != ctx || streams[0].Out != &out || streams[0].Err != &diagnostic { - t.Fatalf("handler invocation context or streams were not forwarded") + if contexts[0] != firstContext || contexts[1] != secondContext || streams[0].Out != &firstOut || streams[0].Err != &firstDiagnostic || streams[1].Out != &secondOut || streams[1].Err != &secondDiagnostic { + t.Fatalf("handler invocation contexts or streams were not forwarded independently") + } +} + +func TestExecuteUsesFreshSessionForHelpAfterFlagBearingInvocation(t *testing.T) { + type input struct { + name commandkit.Supplied[string] + enabled commandkit.Supplied[bool] + } + var received []input + command, err := commandkit.NewCommand(commandkit.CommandSpec[input]{ + Name: "run", + Flags: []commandkit.Flag[input]{ + commandkit.ValueFlag(commandkit.FlagMeta{Name: "name"}, (*string)(nil), func(value string) (string, error) { + return value, nil + }, func(target *input, value commandkit.Supplied[string]) { target.name = value }), + commandkit.BoolFlag(commandkit.FlagMeta{Name: "enabled"}, false, func(target *input, value commandkit.Supplied[bool]) { target.enabled = value }), + }, + Run: func(_ commandkit.Invocation, value input) commandkit.Outcome { + received = append(received, value) + return commandkit.Success() + }, + }) + if err != nil { + t.Fatal(err) + } + app, err := commandkit.New(commandkit.Identity{Name: "example"}, command) + if err != nil { + t.Fatal(err) + } + var firstOutput, secondOutput, diagnostic bytes.Buffer + first := app.Execute(context.Background(), []string{"run", "--name=chosen", "--enabled=false"}, commandkit.Streams{Out: &firstOutput, Err: &diagnostic}) + if first.Kind != commandkit.OutcomeSuccess { + t.Fatalf("flag-bearing invocation = %#v", first) + } + help := app.Execute(context.Background(), []string{"run", "--name=", "--help"}, commandkit.Streams{Out: &secondOutput, Err: &diagnostic}) + if help.Kind != commandkit.OutcomeSuccess || !strings.Contains(secondOutput.String(), "--enabled") { + t.Fatalf("help invocation = %#v, output %q", help, secondOutput.String()) + } + last := app.Execute(context.Background(), []string{"run"}, commandkit.Streams{}) + if last.Kind != commandkit.OutcomeSuccess { + t.Fatalf("omitted-value invocation = %#v", last) + } + if len(received) != 2 || received[0].name.Value != "chosen" || !received[0].name.Set || received[0].enabled.Value || !received[0].enabled.Set { + t.Fatalf("first input = %#v", received) + } + if received[1].name.Set || received[1].name.Value != "" || received[1].enabled.Set { + t.Fatalf("later invocation inherited flag state: %#v", received[1]) + } +} + +func TestExecutePresentsHandlerUsageAfterRegisteringCommand(t *testing.T) { + usageCause := errors.New("handler requests usage") + var app *commandkit.App + var calls int + late, err := commandkit.NewCommand(commandkit.CommandSpec[struct{}]{ + Name: "late", + Run: func(commandkit.Invocation, struct{}) commandkit.Outcome { return commandkit.Success() }, + }) + if err != nil { + t.Fatal(err) + } + command, err := commandkit.NewCommand(commandkit.CommandSpec[struct{}]{ + Name: "run", + Run: func(commandkit.Invocation, struct{}) commandkit.Outcome { + calls++ + if err := app.Register(late); err != nil { + return commandkit.Failure(err) + } + return commandkit.Usage(usageCause) + }, + }) + if err != nil { + t.Fatal(err) + } + app, err = commandkit.New(commandkit.Identity{Name: "example"}, command) + if err != nil { + t.Fatal(err) + } + var diagnostic bytes.Buffer + result := app.Execute(context.Background(), []string{"run"}, commandkit.Streams{Err: &diagnostic}) + if result.Kind != commandkit.OutcomeUsage || !errors.Is(result.Cause, usageCause) || calls != 1 { + t.Fatalf("handler usage = %#v, calls %d", result, calls) + } + if !strings.Contains(diagnostic.String(), "Usage: example run") { + t.Fatalf("handler usage diagnostic = %q", diagnostic.String()) + } + if result := app.Execute(context.Background(), []string{"late"}, commandkit.Streams{}); result.Kind != commandkit.OutcomeSuccess { + t.Fatalf("registered command execution = %#v", result) } } diff --git a/internal/cobra/parse.go b/internal/cobra/parse.go index 4d7afac..71b0526 100644 --- a/internal/cobra/parse.go +++ b/internal/cobra/parse.go @@ -69,6 +69,7 @@ type ParseOptions struct { // Session owns one mutable backend tree for a single command invocation. type Session struct { + metadata *Command root *spfcobra.Command declarations map[*spfcobra.Command]*Command } @@ -79,11 +80,20 @@ func Prepare(root *Command, options ParseOptions) *Session { if root == nil { return session } + session.metadata = root options = normalizeOptions(options) session.root, session.declarations = buildTree(root, options) return session } +// Metadata returns the neutral tree used to prepare this session. +func (session *Session) Metadata() *Command { + if session == nil { + return nil + } + return session.metadata +} + // Parse routes leading command words and parses flags on this session's tree. func (session *Session) Parse(argv []string) Result { result := Result{} diff --git a/parser_test.go b/parser_test.go index c0cb725..3c910a5 100644 --- a/parser_test.go +++ b/parser_test.go @@ -53,6 +53,48 @@ func TestAppParseUsesRegisteredBindingsWithTheAdapter(t *testing.T) { } } +func TestExecutionRefreshesHelpMetadataAfterRegistration(t *testing.T) { + first, err := NewCommand(CommandSpec[struct{}]{ + Name: "first", + Run: func(Invocation, struct{}) Outcome { return Success() }, + }) + if err != nil { + t.Fatal(err) + } + app, err := New(Identity{Name: "example"}, first) + if err != nil { + t.Fatal(err) + } + var output bytes.Buffer + current := newExecution(app, context.Background(), Streams{Out: &output, Err: io.Discard}) + parsed := current.parse([]string{"first"}) + if parsed.Failure != nil { + t.Fatalf("initial parse failed: %s", parsed.Failure.Message) + } + + late, err := NewCommand(CommandSpec[struct{}]{ + Name: "late", + Summary: "registered during execution", + Run: func(Invocation, struct{}) Outcome { return Success() }, + }) + if err != nil { + t.Fatal(err) + } + if err := app.Register(late); err != nil { + t.Fatal(err) + } + + if got := current.renderHelp([]string{"late"}); got.Kind != OutcomeSuccess { + t.Fatalf("help after registration = %#v", got) + } + if !strings.Contains(output.String(), "registered during execution") { + t.Fatalf("help did not use registered metadata: %q", output.String()) + } + if got := current.usageLine([]string{"late"}); !strings.HasPrefix(got, "Usage: example late") { + t.Fatalf("usage after registration = %q", got) + } +} + func TestBridgeTreeRefreshesAfterSuccessfulRegistration(t *testing.T) { app, err := New(Identity{Name: "example"}) if err != nil {