Index backend commands for path lookup
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# Performance implementation plan
|
# Performance implementation plan
|
||||||
|
|
||||||
Status: implementation in progress; Stages 1–3 are complete, Stages 4–11 have not started.
|
Status: implementation in progress; Stages 1–4 are complete, Stages 5–11 have not started.
|
||||||
|
|
||||||
Date: 2026-10-09.
|
Date: 2026-10-09.
|
||||||
|
|
||||||
@@ -376,7 +376,7 @@ construction; text requests retain their report semantics and formatting errors.
|
|||||||
|
|
||||||
## Stage 4: index backend declarations and traverse parents
|
## Stage 4: index backend declarations and traverse parents
|
||||||
|
|
||||||
**Status:** not started. **Prerequisite:** Stage 3.
|
**Status:** complete. **Prerequisite:** Stage 3.
|
||||||
|
|
||||||
**Files:** [adapter](../../internal/cobra/parse.go),
|
**Files:** [adapter](../../internal/cobra/parse.go),
|
||||||
[adapter tests](../../internal/cobra/parse_test.go), relevant benchmarks, and
|
[adapter tests](../../internal/cobra/parse_test.go), relevant benchmarks, and
|
||||||
@@ -402,7 +402,30 @@ parse and dispatch benchmarks.
|
|||||||
**Completion:** declaration lookup uses the index and ancestry traversal is
|
**Completion:** declaration lookup uses the index and ancestry traversal is
|
||||||
proportional to selected depth after backend construction.
|
proportional to selected depth after backend construction.
|
||||||
|
|
||||||
**Evidence:** pending.
|
**Evidence:** `go test -mod=readonly -count=1 ./internal/cobra .` passed with
|
||||||
|
the common offline environment settings. The focused test covers a 24-level
|
||||||
|
path, 128 siblings, root selection, unknown help/usage paths, and exact help and
|
||||||
|
usage resolution. Existing exact-path and parse failure tests remain in place.
|
||||||
|
|
||||||
|
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 `628dc78` with this stage's
|
||||||
|
changes in the working tree. The five-sample adapter command was
|
||||||
|
`GOWORK=off GOTOOLCHAIN=local GOPROXY=off GONOPROXY=none GOSUMDB=off go test -mod=readonly -run '^$' -bench '^BenchmarkPerformanceParse$' -benchmem -benchtime=200ms -count=5 ./internal/cobra`;
|
||||||
|
the dispatch command used the same flags with `-bench
|
||||||
|
'^BenchmarkPerformanceDispatch$' .`. Both passed. Raw outputs are in
|
||||||
|
`/tmp/commandkit-performance-parse-after-stage4.txt` and
|
||||||
|
`/tmp/commandkit-performance-dispatch-after-stage4.txt`.
|
||||||
|
|
||||||
|
Adapter shallow/wide Parse measured 149,426–171,199 ns/op,
|
||||||
|
242,936 B/op, and 1,705 allocs/op, compared with baseline 150,918–171,598
|
||||||
|
ns/op, 242,936 B/op, and 1,705 allocs/op. Deep Parse measured 90,608–104,770
|
||||||
|
ns/op, 119,536 B/op, and 826 allocs/op, compared with baseline 86,282–109,697
|
||||||
|
ns/op, 119,920 B/op, and 827 allocs/op. Timings overlap; deep parsing saves
|
||||||
|
384 B and one allocation. End-to-end dispatch allocations remain effectively
|
||||||
|
unchanged across the measured cases. Timing variation, including deep-help
|
||||||
|
outliers, is substantial, so these runs do not support an end-to-end speedup
|
||||||
|
claim. The comparison isolates the selected-path lookup change while backend
|
||||||
|
tree construction remains in each parse.
|
||||||
|
|
||||||
## Stage 5: scan YAML preambles without splitting documents
|
## Stage 5: scan YAML preambles without splitting documents
|
||||||
|
|
||||||
|
|||||||
@@ -71,8 +71,9 @@ func RenderHelp(root *Command, path []string) (string, *Failure) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return "", &Failure{Message: err.Error()}
|
return "", &Failure{Message: err.Error()}
|
||||||
}
|
}
|
||||||
declaration := declarationFor(nodes, selected)
|
declaration := nodes[selected]
|
||||||
if declaration == nil || len(remaining) != 0 || !equalPath(path, pathFor(root, declaration)) {
|
selectedPath, pathOK := pathFor(selected, nodes)
|
||||||
|
if declaration == nil || !pathOK || len(remaining) != 0 || !equalPath(path, selectedPath) {
|
||||||
return "", &Failure{Message: "help path is not registered"}
|
return "", &Failure{Message: "help path is not registered"}
|
||||||
}
|
}
|
||||||
selected.SetOut(buffer)
|
selected.SetOut(buffer)
|
||||||
@@ -91,8 +92,9 @@ func UsageLine(root *Command, path []string) (string, *Failure) {
|
|||||||
options := normalizeOptions(ParseOptions{Context: context.Background(), In: strings.NewReader(""), Out: io.Discard, Err: io.Discard})
|
options := normalizeOptions(ParseOptions{Context: context.Background(), In: strings.NewReader(""), Out: io.Discard, Err: io.Discard})
|
||||||
backendRoot, nodes := buildTree(root, options)
|
backendRoot, nodes := buildTree(root, options)
|
||||||
selected, remaining, err := backendRoot.Find(path)
|
selected, remaining, err := backendRoot.Find(path)
|
||||||
declaration := declarationFor(nodes, selected)
|
declaration := nodes[selected]
|
||||||
if err != nil || declaration == nil || len(remaining) != 0 || !equalPath(path, pathFor(root, declaration)) {
|
selectedPath, pathOK := pathFor(selected, nodes)
|
||||||
|
if err != nil || declaration == nil || !pathOK || len(remaining) != 0 || !equalPath(path, selectedPath) {
|
||||||
return "", &Failure{Message: "usage path is not registered"}
|
return "", &Failure{Message: "usage path is not registered"}
|
||||||
}
|
}
|
||||||
line := selected.UseLine()
|
line := selected.UseLine()
|
||||||
@@ -128,11 +130,15 @@ func Parse(root *Command, argv []string, options ParseOptions) Result {
|
|||||||
if selected == nil {
|
if selected == nil {
|
||||||
return failure(result, "no command was selected")
|
return failure(result, "no command was selected")
|
||||||
}
|
}
|
||||||
selectedNode := declarationFor(nodes, selected)
|
selectedNode := nodes[selected]
|
||||||
if selectedNode == nil {
|
if selectedNode == nil {
|
||||||
return failure(result, "selected command is not part of the declared tree")
|
return failure(result, "selected command is not part of the declared tree")
|
||||||
}
|
}
|
||||||
result.Path = pathFor(root, selectedNode)
|
var pathOK bool
|
||||||
|
result.Path, pathOK = pathFor(selected, nodes)
|
||||||
|
if !pathOK {
|
||||||
|
return failure(result, "selected command is not part of the declared tree")
|
||||||
|
}
|
||||||
consumed := len(leading) - len(unconsumed)
|
consumed := len(leading) - len(unconsumed)
|
||||||
if consumed != len(result.Path) || !equalPath(leading[:consumed], result.Path) {
|
if consumed != len(result.Path) || !equalPath(leading[:consumed], result.Path) {
|
||||||
return failure(result, "command path must match a registered command exactly")
|
return failure(result, "command path must match a registered command exactly")
|
||||||
@@ -180,8 +186,8 @@ func normalizeOptions(options ParseOptions) ParseOptions {
|
|||||||
return options
|
return options
|
||||||
}
|
}
|
||||||
|
|
||||||
func buildTree(root *Command, options ParseOptions) (*spfcobra.Command, map[*Command]*spfcobra.Command) {
|
func buildTree(root *Command, options ParseOptions) (*spfcobra.Command, map[*spfcobra.Command]*Command) {
|
||||||
nodes := make(map[*Command]*spfcobra.Command)
|
nodes := make(map[*spfcobra.Command]*Command)
|
||||||
var build func(*Command) *spfcobra.Command
|
var build func(*Command) *spfcobra.Command
|
||||||
build = func(declaration *Command) *spfcobra.Command {
|
build = func(declaration *Command) *spfcobra.Command {
|
||||||
backend := &spfcobra.Command{
|
backend := &spfcobra.Command{
|
||||||
@@ -223,7 +229,7 @@ func buildTree(root *Command, options ParseOptions) (*spfcobra.Command, map[*Com
|
|||||||
// Cobra's automatic help flag is suppressed because CommandKit declares
|
// Cobra's automatic help flag is suppressed because CommandKit declares
|
||||||
// and interprets the local help flag itself.
|
// and interprets the local help flag itself.
|
||||||
backend.SetHelpCommand(nil)
|
backend.SetHelpCommand(nil)
|
||||||
nodes[declaration] = backend
|
nodes[backend] = declaration
|
||||||
for _, child := range declaration.Children {
|
for _, child := range declaration.Children {
|
||||||
backend.AddCommand(build(child))
|
backend.AddCommand(build(child))
|
||||||
}
|
}
|
||||||
@@ -320,15 +326,6 @@ func equalPath(left, right []string) bool {
|
|||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
func declarationFor(nodes map[*Command]*spfcobra.Command, selected *spfcobra.Command) *Command {
|
|
||||||
for declaration, backend := range nodes {
|
|
||||||
if backend == selected {
|
|
||||||
return declaration
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
|
|
||||||
func splitRoutingPrefix(argv []string) ([]string, []string) {
|
func splitRoutingPrefix(argv []string) ([]string, []string) {
|
||||||
for index, argument := range argv {
|
for index, argument := range argv {
|
||||||
if argument == "--" || (strings.HasPrefix(argument, "-") && argument != "-") {
|
if argument == "--" || (strings.HasPrefix(argument, "-") && argument != "-") {
|
||||||
@@ -338,29 +335,21 @@ func splitRoutingPrefix(argv []string) ([]string, []string) {
|
|||||||
return append([]string(nil), argv...), nil
|
return append([]string(nil), argv...), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func pathFor(root, selected *Command) []string {
|
func pathFor(selected *spfcobra.Command, declarations map[*spfcobra.Command]*Command) ([]string, bool) {
|
||||||
var reverse []string
|
var reverse []string
|
||||||
for current := selected; current != nil && current != root; {
|
for current := selected; current != nil; current = current.Parent() {
|
||||||
reverse = append(reverse, current.Name)
|
declaration := declarations[current]
|
||||||
current = parentOf(root, current)
|
if declaration == nil {
|
||||||
}
|
return nil, false
|
||||||
path := make([]string, len(reverse))
|
|
||||||
for index := range reverse {
|
|
||||||
path[len(reverse)-index-1] = reverse[index]
|
|
||||||
}
|
|
||||||
return path
|
|
||||||
}
|
|
||||||
|
|
||||||
func parentOf(root, target *Command) *Command {
|
|
||||||
for _, child := range root.Children {
|
|
||||||
if child == target {
|
|
||||||
return root
|
|
||||||
}
|
}
|
||||||
if parent := parentOf(child, target); parent != nil {
|
if current.Parent() != nil {
|
||||||
return parent
|
reverse = append(reverse, declaration.Name)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return nil
|
for left, right := 0, len(reverse)-1; left < right; left, right = left+1, right-1 {
|
||||||
|
reverse[left], reverse[right] = reverse[right], reverse[left]
|
||||||
|
}
|
||||||
|
return reverse, true
|
||||||
}
|
}
|
||||||
|
|
||||||
func failure(result Result, message string) Result {
|
func failure(result Result, message string) Result {
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package cobra
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"reflect"
|
"reflect"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -130,3 +131,51 @@ func TestParseReturnsNeutralFailuresAndFreshState(t *testing.T) {
|
|||||||
t.Fatalf("unknown flag failure = %#v", failed.Failure)
|
t.Fatalf("unknown flag failure = %#v", failed.Failure)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSelectedPathsResolveThroughBackendParents(t *testing.T) {
|
||||||
|
root := &Command{Name: "app"}
|
||||||
|
for index := 0; index < 128; index++ {
|
||||||
|
root.Children = append(root.Children, &Command{Name: fmt.Sprintf("wide%03d", index), Runnable: true})
|
||||||
|
}
|
||||||
|
path := make([]string, 24)
|
||||||
|
current := root
|
||||||
|
for index := range path {
|
||||||
|
path[index] = fmt.Sprintf("deep%02d", index)
|
||||||
|
child := &Command{Name: path[index]}
|
||||||
|
if index == len(path)-1 {
|
||||||
|
child.Runnable = true
|
||||||
|
}
|
||||||
|
current.Children = append(current.Children, child)
|
||||||
|
current = child
|
||||||
|
}
|
||||||
|
|
||||||
|
result := Parse(root, path, ParseOptions{})
|
||||||
|
if result.Failure != nil || !reflect.DeepEqual(result.Path, path) {
|
||||||
|
t.Fatalf("deep parse path = %#v", result)
|
||||||
|
}
|
||||||
|
rootResult := Parse(root, nil, ParseOptions{})
|
||||||
|
if rootResult.Failure != nil || len(rootResult.Path) != 0 {
|
||||||
|
t.Fatalf("root parse path = %#v", rootResult)
|
||||||
|
}
|
||||||
|
|
||||||
|
help, failure := RenderHelp(root, path)
|
||||||
|
if failure != nil || !strings.Contains(help, path[len(path)-1]) {
|
||||||
|
t.Fatalf("deep help = %q, %v", help, failure)
|
||||||
|
}
|
||||||
|
if _, failure := RenderHelp(root, []string{"wide128"}); failure == nil {
|
||||||
|
t.Fatal("help accepted an unknown sibling")
|
||||||
|
}
|
||||||
|
line, failure := UsageLine(root, path)
|
||||||
|
if failure != nil || !strings.Contains(line, path[len(path)-1]) {
|
||||||
|
t.Fatalf("deep usage line = %q, %v", line, failure)
|
||||||
|
}
|
||||||
|
if _, failure := UsageLine(root, []string{"wide128"}); failure == nil {
|
||||||
|
t.Fatal("usage accepted an unknown sibling")
|
||||||
|
}
|
||||||
|
if _, failure := RenderHelp(root, nil); failure != nil {
|
||||||
|
t.Fatalf("root help failed: %v", failure)
|
||||||
|
}
|
||||||
|
if _, failure := UsageLine(root, nil); failure != nil {
|
||||||
|
t.Fatalf("root usage failed: %v", failure)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user