fix: address final m5 m6 review gaps
- commit
- e7f30ce9902a79b366d7963682cd2d95e516a758
- author
- brice · · 2026-07-02 17:13 UTC
- parent
- f5b4f9b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +27 -11
M docs/performance.md +4 -6
| @@ -8,19 +8,17 @@ Run microbenchmarks with: | |||
| 8 | 8 | go test ./pkg/renderer ./pkg/vt ./internal/adapters/ipc -bench=. -benchmem | |
| 9 | 9 | ``` | |
| 10 | 10 | ||
| 11 | - | Demo flood workloads are scripted in `scripts/bench-workloads.sh` for `yes`, `seq`, and `cat` styles. Capture bytes/frame and syscalls/frame externally (for example with `strace -c` around the client/daemon) and allocations with `-benchmem`/pprof. | |
| 11 | + | Demo flood workloads are scripted in `scripts/bench-workloads.sh` for `yes`, `seq`, and `cat` styles for raw/tmux runs. vev does not currently support `vev new <name> -- command`, so automated vev workload command overrides are intentionally not documented. Capture bytes/frame and syscalls/frame externally (for example with `strace -c` around the client/daemon) and allocations with `-benchmem`/pprof. | |
| 12 | 12 | ||
| 13 | 13 | ## Local baseline status | |
| 14 | 14 | ||
| 15 | - | Full end-to-end vev-vs-tmux workload baselines are deferred/waived for this non-interactive branch because they require an interactive terminal/daemon pair and external tracing tools around both processes. Run them later from an interactive shell with these exact command shapes, substituting only the vev binary path, tmux binary path, terminal size, duration, and sample file path: | |
| 15 | + | Full end-to-end vev-vs-tmux workload baselines are deferred/waived for this non-interactive branch because they require an interactive terminal/daemon pair, external tracing tools around both processes, and (for automated vev flood workloads) future command override support. Until that exists, only start the daemon/client normally and do not claim a scripted vev flood command. tmux/raw comparison commands remain useful for shaping workloads: | |
| 16 | 16 | ||
| 17 | 17 | ```sh | |
| 18 | - | VEV_PPROF_ADDR=127.0.0.1:6060 strace -ff -c -o vev-yes.strace vev --daemon | |
| 19 | - | strace -ff -c -o vev-client-yes.strace vev new perf-yes -- sh -lc 'timeout 30s yes' | |
| 18 | + | VEV_PPROF_ADDR=127.0.0.1:6060 strace -ff -c -o vev-daemon.strace vev --daemon | |
| 19 | + | strace -ff -c -o vev-client.strace vev new perf-manual | |
| 20 | 20 | strace -ff -c -o tmux-yes.strace tmux new-session -d -s perf-yes 'timeout 30s yes' | |
| 21 | - | strace -ff -c -o vev-client-seq.strace vev new perf-seq -- sh -lc 'timeout 30s sh -c "while :; do seq 1 10000; done"' | |
| 22 | 21 | strace -ff -c -o tmux-seq.strace tmux new-session -d -s perf-seq 'timeout 30s sh -c "while :; do seq 1 10000; done"' | |
| 23 | - | strace -ff -c -o vev-client-cat.strace vev new perf-cat -- sh -lc 'timeout 30s sh -c "while :; do cat /path/to/sample.txt; done"' | |
| 24 | 22 | strace -ff -c -o tmux-cat.strace tmux new-session -d -s perf-cat 'timeout 30s sh -c "while :; do cat /path/to/sample.txt; done"' | |
| 25 | 23 | ``` | |
| 26 | 24 | ||
| @@ -8,19 +8,17 @@ Run microbenchmarks with: | |||||
| 8 | go test ./pkg/renderer ./pkg/vt ./internal/adapters/ipc -bench=. -benchmem | 8 | go test ./pkg/renderer ./pkg/vt ./internal/adapters/ipc -bench=. -benchmem | ||
| 9 | ``` | 9 | ``` | ||
| 10 | 10 | ||||
| 11 | - | Demo flood workloads are scripted in `scripts/bench-workloads.sh` for `yes`, `seq`, and `cat` styles. Capture bytes/frame and syscalls/frame externally (for example with `strace -c` around the client/daemon) and allocations with `-benchmem`/pprof. | 11 | + | Demo flood workloads are scripted in `scripts/bench-workloads.sh` for `yes`, `seq`, and `cat` styles for raw/tmux runs. vev does not currently support `vev new <name> -- command`, so automated vev workload command overrides are intentionally not documented. Capture bytes/frame and syscalls/frame externally (for example with `strace -c` around the client/daemon) and allocations with `-benchmem`/pprof. |
| 12 | 12 | ||||
| 13 | ## Local baseline status | 13 | ## Local baseline status | ||
| 14 | 14 | ||||
| 15 | - | Full end-to-end vev-vs-tmux workload baselines are deferred/waived for this non-interactive branch because they require an interactive terminal/daemon pair and external tracing tools around both processes. Run them later from an interactive shell with these exact command shapes, substituting only the vev binary path, tmux binary path, terminal size, duration, and sample file path: | 15 | + | Full end-to-end vev-vs-tmux workload baselines are deferred/waived for this non-interactive branch because they require an interactive terminal/daemon pair, external tracing tools around both processes, and (for automated vev flood workloads) future command override support. Until that exists, only start the daemon/client normally and do not claim a scripted vev flood command. tmux/raw comparison commands remain useful for shaping workloads: |
| 16 | 16 | ||||
| 17 | ```sh | 17 | ```sh | ||
| 18 | - | VEV_PPROF_ADDR=127.0.0.1:6060 strace -ff -c -o vev-yes.strace vev --daemon | 18 | + | VEV_PPROF_ADDR=127.0.0.1:6060 strace -ff -c -o vev-daemon.strace vev --daemon |
| 19 | - | strace -ff -c -o vev-client-yes.strace vev new perf-yes -- sh -lc 'timeout 30s yes' | 19 | + | strace -ff -c -o vev-client.strace vev new perf-manual |
| 20 | strace -ff -c -o tmux-yes.strace tmux new-session -d -s perf-yes 'timeout 30s yes' | 20 | strace -ff -c -o tmux-yes.strace tmux new-session -d -s perf-yes 'timeout 30s yes' | ||
| 21 | - | strace -ff -c -o vev-client-seq.strace vev new perf-seq -- sh -lc 'timeout 30s sh -c "while :; do seq 1 10000; done"' | |||
| 22 | strace -ff -c -o tmux-seq.strace tmux new-session -d -s perf-seq 'timeout 30s sh -c "while :; do seq 1 10000; done"' | 21 | strace -ff -c -o tmux-seq.strace tmux new-session -d -s perf-seq 'timeout 30s sh -c "while :; do seq 1 10000; done"' | ||
| 23 | - | strace -ff -c -o vev-client-cat.strace vev new perf-cat -- sh -lc 'timeout 30s sh -c "while :; do cat /path/to/sample.txt; done"' | |||
| 24 | strace -ff -c -o tmux-cat.strace tmux new-session -d -s perf-cat 'timeout 30s sh -c "while :; do cat /path/to/sample.txt; done"' | 22 | strace -ff -c -o tmux-cat.strace tmux new-session -d -s perf-cat 'timeout 30s sh -c "while :; do cat /path/to/sample.txt; done"' | ||
| 25 | ``` | 23 | ``` | ||
| 26 | 24 | ||||
M internal/app/run.go +12 -0
| @@ -110,15 +110,24 @@ func parseArgs(args []string) (command, error) { | |||
| 110 | 110 | if len(args) < 2 || args[1] == "" { | |
| 111 | 111 | return command{}, usagef("`new` requires a session name") | |
| 112 | 112 | } | |
| 113 | + | if len(args) > 2 { | |
| 114 | + | return command{}, usagef("`new` does not support command overrides") | |
| 115 | + | } | |
| 113 | 116 | return command{kind: kindAttach, intent: ports.IntentNew, name: args[1]}, nil | |
| 114 | 117 | case "attach", "a": | |
| 115 | 118 | if len(args) < 2 || args[1] == "" { | |
| 116 | 119 | return command{}, usagef("`attach` requires a session name") | |
| 117 | 120 | } | |
| 121 | + | if len(args) > 2 { | |
| 122 | + | return command{}, usagef("`attach` accepts exactly one session name or remote target") | |
| 123 | + | } | |
| 118 | 124 | cmd := command{kind: kindAttach, intent: ports.IntentAttach, name: args[1]} | |
| 119 | 125 | if target, session, ok := parseRemoteAttachTarget(args[1]); ok { | |
| 120 | 126 | cmd.remoteTarget = target | |
| 121 | 127 | cmd.name = session | |
| 128 | + | if session == "" { | |
| 129 | + | cmd.intent = ports.IntentEphemeral | |
| 130 | + | } | |
| 122 | 131 | } | |
| 123 | 132 | return cmd, nil | |
| 124 | 133 | case "ls", "list": | |
| @@ -127,6 +136,9 @@ func parseArgs(args []string) (command, error) { | |||
| 127 | 136 | if len(args) < 2 || args[1] == "" { | |
| 128 | 137 | return command{}, usagef("`kill` requires a session name") | |
| 129 | 138 | } | |
| 139 | + | if len(args) > 2 { | |
| 140 | + | return command{}, usagef("`kill` accepts exactly one session name") | |
| 141 | + | } | |
| 130 | 142 | return command{kind: kindKill, name: args[1]}, nil | |
| 131 | 143 | case "-h", "--help", "help": | |
| 132 | 144 | return command{kind: kindHelp}, nil | |
| @@ -110,15 +110,24 @@ func parseArgs(args []string) (command, error) { | |||||
| 110 | if len(args) < 2 || args[1] == "" { | 110 | if len(args) < 2 || args[1] == "" { | ||
| 111 | return command{}, usagef("`new` requires a session name") | 111 | return command{}, usagef("`new` requires a session name") | ||
| 112 | } | 112 | } | ||
| 113 | + | if len(args) > 2 { | |||
| 114 | + | return command{}, usagef("`new` does not support command overrides") | |||
| 115 | + | } | |||
| 113 | return command{kind: kindAttach, intent: ports.IntentNew, name: args[1]}, nil | 116 | return command{kind: kindAttach, intent: ports.IntentNew, name: args[1]}, nil | ||
| 114 | case "attach", "a": | 117 | case "attach", "a": | ||
| 115 | if len(args) < 2 || args[1] == "" { | 118 | if len(args) < 2 || args[1] == "" { | ||
| 116 | return command{}, usagef("`attach` requires a session name") | 119 | return command{}, usagef("`attach` requires a session name") | ||
| 117 | } | 120 | } | ||
| 121 | + | if len(args) > 2 { | |||
| 122 | + | return command{}, usagef("`attach` accepts exactly one session name or remote target") | |||
| 123 | + | } | |||
| 118 | cmd := command{kind: kindAttach, intent: ports.IntentAttach, name: args[1]} | 124 | cmd := command{kind: kindAttach, intent: ports.IntentAttach, name: args[1]} | ||
| 119 | if target, session, ok := parseRemoteAttachTarget(args[1]); ok { | 125 | if target, session, ok := parseRemoteAttachTarget(args[1]); ok { | ||
| 120 | cmd.remoteTarget = target | 126 | cmd.remoteTarget = target | ||
| 121 | cmd.name = session | 127 | cmd.name = session | ||
| 128 | + | if session == "" { | |||
| 129 | + | cmd.intent = ports.IntentEphemeral | |||
| 130 | + | } | |||
| 122 | } | 131 | } | ||
| 123 | return cmd, nil | 132 | return cmd, nil | ||
| 124 | case "ls", "list": | 133 | case "ls", "list": | ||
| @@ -127,6 +136,9 @@ func parseArgs(args []string) (command, error) { | |||||
| 127 | if len(args) < 2 || args[1] == "" { | 136 | if len(args) < 2 || args[1] == "" { | ||
| 128 | return command{}, usagef("`kill` requires a session name") | 137 | return command{}, usagef("`kill` requires a session name") | ||
| 129 | } | 138 | } | ||
| 139 | + | if len(args) > 2 { | |||
| 140 | + | return command{}, usagef("`kill` accepts exactly one session name") | |||
| 141 | + | } | |||
| 130 | return command{kind: kindKill, name: args[1]}, nil | 142 | return command{kind: kindKill, name: args[1]}, nil | ||
| 131 | case "-h", "--help", "help": | 143 | case "-h", "--help", "help": | ||
| 132 | return command{kind: kindHelp}, nil | 144 | return command{kind: kindHelp}, nil | ||
M internal/app/run_test.go +5 -1
| @@ -22,15 +22,19 @@ func TestParseArgs(t *testing.T) { | |||
| 22 | 22 | {name: "new named", args: []string{"new", "work"}, wantKind: kindAttach, wantIntent: ports.IntentNew, wantName: "work"}, | |
| 23 | 23 | {name: "new without name", args: []string{"new"}, wantErr: true}, | |
| 24 | 24 | {name: "new empty name", args: []string{"new", ""}, wantErr: true}, | |
| 25 | + | {name: "new command override unsupported", args: []string{"new", "work", "--", "sh"}, wantErr: true}, | |
| 25 | 26 | {name: "attach named", args: []string{"attach", "work"}, wantKind: kindAttach, wantIntent: ports.IntentAttach, wantName: "work"}, | |
| 26 | 27 | {name: "attach alias a", args: []string{"a", "work"}, wantKind: kindAttach, wantIntent: ports.IntentAttach, wantName: "work"}, | |
| 27 | - | {name: "attach remote without session", args: []string{"attach", "user@example.com"}, wantKind: kindAttach, wantIntent: ports.IntentAttach, wantRemote: "user@example.com"}, | |
| 28 | + | {name: "attach remote without session uses ephemeral", args: []string{"attach", "user@example.com"}, wantKind: kindAttach, wantIntent: ports.IntentEphemeral, wantRemote: "user@example.com"}, | |
| 29 | + | {name: "attach remote with empty session uses ephemeral", args: []string{"attach", "user@example.com:"}, wantKind: kindAttach, wantIntent: ports.IntentEphemeral, wantRemote: "user@example.com"}, | |
| 28 | 30 | {name: "attach remote with session", args: []string{"attach", "user@example.com:work"}, wantKind: kindAttach, wantIntent: ports.IntentAttach, wantName: "work", wantRemote: "user@example.com"}, | |
| 31 | + | {name: "attach extra arg", args: []string{"attach", "work", "extra"}, wantErr: true}, | |
| 29 | 32 | {name: "attach without name", args: []string{"attach"}, wantErr: true}, | |
| 30 | 33 | {name: "ls", args: []string{"ls"}, wantKind: kindList}, | |
| 31 | 34 | {name: "list", args: []string{"list"}, wantKind: kindList}, | |
| 32 | 35 | {name: "kill named", args: []string{"kill", "work"}, wantKind: kindKill, wantName: "work"}, | |
| 33 | 36 | {name: "kill without name", args: []string{"kill"}, wantErr: true}, | |
| 37 | + | {name: "kill extra arg", args: []string{"kill", "work", "extra"}, wantErr: true}, | |
| 34 | 38 | {name: "daemon", args: []string{"--daemon"}, wantKind: kindDaemon}, | |
| 35 | 39 | {name: "stdio", args: []string{"_stdio"}, wantKind: kindStdio}, | |
| 36 | 40 | {name: "stdio with session", args: []string{"_stdio", "work"}, wantKind: kindStdio, wantName: "work"}, | |
| @@ -22,15 +22,19 @@ func TestParseArgs(t *testing.T) { | |||||
| 22 | {name: "new named", args: []string{"new", "work"}, wantKind: kindAttach, wantIntent: ports.IntentNew, wantName: "work"}, | 22 | {name: "new named", args: []string{"new", "work"}, wantKind: kindAttach, wantIntent: ports.IntentNew, wantName: "work"}, | ||
| 23 | {name: "new without name", args: []string{"new"}, wantErr: true}, | 23 | {name: "new without name", args: []string{"new"}, wantErr: true}, | ||
| 24 | {name: "new empty name", args: []string{"new", ""}, wantErr: true}, | 24 | {name: "new empty name", args: []string{"new", ""}, wantErr: true}, | ||
| 25 | + | {name: "new command override unsupported", args: []string{"new", "work", "--", "sh"}, wantErr: true}, | |||
| 25 | {name: "attach named", args: []string{"attach", "work"}, wantKind: kindAttach, wantIntent: ports.IntentAttach, wantName: "work"}, | 26 | {name: "attach named", args: []string{"attach", "work"}, wantKind: kindAttach, wantIntent: ports.IntentAttach, wantName: "work"}, | ||
| 26 | {name: "attach alias a", args: []string{"a", "work"}, wantKind: kindAttach, wantIntent: ports.IntentAttach, wantName: "work"}, | 27 | {name: "attach alias a", args: []string{"a", "work"}, wantKind: kindAttach, wantIntent: ports.IntentAttach, wantName: "work"}, | ||
| 27 | - | {name: "attach remote without session", args: []string{"attach", "user@example.com"}, wantKind: kindAttach, wantIntent: ports.IntentAttach, wantRemote: "user@example.com"}, | 28 | + | {name: "attach remote without session uses ephemeral", args: []string{"attach", "user@example.com"}, wantKind: kindAttach, wantIntent: ports.IntentEphemeral, wantRemote: "user@example.com"}, |
| 29 | + | {name: "attach remote with empty session uses ephemeral", args: []string{"attach", "user@example.com:"}, wantKind: kindAttach, wantIntent: ports.IntentEphemeral, wantRemote: "user@example.com"}, | |||
| 28 | {name: "attach remote with session", args: []string{"attach", "user@example.com:work"}, wantKind: kindAttach, wantIntent: ports.IntentAttach, wantName: "work", wantRemote: "user@example.com"}, | 30 | {name: "attach remote with session", args: []string{"attach", "user@example.com:work"}, wantKind: kindAttach, wantIntent: ports.IntentAttach, wantName: "work", wantRemote: "user@example.com"}, | ||
| 31 | + | {name: "attach extra arg", args: []string{"attach", "work", "extra"}, wantErr: true}, | |||
| 29 | {name: "attach without name", args: []string{"attach"}, wantErr: true}, | 32 | {name: "attach without name", args: []string{"attach"}, wantErr: true}, | ||
| 30 | {name: "ls", args: []string{"ls"}, wantKind: kindList}, | 33 | {name: "ls", args: []string{"ls"}, wantKind: kindList}, | ||
| 31 | {name: "list", args: []string{"list"}, wantKind: kindList}, | 34 | {name: "list", args: []string{"list"}, wantKind: kindList}, | ||
| 32 | {name: "kill named", args: []string{"kill", "work"}, wantKind: kindKill, wantName: "work"}, | 35 | {name: "kill named", args: []string{"kill", "work"}, wantKind: kindKill, wantName: "work"}, | ||
| 33 | {name: "kill without name", args: []string{"kill"}, wantErr: true}, | 36 | {name: "kill without name", args: []string{"kill"}, wantErr: true}, | ||
| 37 | + | {name: "kill extra arg", args: []string{"kill", "work", "extra"}, wantErr: true}, | |||
| 34 | {name: "daemon", args: []string{"--daemon"}, wantKind: kindDaemon}, | 38 | {name: "daemon", args: []string{"--daemon"}, wantKind: kindDaemon}, | ||
| 35 | {name: "stdio", args: []string{"_stdio"}, wantKind: kindStdio}, | 39 | {name: "stdio", args: []string{"_stdio"}, wantKind: kindStdio}, | ||
| 36 | {name: "stdio with session", args: []string{"_stdio", "work"}, wantKind: kindStdio, wantName: "work"}, | 40 | {name: "stdio with session", args: []string{"_stdio", "work"}, wantKind: kindStdio, wantName: "work"}, | ||
M scripts/bench-workloads.sh +6 -4
| @@ -1,9 +1,11 @@ | |||
| 1 | 1 | #!/usr/bin/env bash | |
| 2 | 2 | set -euo pipefail | |
| 3 | 3 | ||
| 4 | - | # Demo workload driver for comparing vev and tmux manually. | |
| 5 | - | # Usage: TARGET=vev|tmux WORKLOAD=yes|seq|cat DURATION=5 ./scripts/bench-workloads.sh | |
| 6 | - | TARGET=${TARGET:-vev} | |
| 4 | + | # Demo workload driver for raw/tmux workload generation. | |
| 5 | + | # vev does not currently support `vev new <name> -- command`, so this script | |
| 6 | + | # intentionally does not advertise an automated vev workload path. | |
| 7 | + | # Usage: TARGET=raw|tmux WORKLOAD=yes|seq|cat DURATION=5 ./scripts/bench-workloads.sh | |
| 8 | + | TARGET=${TARGET:-raw} | |
| 7 | 9 | WORKLOAD=${WORKLOAD:-yes} | |
| 8 | 10 | DURATION=${DURATION:-5} | |
| 9 | 11 | ||
| @@ -17,7 +19,7 @@ esac | |||
| 17 | 19 | echo "target=$TARGET workload=$WORKLOAD duration=${DURATION}s" | |
| 18 | 20 | echo "command=$CMD" | |
| 19 | 21 | case "$TARGET" in | |
| 20 | - | vev) timeout "$DURATION" ./vev new bench -- sh -lc "$CMD" ;; | |
| 22 | + | vev) echo "TARGET=vev is unsupported until vev implements command overrides" >&2; exit 2 ;; | |
| 21 | 23 | tmux) timeout "$DURATION" tmux new-session -d -s vev-bench "$CMD" && sleep "$DURATION" && tmux kill-session -t vev-bench ;; | |
| 22 | 24 | raw) timeout "$DURATION" sh -lc "$CMD" ;; | |
| 23 | 25 | *) echo "unknown TARGET: $TARGET" >&2; exit 2 ;; | |
| @@ -1,9 +1,11 @@ | |||||
| 1 | #!/usr/bin/env bash | 1 | #!/usr/bin/env bash | ||
| 2 | set -euo pipefail | 2 | set -euo pipefail | ||
| 3 | 3 | ||||
| 4 | - | # Demo workload driver for comparing vev and tmux manually. | 4 | + | # Demo workload driver for raw/tmux workload generation. |
| 5 | - | # Usage: TARGET=vev|tmux WORKLOAD=yes|seq|cat DURATION=5 ./scripts/bench-workloads.sh | 5 | + | # vev does not currently support `vev new <name> -- command`, so this script |
| 6 | - | TARGET=${TARGET:-vev} | 6 | + | # intentionally does not advertise an automated vev workload path. |
| 7 | + | # Usage: TARGET=raw|tmux WORKLOAD=yes|seq|cat DURATION=5 ./scripts/bench-workloads.sh | |||
| 8 | + | TARGET=${TARGET:-raw} | |||
| 7 | WORKLOAD=${WORKLOAD:-yes} | 9 | WORKLOAD=${WORKLOAD:-yes} | ||
| 8 | DURATION=${DURATION:-5} | 10 | DURATION=${DURATION:-5} | ||
| 9 | 11 | ||||
| @@ -17,7 +19,7 @@ esac | |||||
| 17 | echo "target=$TARGET workload=$WORKLOAD duration=${DURATION}s" | 19 | echo "target=$TARGET workload=$WORKLOAD duration=${DURATION}s" | ||
| 18 | echo "command=$CMD" | 20 | echo "command=$CMD" | ||
| 19 | case "$TARGET" in | 21 | case "$TARGET" in | ||
| 20 | - | vev) timeout "$DURATION" ./vev new bench -- sh -lc "$CMD" ;; | 22 | + | vev) echo "TARGET=vev is unsupported until vev implements command overrides" >&2; exit 2 ;; |
| 21 | tmux) timeout "$DURATION" tmux new-session -d -s vev-bench "$CMD" && sleep "$DURATION" && tmux kill-session -t vev-bench ;; | 23 | tmux) timeout "$DURATION" tmux new-session -d -s vev-bench "$CMD" && sleep "$DURATION" && tmux kill-session -t vev-bench ;; | ||
| 22 | raw) timeout "$DURATION" sh -lc "$CMD" ;; | 24 | raw) timeout "$DURATION" sh -lc "$CMD" ;; | ||
| 23 | *) echo "unknown TARGET: $TARGET" >&2; exit 2 ;; | 25 | *) echo "unknown TARGET: $TARGET" >&2; exit 2 ;; | ||