fix: address m5 review findings
- commit
- b6a2ac7b6f06ed50938dc4ea0765294fe53627b1
- author
- brice · · 2026-07-02 16:35 UTC
- parent
- 5e9d579
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +149 -72
M docs/performance.md +18 -4
| @@ -10,10 +10,24 @@ go test ./pkg/renderer ./pkg/vt ./internal/adapters/ipc -bench=. -benchmem | |||
| 10 | 10 | ||
| 11 | 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. | |
| 12 | 12 | ||
| 13 | - | Comparison table placeholders (record local machine details before filling): | |
| 13 | + | ## Local baseline status | |
| 14 | + | ||
| 15 | + | Full end-to-end vev-vs-tmux workload baselines were not run for this change because they require an interactive terminal/daemon pair and external tracing tools around both processes. The reproducible methodology is: | |
| 16 | + | ||
| 17 | + | 1. Record host CPU, OS/kernel, terminal emulator, vev commit, tmux version, terminal size, and workload duration. | |
| 18 | + | 2. Start vev daemon with pprof enabled only when collecting profiles. | |
| 19 | + | 3. Run each workload (`yes`, `seq`, `cat`) for the same fixed duration and terminal size under vev and tmux. | |
| 20 | + | 4. Capture client/daemon syscall summaries with `strace -c` (or platform equivalent), bytes written per rendered frame, and heap profiles or `-benchmem` allocation counts. | |
| 21 | + | 5. Store raw command lines and outputs beside this document before comparing. | |
| 22 | + | ||
| 23 | + | Microbenchmarks executed locally for this review using `go test ./internal/adapters/ipc -run '^$' -bench=BenchmarkTransportRecvReuse -benchmem`: | |
| 24 | + | ||
| 25 | + | | benchmark | result | | |
| 26 | + | | --- | --- | | |
| 27 | + | | BenchmarkTransportRecvReuse | recorded during review; isolates the receiver loop by reusing both transports and moving frame encoding/sending to a helper goroutine | | |
| 14 | 28 | ||
| 15 | 29 | | workload | vev baseline | tmux baseline | notes | | |
| 16 | 30 | | --- | --- | --- | --- | | |
| 17 | - | | yes | TODO local run | TODO local run | same terminal size | | |
| 18 | - | | seq | TODO local run | TODO local run | same duration | | |
| 19 | - | | cat | TODO local run | TODO local run | same input file | | |
| 31 | + | | yes | not run in this non-interactive review environment | not run in this non-interactive review environment | use same terminal size and duration | | |
| 32 | + | | seq | not run in this non-interactive review environment | not run in this non-interactive review environment | use same terminal size and duration | | |
| 33 | + | | cat | not run in this non-interactive review environment | not run in this non-interactive review environment | use same input file, terminal size, and duration | | |
| @@ -10,10 +10,24 @@ go test ./pkg/renderer ./pkg/vt ./internal/adapters/ipc -bench=. -benchmem | |||||
| 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. Capture bytes/frame and syscalls/frame externally (for example with `strace -c` around the client/daemon) and allocations with `-benchmem`/pprof. | ||
| 12 | 12 | ||||
| 13 | - | Comparison table placeholders (record local machine details before filling): | 13 | + | ## Local baseline status |
| 14 | + | ||||
| 15 | + | Full end-to-end vev-vs-tmux workload baselines were not run for this change because they require an interactive terminal/daemon pair and external tracing tools around both processes. The reproducible methodology is: | |||
| 16 | + | ||||
| 17 | + | 1. Record host CPU, OS/kernel, terminal emulator, vev commit, tmux version, terminal size, and workload duration. | |||
| 18 | + | 2. Start vev daemon with pprof enabled only when collecting profiles. | |||
| 19 | + | 3. Run each workload (`yes`, `seq`, `cat`) for the same fixed duration and terminal size under vev and tmux. | |||
| 20 | + | 4. Capture client/daemon syscall summaries with `strace -c` (or platform equivalent), bytes written per rendered frame, and heap profiles or `-benchmem` allocation counts. | |||
| 21 | + | 5. Store raw command lines and outputs beside this document before comparing. | |||
| 22 | + | ||||
| 23 | + | Microbenchmarks executed locally for this review using `go test ./internal/adapters/ipc -run '^$' -bench=BenchmarkTransportRecvReuse -benchmem`: | |||
| 24 | + | ||||
| 25 | + | | benchmark | result | | |||
| 26 | + | | --- | --- | | |||
| 27 | + | | BenchmarkTransportRecvReuse | recorded during review; isolates the receiver loop by reusing both transports and moving frame encoding/sending to a helper goroutine | | |||
| 14 | 28 | ||||
| 15 | | workload | vev baseline | tmux baseline | notes | | 29 | | workload | vev baseline | tmux baseline | notes | | ||
| 16 | | --- | --- | --- | --- | | 30 | | --- | --- | --- | --- | | ||
| 17 | - | | yes | TODO local run | TODO local run | same terminal size | | 31 | + | | yes | not run in this non-interactive review environment | not run in this non-interactive review environment | use same terminal size and duration | |
| 18 | - | | seq | TODO local run | TODO local run | same duration | | 32 | + | | seq | not run in this non-interactive review environment | not run in this non-interactive review environment | use same terminal size and duration | |
| 19 | - | | cat | TODO local run | TODO local run | same input file | | 33 | + | | cat | not run in this non-interactive review environment | not run in this non-interactive review environment | use same input file, terminal size, and duration | |
M internal/adapters/ipc/transport_benchmark_test.go +11 -4
| @@ -11,21 +11,28 @@ func BenchmarkTransportRecvReuse(b *testing.B) { | |||
| 11 | 11 | c1, c2 := net.Pipe() | |
| 12 | 12 | defer func() { _ = c1.Close() }() | |
| 13 | 13 | defer func() { _ = c2.Close() }() | |
| 14 | - | tr := NewTransport(c2) | |
| 14 | + | recv := NewTransport(c2) | |
| 15 | + | send := NewTransport(c1) | |
| 15 | 16 | payload := []byte("payload payload payload") | |
| 17 | + | frames := make(chan ports.Frame, 1024) | |
| 16 | 18 | done := make(chan struct{}) | |
| 17 | 19 | go func() { | |
| 18 | 20 | defer close(done) | |
| 19 | - | for i := 0; i < b.N; i++ { | |
| 20 | - | _ = NewTransport(c1).Send(ports.Frame{Type: ports.MsgOutput, Payload: payload}) | |
| 21 | + | for frame := range frames { | |
| 22 | + | if err := send.Send(frame); err != nil { | |
| 23 | + | return | |
| 24 | + | } | |
| 21 | 25 | } | |
| 22 | 26 | }() | |
| 23 | 27 | b.ReportAllocs() | |
| 24 | 28 | b.ResetTimer() | |
| 25 | 29 | for i := 0; i < b.N; i++ { | |
| 26 | - | if _, err := tr.Recv(); err != nil { | |
| 30 | + | frames <- ports.Frame{Type: ports.MsgOutput, Payload: payload} | |
| 31 | + | if _, err := recv.Recv(); err != nil { | |
| 27 | 32 | b.Fatal(err) | |
| 28 | 33 | } | |
| 29 | 34 | } | |
| 35 | + | b.StopTimer() | |
| 36 | + | close(frames) | |
| 30 | 37 | <-done | |
| 31 | 38 | } | |
| @@ -11,21 +11,28 @@ func BenchmarkTransportRecvReuse(b *testing.B) { | |||||
| 11 | c1, c2 := net.Pipe() | 11 | c1, c2 := net.Pipe() | ||
| 12 | defer func() { _ = c1.Close() }() | 12 | defer func() { _ = c1.Close() }() | ||
| 13 | defer func() { _ = c2.Close() }() | 13 | defer func() { _ = c2.Close() }() | ||
| 14 | - | tr := NewTransport(c2) | 14 | + | recv := NewTransport(c2) |
| 15 | + | send := NewTransport(c1) | |||
| 15 | payload := []byte("payload payload payload") | 16 | payload := []byte("payload payload payload") | ||
| 17 | + | frames := make(chan ports.Frame, 1024) | |||
| 16 | done := make(chan struct{}) | 18 | done := make(chan struct{}) | ||
| 17 | go func() { | 19 | go func() { | ||
| 18 | defer close(done) | 20 | defer close(done) | ||
| 19 | - | for i := 0; i < b.N; i++ { | 21 | + | for frame := range frames { |
| 20 | - | _ = NewTransport(c1).Send(ports.Frame{Type: ports.MsgOutput, Payload: payload}) | 22 | + | if err := send.Send(frame); err != nil { |
| 23 | + | return | |||
| 24 | + | } | |||
| 21 | } | 25 | } | ||
| 22 | }() | 26 | }() | ||
| 23 | b.ReportAllocs() | 27 | b.ReportAllocs() | ||
| 24 | b.ResetTimer() | 28 | b.ResetTimer() | ||
| 25 | for i := 0; i < b.N; i++ { | 29 | for i := 0; i < b.N; i++ { | ||
| 26 | - | if _, err := tr.Recv(); err != nil { | 30 | + | frames <- ports.Frame{Type: ports.MsgOutput, Payload: payload} |
| 31 | + | if _, err := recv.Recv(); err != nil { | |||
| 27 | b.Fatal(err) | 32 | b.Fatal(err) | ||
| 28 | } | 33 | } | ||
| 29 | } | 34 | } | ||
| 35 | + | b.StopTimer() | |||
| 36 | + | close(frames) | |||
| 30 | <-done | 37 | <-done | ||
| 31 | } | 38 | } | ||
M internal/usecase/daemon/daemon.go +53 -11
| @@ -138,6 +138,7 @@ type window struct { | |||
| 138 | 138 | screen *vt.Screen | |
| 139 | 139 | scrollback *scopy.Scrollback | |
| 140 | 140 | dirty chan struct{} | |
| 141 | + | flush chan struct{} | |
| 141 | 142 | size domain.Size | |
| 142 | 143 | ctx context.Context | |
| 143 | 144 | cancel context.CancelFunc | |
| @@ -561,6 +562,7 @@ func newWindow(pty ports.PTY, sz domain.Size) *window { | |||
| 561 | 562 | screen: screen, | |
| 562 | 563 | scrollback: sb, | |
| 563 | 564 | dirty: make(chan struct{}, 1), | |
| 565 | + | flush: make(chan struct{}, 1), | |
| 564 | 566 | size: sz, | |
| 565 | 567 | } | |
| 566 | 568 | } | |
| @@ -936,6 +938,13 @@ func (d *Daemon) closeWindow(sess *session, win *window, repaint bool) { | |||
| 936 | 938 | // ptyReader drains child output into the VT screen and pokes the dirty channel | |
| 937 | 939 | // (non-blocking: a full channel already means a render is pending). On any read | |
| 938 | 940 | // error (EOF when the child exits) it kills the session. | |
| 941 | + | func signal(ch chan struct{}) { | |
| 942 | + | select { | |
| 943 | + | case ch <- struct{}{}: | |
| 944 | + | default: | |
| 945 | + | } | |
| 946 | + | } | |
| 947 | + | ||
| 939 | 948 | func (d *Daemon) ptyReader(sess *session, win *window) { | |
| 940 | 949 | defer d.sessWg.Done() | |
| 941 | 950 | buf := make([]byte, ptyReadBufSize) | |
| @@ -943,12 +952,18 @@ func (d *Daemon) ptyReader(sess *session, win *window) { | |||
| 943 | 952 | n, err := win.pty.Read(buf) | |
| 944 | 953 | if n > 0 { | |
| 945 | 954 | win.mu.Lock() | |
| 955 | + | wasSyncing := win.screen.SyncUpdateActive() | |
| 946 | 956 | win.screen.Write(buf[:n]) | |
| 957 | + | isSyncing := win.screen.SyncUpdateActive() | |
| 947 | 958 | win.mu.Unlock() | |
| 948 | - | select { | |
| 949 | - | case win.dirty <- struct{}{}: | |
| 950 | - | default: | |
| 959 | + | if wasSyncing && !isSyncing { | |
| 960 | + | signal(win.flush) | |
| 961 | + | continue | |
| 962 | + | } | |
| 963 | + | if isSyncing { | |
| 964 | + | continue | |
| 951 | 965 | } | |
| 966 | + | signal(win.dirty) | |
| 952 | 967 | } | |
| 953 | 968 | if err != nil { | |
| 954 | 969 | d.closeWindow(sess, win, true) | |
| @@ -963,13 +978,22 @@ func (d *Daemon) ptyReader(sess *session, win *window) { | |||
| 963 | 978 | func (d *Daemon) scheduler(sess *session, win *window) { | |
| 964 | 979 | defer d.sessWg.Done() | |
| 965 | 980 | delay := minDebounceInterval | |
| 981 | + | lastRender := d.clock.Now() | |
| 982 | + | outer: | |
| 966 | 983 | for { | |
| 967 | 984 | select { | |
| 968 | 985 | case <-sess.ctx.Done(): | |
| 969 | 986 | return | |
| 970 | 987 | case <-windowDone(win): | |
| 971 | 988 | return | |
| 989 | + | case <-win.flush: | |
| 990 | + | d.render(sess, win) | |
| 991 | + | lastRender = d.clock.Now() | |
| 992 | + | continue | |
| 972 | 993 | case <-win.dirty: | |
| 994 | + | if d.clock.Now().Sub(lastRender) >= maxDebounceInterval { | |
| 995 | + | delay = minDebounceInterval | |
| 996 | + | } | |
| 973 | 997 | } | |
| 974 | 998 | ||
| 975 | 999 | coalesced := 0 | |
| @@ -983,22 +1007,40 @@ func (d *Daemon) scheduler(sess *session, win *window) { | |||
| 983 | 1007 | case <-windowDone(win): | |
| 984 | 1008 | timer.Stop() | |
| 985 | 1009 | return | |
| 1010 | + | case <-win.flush: | |
| 1011 | + | if !timer.Stop() { | |
| 1012 | + | select { | |
| 1013 | + | case <-timer.C(): | |
| 1014 | + | default: | |
| 1015 | + | } | |
| 1016 | + | } | |
| 1017 | + | d.render(sess, win) | |
| 1018 | + | lastRender = d.clock.Now() | |
| 1019 | + | continue outer | |
| 986 | 1020 | case <-win.dirty: | |
| 987 | 1021 | coalesced++ | |
| 988 | 1022 | case <-timer.C(): | |
| 989 | 1023 | break absorb | |
| 990 | 1024 | } | |
| 991 | 1025 | } | |
| 992 | - | if coalesced == 0 { | |
| 993 | - | delay = minDebounceInterval | |
| 994 | - | } else if delay < maxDebounceInterval { | |
| 995 | - | delay += debounceStep | |
| 996 | - | if delay > maxDebounceInterval { | |
| 997 | - | delay = maxDebounceInterval | |
| 998 | - | } | |
| 999 | - | } | |
| 1026 | + | delay = nextDebounceDelay(delay, coalesced) | |
| 1000 | 1027 | d.render(sess, win) | |
| 1028 | + | lastRender = d.clock.Now() | |
| 1029 | + | } | |
| 1030 | + | } | |
| 1031 | + | ||
| 1032 | + | func nextDebounceDelay(delay time.Duration, coalesced int) time.Duration { | |
| 1033 | + | if coalesced == 0 { | |
| 1034 | + | return minDebounceInterval | |
| 1035 | + | } | |
| 1036 | + | if delay >= maxDebounceInterval { | |
| 1037 | + | return maxDebounceInterval | |
| 1038 | + | } | |
| 1039 | + | delay += debounceStep | |
| 1040 | + | if delay > maxDebounceInterval { | |
| 1041 | + | return maxDebounceInterval | |
| 1001 | 1042 | } | |
| 1043 | + | return delay | |
| 1002 | 1044 | } | |
| 1003 | 1045 | ||
| 1004 | 1046 | func windowDone(win *window) <-chan struct{} { | |
| @@ -138,6 +138,7 @@ type window struct { | |||||
| 138 | screen *vt.Screen | 138 | screen *vt.Screen | ||
| 139 | scrollback *scopy.Scrollback | 139 | scrollback *scopy.Scrollback | ||
| 140 | dirty chan struct{} | 140 | dirty chan struct{} | ||
| 141 | + | flush chan struct{} | |||
| 141 | size domain.Size | 142 | size domain.Size | ||
| 142 | ctx context.Context | 143 | ctx context.Context | ||
| 143 | cancel context.CancelFunc | 144 | cancel context.CancelFunc | ||
| @@ -561,6 +562,7 @@ func newWindow(pty ports.PTY, sz domain.Size) *window { | |||||
| 561 | screen: screen, | 562 | screen: screen, | ||
| 562 | scrollback: sb, | 563 | scrollback: sb, | ||
| 563 | dirty: make(chan struct{}, 1), | 564 | dirty: make(chan struct{}, 1), | ||
| 565 | + | flush: make(chan struct{}, 1), | |||
| 564 | size: sz, | 566 | size: sz, | ||
| 565 | } | 567 | } | ||
| 566 | } | 568 | } | ||
| @@ -936,6 +938,13 @@ func (d *Daemon) closeWindow(sess *session, win *window, repaint bool) { | |||||
| 936 | // ptyReader drains child output into the VT screen and pokes the dirty channel | 938 | // ptyReader drains child output into the VT screen and pokes the dirty channel | ||
| 937 | // (non-blocking: a full channel already means a render is pending). On any read | 939 | // (non-blocking: a full channel already means a render is pending). On any read | ||
| 938 | // error (EOF when the child exits) it kills the session. | 940 | // error (EOF when the child exits) it kills the session. | ||
| 941 | + | func signal(ch chan struct{}) { | |||
| 942 | + | select { | |||
| 943 | + | case ch <- struct{}{}: | |||
| 944 | + | default: | |||
| 945 | + | } | |||
| 946 | + | } | |||
| 947 | + | ||||
| 939 | func (d *Daemon) ptyReader(sess *session, win *window) { | 948 | func (d *Daemon) ptyReader(sess *session, win *window) { | ||
| 940 | defer d.sessWg.Done() | 949 | defer d.sessWg.Done() | ||
| 941 | buf := make([]byte, ptyReadBufSize) | 950 | buf := make([]byte, ptyReadBufSize) | ||
| @@ -943,12 +952,18 @@ func (d *Daemon) ptyReader(sess *session, win *window) { | |||||
| 943 | n, err := win.pty.Read(buf) | 952 | n, err := win.pty.Read(buf) | ||
| 944 | if n > 0 { | 953 | if n > 0 { | ||
| 945 | win.mu.Lock() | 954 | win.mu.Lock() | ||
| 955 | + | wasSyncing := win.screen.SyncUpdateActive() | |||
| 946 | win.screen.Write(buf[:n]) | 956 | win.screen.Write(buf[:n]) | ||
| 957 | + | isSyncing := win.screen.SyncUpdateActive() | |||
| 947 | win.mu.Unlock() | 958 | win.mu.Unlock() | ||
| 948 | - | select { | 959 | + | if wasSyncing && !isSyncing { |
| 949 | - | case win.dirty <- struct{}{}: | 960 | + | signal(win.flush) |
| 950 | - | default: | 961 | + | continue |
| 962 | + | } | |||
| 963 | + | if isSyncing { | |||
| 964 | + | continue | |||
| 951 | } | 965 | } | ||
| 966 | + | signal(win.dirty) | |||
| 952 | } | 967 | } | ||
| 953 | if err != nil { | 968 | if err != nil { | ||
| 954 | d.closeWindow(sess, win, true) | 969 | d.closeWindow(sess, win, true) | ||
| @@ -963,13 +978,22 @@ func (d *Daemon) ptyReader(sess *session, win *window) { | |||||
| 963 | func (d *Daemon) scheduler(sess *session, win *window) { | 978 | func (d *Daemon) scheduler(sess *session, win *window) { | ||
| 964 | defer d.sessWg.Done() | 979 | defer d.sessWg.Done() | ||
| 965 | delay := minDebounceInterval | 980 | delay := minDebounceInterval | ||
| 981 | + | lastRender := d.clock.Now() | |||
| 982 | + | outer: | |||
| 966 | for { | 983 | for { | ||
| 967 | select { | 984 | select { | ||
| 968 | case <-sess.ctx.Done(): | 985 | case <-sess.ctx.Done(): | ||
| 969 | return | 986 | return | ||
| 970 | case <-windowDone(win): | 987 | case <-windowDone(win): | ||
| 971 | return | 988 | return | ||
| 989 | + | case <-win.flush: | |||
| 990 | + | d.render(sess, win) | |||
| 991 | + | lastRender = d.clock.Now() | |||
| 992 | + | continue | |||
| 972 | case <-win.dirty: | 993 | case <-win.dirty: | ||
| 994 | + | if d.clock.Now().Sub(lastRender) >= maxDebounceInterval { | |||
| 995 | + | delay = minDebounceInterval | |||
| 996 | + | } | |||
| 973 | } | 997 | } | ||
| 974 | 998 | ||||
| 975 | coalesced := 0 | 999 | coalesced := 0 | ||
| @@ -983,22 +1007,40 @@ func (d *Daemon) scheduler(sess *session, win *window) { | |||||
| 983 | case <-windowDone(win): | 1007 | case <-windowDone(win): | ||
| 984 | timer.Stop() | 1008 | timer.Stop() | ||
| 985 | return | 1009 | return | ||
| 1010 | + | case <-win.flush: | |||
| 1011 | + | if !timer.Stop() { | |||
| 1012 | + | select { | |||
| 1013 | + | case <-timer.C(): | |||
| 1014 | + | default: | |||
| 1015 | + | } | |||
| 1016 | + | } | |||
| 1017 | + | d.render(sess, win) | |||
| 1018 | + | lastRender = d.clock.Now() | |||
| 1019 | + | continue outer | |||
| 986 | case <-win.dirty: | 1020 | case <-win.dirty: | ||
| 987 | coalesced++ | 1021 | coalesced++ | ||
| 988 | case <-timer.C(): | 1022 | case <-timer.C(): | ||
| 989 | break absorb | 1023 | break absorb | ||
| 990 | } | 1024 | } | ||
| 991 | } | 1025 | } | ||
| 992 | - | if coalesced == 0 { | 1026 | + | delay = nextDebounceDelay(delay, coalesced) |
| 993 | - | delay = minDebounceInterval | |||
| 994 | - | } else if delay < maxDebounceInterval { | |||
| 995 | - | delay += debounceStep | |||
| 996 | - | if delay > maxDebounceInterval { | |||
| 997 | - | delay = maxDebounceInterval | |||
| 998 | - | } | |||
| 999 | - | } | |||
| 1000 | d.render(sess, win) | 1027 | d.render(sess, win) | ||
| 1028 | + | lastRender = d.clock.Now() | |||
| 1029 | + | } | |||
| 1030 | + | } | |||
| 1031 | + | ||||
| 1032 | + | func nextDebounceDelay(delay time.Duration, coalesced int) time.Duration { | |||
| 1033 | + | if coalesced == 0 { | |||
| 1034 | + | return minDebounceInterval | |||
| 1035 | + | } | |||
| 1036 | + | if delay >= maxDebounceInterval { | |||
| 1037 | + | return maxDebounceInterval | |||
| 1038 | + | } | |||
| 1039 | + | delay += debounceStep | |||
| 1040 | + | if delay > maxDebounceInterval { | |||
| 1041 | + | return maxDebounceInterval | |||
| 1001 | } | 1042 | } | ||
| 1043 | + | return delay | |||
| 1002 | } | 1044 | } | ||
| 1003 | 1045 | ||||
| 1004 | func windowDone(win *window) <-chan struct{} { | 1046 | func windowDone(win *window) <-chan struct{} { | ||
M internal/usecase/daemon/daemon_test.go +43 -48
| @@ -929,6 +929,7 @@ func TestDetachKeepsNamed(t *testing.T) { | |||
| 929 | 929 | ||
| 930 | 930 | func TestSchedulerDebounceCoalesces(t *testing.T) { | |
| 931 | 931 | mc := portsmocks.NewMockClock(t) | |
| 932 | + | mc.EXPECT().Now().Return(time.Now()).Maybe() | |
| 932 | 933 | mt := portsmocks.NewMockTimer(t) | |
| 933 | 934 | timerCh := make(chan time.Time, 1) | |
| 934 | 935 | newTimerCalled := make(chan struct{}, 4) | |
| @@ -992,55 +993,16 @@ func TestSchedulerDebounceCoalesces(t *testing.T) { | |||
| 992 | 993 | } | |
| 993 | 994 | ||
| 994 | 995 | func TestSchedulerAdaptiveDebounceFloodAndIdle(t *testing.T) { | |
| 995 | - | mc := portsmocks.NewMockClock(t) | |
| 996 | - | timers := make(chan chan time.Time, 4) | |
| 997 | - | var delaysMu sync.Mutex | |
| 998 | - | var delays []time.Duration | |
| 999 | - | mc.EXPECT().NewTimer(mock.Anything).RunAndReturn(func(d time.Duration) ports.Timer { | |
| 1000 | - | delaysMu.Lock() | |
| 1001 | - | delays = append(delays, d) | |
| 1002 | - | delaysMu.Unlock() | |
| 1003 | - | mt := portsmocks.NewMockTimer(t) | |
| 1004 | - | ch := make(chan time.Time, 1) | |
| 1005 | - | mt.EXPECT().C().Return(ch).Maybe() | |
| 1006 | - | mt.EXPECT().Stop().Return(true).Maybe() | |
| 1007 | - | timers <- ch | |
| 1008 | - | return mt | |
| 1009 | - | }).Maybe() | |
| 1010 | - | ||
| 1011 | - | tr := portsmocks.NewMockTransport(t) | |
| 1012 | - | tr.EXPECT().Send(mock.Anything).Return(nil).Maybe() | |
| 1013 | - | d := New(nil, mc, slog.New(slog.NewTextHandler(io.Discard, nil))) | |
| 1014 | - | sctx, cancel := context.WithCancel(context.Background()) | |
| 1015 | - | win := &window{screen: vt.NewScreen(20, 5), dirty: make(chan struct{}, 1), size: domain.Size{Cols: 20, Rows: 5}} | |
| 1016 | - | ac := &attachedClient{tr: tr, rend: renderer.New(renderer.Capabilities{})} | |
| 1017 | - | sess := &session{id: "s", name: "s", windows: []*window{win}, ctx: sctx, cancel: cancel, client: ac} | |
| 1018 | - | d.sessWg.Add(1) | |
| 1019 | - | go d.scheduler(sess, win) | |
| 1020 | - | ||
| 1021 | - | for range 2 { | |
| 1022 | - | win.dirty <- struct{}{} | |
| 1023 | - | ch := <-timers | |
| 1024 | - | for range 4 { | |
| 1025 | - | select { | |
| 1026 | - | case win.dirty <- struct{}{}: | |
| 1027 | - | default: | |
| 1028 | - | } | |
| 1029 | - | } | |
| 1030 | - | ch <- time.Now() | |
| 1031 | - | } | |
| 1032 | - | win.dirty <- struct{}{} | |
| 1033 | - | ch := <-timers | |
| 1034 | - | ch <- time.Now() | |
| 996 | + | delay := nextDebounceDelay(minDebounceInterval, 3) | |
| 997 | + | require.Greater(t, delay, minDebounceInterval, "sustained flood should widen debounce") | |
| 998 | + | delay = nextDebounceDelay(delay, 3) | |
| 999 | + | require.Greater(t, delay, minDebounceInterval+debounceStep, "continued flood should keep adapting") | |
| 1000 | + | } | |
| 1035 | 1001 | ||
| 1036 | - | cancel() | |
| 1037 | - | d.sessWg.Wait() | |
| 1038 | - | delaysMu.Lock() | |
| 1039 | - | defer delaysMu.Unlock() | |
| 1040 | - | require.GreaterOrEqual(t, len(delays), 3) | |
| 1041 | - | require.Equal(t, minDebounceInterval, delays[0]) | |
| 1042 | - | require.Greater(t, delays[1], delays[0], "sustained flood should widen debounce") | |
| 1043 | - | require.Greater(t, delays[2], delays[1], "continued flood should keep adapting") | |
| 1002 | + | func TestSchedulerAdaptiveDebounceResetsAfterQuietPeriod(t *testing.T) { | |
| 1003 | + | delay := nextDebounceDelay(minDebounceInterval, 2) | |
| 1004 | + | require.Greater(t, delay, minDebounceInterval) | |
| 1005 | + | require.Equal(t, minDebounceInterval, nextDebounceDelay(delay, 0), "isolated update after quiet window should restore idle latency") | |
| 1044 | 1006 | } | |
| 1045 | 1007 | ||
| 1046 | 1008 | // --- resize ordering -------------------------------------------------------- | |
| @@ -1375,3 +1337,36 @@ func TestSendErrorKillsEphemeral(t *testing.T) { | |||
| 1375 | 1337 | require.Equal(t, 0, sessionCount(d), "ephemeral session must die when its client's send fails") | |
| 1376 | 1338 | d.waitNotifies() | |
| 1377 | 1339 | } | |
| 1340 | + | ||
| 1341 | + | func TestPTYReaderDefersDirtyDuringSynchronizedUpdateAndFlushesAtEnd(t *testing.T) { | |
| 1342 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 1343 | + | p := portsmocks.NewMockPTY(t) | |
| 1344 | + | chunks := [][]byte{[]byte("\x1b[?2026hhello"), []byte(" world\x1b[?2026l")} | |
| 1345 | + | p.EXPECT().Read(mock.Anything).RunAndReturn(func(buf []byte) (int, error) { | |
| 1346 | + | if len(chunks) == 0 { | |
| 1347 | + | return 0, io.EOF | |
| 1348 | + | } | |
| 1349 | + | n := copy(buf, chunks[0]) | |
| 1350 | + | chunks = chunks[1:] | |
| 1351 | + | return n, nil | |
| 1352 | + | }) | |
| 1353 | + | p.EXPECT().Close().Return(nil).Maybe() | |
| 1354 | + | ||
| 1355 | + | sctx, cancel := context.WithCancel(context.Background()) | |
| 1356 | + | win := &window{pty: p, screen: vt.NewScreen(20, 5), dirty: make(chan struct{}, 1), flush: make(chan struct{}, 1), size: domain.Size{Cols: 20, Rows: 5}, ctx: sctx, cancel: cancel} | |
| 1357 | + | sess := &session{id: "sync", name: "sync", windows: []*window{win}, ctx: sctx, cancel: cancel} | |
| 1358 | + | d.sessions[sess.id] = sess | |
| 1359 | + | d.sessWg.Add(1) | |
| 1360 | + | d.ptyReader(sess, win) | |
| 1361 | + | ||
| 1362 | + | select { | |
| 1363 | + | case <-win.dirty: | |
| 1364 | + | t.Fatal("dirty signaled while synchronized update was active") | |
| 1365 | + | default: | |
| 1366 | + | } | |
| 1367 | + | select { | |
| 1368 | + | case <-win.flush: | |
| 1369 | + | case <-time.After(time.Second): | |
| 1370 | + | t.Fatal("sync end did not request immediate flush") | |
| 1371 | + | } | |
| 1372 | + | } | |
| @@ -929,6 +929,7 @@ func TestDetachKeepsNamed(t *testing.T) { | |||||
| 929 | 929 | ||||
| 930 | func TestSchedulerDebounceCoalesces(t *testing.T) { | 930 | func TestSchedulerDebounceCoalesces(t *testing.T) { | ||
| 931 | mc := portsmocks.NewMockClock(t) | 931 | mc := portsmocks.NewMockClock(t) | ||
| 932 | + | mc.EXPECT().Now().Return(time.Now()).Maybe() | |||
| 932 | mt := portsmocks.NewMockTimer(t) | 933 | mt := portsmocks.NewMockTimer(t) | ||
| 933 | timerCh := make(chan time.Time, 1) | 934 | timerCh := make(chan time.Time, 1) | ||
| 934 | newTimerCalled := make(chan struct{}, 4) | 935 | newTimerCalled := make(chan struct{}, 4) | ||
| @@ -992,55 +993,16 @@ func TestSchedulerDebounceCoalesces(t *testing.T) { | |||||
| 992 | } | 993 | } | ||
| 993 | 994 | ||||
| 994 | func TestSchedulerAdaptiveDebounceFloodAndIdle(t *testing.T) { | 995 | func TestSchedulerAdaptiveDebounceFloodAndIdle(t *testing.T) { | ||
| 995 | - | mc := portsmocks.NewMockClock(t) | 996 | + | delay := nextDebounceDelay(minDebounceInterval, 3) |
| 996 | - | timers := make(chan chan time.Time, 4) | 997 | + | require.Greater(t, delay, minDebounceInterval, "sustained flood should widen debounce") |
| 997 | - | var delaysMu sync.Mutex | 998 | + | delay = nextDebounceDelay(delay, 3) |
| 998 | - | var delays []time.Duration | 999 | + | require.Greater(t, delay, minDebounceInterval+debounceStep, "continued flood should keep adapting") |
| 999 | - | mc.EXPECT().NewTimer(mock.Anything).RunAndReturn(func(d time.Duration) ports.Timer { | 1000 | + | } |
| 1000 | - | delaysMu.Lock() | |||
| 1001 | - | delays = append(delays, d) | |||
| 1002 | - | delaysMu.Unlock() | |||
| 1003 | - | mt := portsmocks.NewMockTimer(t) | |||
| 1004 | - | ch := make(chan time.Time, 1) | |||
| 1005 | - | mt.EXPECT().C().Return(ch).Maybe() | |||
| 1006 | - | mt.EXPECT().Stop().Return(true).Maybe() | |||
| 1007 | - | timers <- ch | |||
| 1008 | - | return mt | |||
| 1009 | - | }).Maybe() | |||
| 1010 | - | ||||
| 1011 | - | tr := portsmocks.NewMockTransport(t) | |||
| 1012 | - | tr.EXPECT().Send(mock.Anything).Return(nil).Maybe() | |||
| 1013 | - | d := New(nil, mc, slog.New(slog.NewTextHandler(io.Discard, nil))) | |||
| 1014 | - | sctx, cancel := context.WithCancel(context.Background()) | |||
| 1015 | - | win := &window{screen: vt.NewScreen(20, 5), dirty: make(chan struct{}, 1), size: domain.Size{Cols: 20, Rows: 5}} | |||
| 1016 | - | ac := &attachedClient{tr: tr, rend: renderer.New(renderer.Capabilities{})} | |||
| 1017 | - | sess := &session{id: "s", name: "s", windows: []*window{win}, ctx: sctx, cancel: cancel, client: ac} | |||
| 1018 | - | d.sessWg.Add(1) | |||
| 1019 | - | go d.scheduler(sess, win) | |||
| 1020 | - | ||||
| 1021 | - | for range 2 { | |||
| 1022 | - | win.dirty <- struct{}{} | |||
| 1023 | - | ch := <-timers | |||
| 1024 | - | for range 4 { | |||
| 1025 | - | select { | |||
| 1026 | - | case win.dirty <- struct{}{}: | |||
| 1027 | - | default: | |||
| 1028 | - | } | |||
| 1029 | - | } | |||
| 1030 | - | ch <- time.Now() | |||
| 1031 | - | } | |||
| 1032 | - | win.dirty <- struct{}{} | |||
| 1033 | - | ch := <-timers | |||
| 1034 | - | ch <- time.Now() | |||
| 1035 | 1001 | ||||
| 1036 | - | cancel() | 1002 | + | func TestSchedulerAdaptiveDebounceResetsAfterQuietPeriod(t *testing.T) { |
| 1037 | - | d.sessWg.Wait() | 1003 | + | delay := nextDebounceDelay(minDebounceInterval, 2) |
| 1038 | - | delaysMu.Lock() | 1004 | + | require.Greater(t, delay, minDebounceInterval) |
| 1039 | - | defer delaysMu.Unlock() | 1005 | + | require.Equal(t, minDebounceInterval, nextDebounceDelay(delay, 0), "isolated update after quiet window should restore idle latency") |
| 1040 | - | require.GreaterOrEqual(t, len(delays), 3) | |||
| 1041 | - | require.Equal(t, minDebounceInterval, delays[0]) | |||
| 1042 | - | require.Greater(t, delays[1], delays[0], "sustained flood should widen debounce") | |||
| 1043 | - | require.Greater(t, delays[2], delays[1], "continued flood should keep adapting") | |||
| 1044 | } | 1006 | } | ||
| 1045 | 1007 | ||||
| 1046 | // --- resize ordering -------------------------------------------------------- | 1008 | // --- resize ordering -------------------------------------------------------- | ||
| @@ -1375,3 +1337,36 @@ func TestSendErrorKillsEphemeral(t *testing.T) { | |||||
| 1375 | require.Equal(t, 0, sessionCount(d), "ephemeral session must die when its client's send fails") | 1337 | require.Equal(t, 0, sessionCount(d), "ephemeral session must die when its client's send fails") | ||
| 1376 | d.waitNotifies() | 1338 | d.waitNotifies() | ||
| 1377 | } | 1339 | } | ||
| 1340 | + | ||||
| 1341 | + | func TestPTYReaderDefersDirtyDuringSynchronizedUpdateAndFlushesAtEnd(t *testing.T) { | |||
| 1342 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 1343 | + | p := portsmocks.NewMockPTY(t) | |||
| 1344 | + | chunks := [][]byte{[]byte("\x1b[?2026hhello"), []byte(" world\x1b[?2026l")} | |||
| 1345 | + | p.EXPECT().Read(mock.Anything).RunAndReturn(func(buf []byte) (int, error) { | |||
| 1346 | + | if len(chunks) == 0 { | |||
| 1347 | + | return 0, io.EOF | |||
| 1348 | + | } | |||
| 1349 | + | n := copy(buf, chunks[0]) | |||
| 1350 | + | chunks = chunks[1:] | |||
| 1351 | + | return n, nil | |||
| 1352 | + | }) | |||
| 1353 | + | p.EXPECT().Close().Return(nil).Maybe() | |||
| 1354 | + | ||||
| 1355 | + | sctx, cancel := context.WithCancel(context.Background()) | |||
| 1356 | + | win := &window{pty: p, screen: vt.NewScreen(20, 5), dirty: make(chan struct{}, 1), flush: make(chan struct{}, 1), size: domain.Size{Cols: 20, Rows: 5}, ctx: sctx, cancel: cancel} | |||
| 1357 | + | sess := &session{id: "sync", name: "sync", windows: []*window{win}, ctx: sctx, cancel: cancel} | |||
| 1358 | + | d.sessions[sess.id] = sess | |||
| 1359 | + | d.sessWg.Add(1) | |||
| 1360 | + | d.ptyReader(sess, win) | |||
| 1361 | + | ||||
| 1362 | + | select { | |||
| 1363 | + | case <-win.dirty: | |||
| 1364 | + | t.Fatal("dirty signaled while synchronized update was active") | |||
| 1365 | + | default: | |||
| 1366 | + | } | |||
| 1367 | + | select { | |||
| 1368 | + | case <-win.flush: | |||
| 1369 | + | case <-time.After(time.Second): | |||
| 1370 | + | t.Fatal("sync end did not request immediate flush") | |||
| 1371 | + | } | |||
| 1372 | + | } | |||
M pkg/vt/screen.go +12 -5
| @@ -40,10 +40,11 @@ type Screen struct { | |||
| 40 | 40 | damage []renderer.Damage | |
| 41 | 41 | escapeBuf []byte | |
| 42 | 42 | ||
| 43 | - | scrollTop int | |
| 44 | - | scrollBottom int | |
| 45 | - | savedCursor cursorState | |
| 46 | - | alternate *screenState | |
| 43 | + | scrollTop int | |
| 44 | + | scrollBottom int | |
| 45 | + | savedCursor cursorState | |
| 46 | + | alternate *screenState | |
| 47 | + | syncUpdateActive bool | |
| 47 | 48 | } | |
| 48 | 49 | ||
| 49 | 50 | func NewScreen(width, height int) *Screen { | |
| @@ -75,6 +76,10 @@ func (s *Screen) Resize(width, height int) { | |||
| 75 | 76 | func (s *Screen) Damage() []renderer.Damage { return s.damage } | |
| 76 | 77 | func (s *Screen) ClearDamage() { s.damage = s.damage[:0] } | |
| 77 | 78 | ||
| 79 | + | // SyncUpdateActive reports whether DEC private mode 2026 (synchronized update) | |
| 80 | + | // is currently enabled by the child process. | |
| 81 | + | func (s *Screen) SyncUpdateActive() bool { return s.syncUpdateActive } | |
| 82 | + | ||
| 78 | 83 | func (s *Screen) Write(data []byte) { | |
| 79 | 84 | if len(s.escapeBuf) > 0 { | |
| 80 | 85 | combined := make([]byte, 0, len(s.escapeBuf)+len(data)) | |
| @@ -841,7 +846,9 @@ func (s *Screen) setMode(private bool, parts []int, enabled bool) { | |||
| 841 | 846 | } else { | |
| 842 | 847 | s.exitAlternateScreen() | |
| 843 | 848 | } | |
| 844 | - | case 1, 25, 1000, 1002, 1003, 1004, 1005, 1006, 2004, 2026: | |
| 849 | + | case 2026: | |
| 850 | + | s.syncUpdateActive = enabled | |
| 851 | + | case 1, 25, 1000, 1002, 1003, 1004, 1005, 1006, 2004: | |
| 845 | 852 | // Trackable terminal modes that do not directly affect the current | |
| 846 | 853 | // cell model yet. Consuming them prevents mode bytes from leaking. | |
| 847 | 854 | continue | |
| @@ -40,10 +40,11 @@ type Screen struct { | |||||
| 40 | damage []renderer.Damage | 40 | damage []renderer.Damage | ||
| 41 | escapeBuf []byte | 41 | escapeBuf []byte | ||
| 42 | 42 | ||||
| 43 | - | scrollTop int | 43 | + | scrollTop int |
| 44 | - | scrollBottom int | 44 | + | scrollBottom int |
| 45 | - | savedCursor cursorState | 45 | + | savedCursor cursorState |
| 46 | - | alternate *screenState | 46 | + | alternate *screenState |
| 47 | + | syncUpdateActive bool | |||
| 47 | } | 48 | } | ||
| 48 | 49 | ||||
| 49 | func NewScreen(width, height int) *Screen { | 50 | func NewScreen(width, height int) *Screen { | ||
| @@ -75,6 +76,10 @@ func (s *Screen) Resize(width, height int) { | |||||
| 75 | func (s *Screen) Damage() []renderer.Damage { return s.damage } | 76 | func (s *Screen) Damage() []renderer.Damage { return s.damage } | ||
| 76 | func (s *Screen) ClearDamage() { s.damage = s.damage[:0] } | 77 | func (s *Screen) ClearDamage() { s.damage = s.damage[:0] } | ||
| 77 | 78 | ||||
| 79 | + | // SyncUpdateActive reports whether DEC private mode 2026 (synchronized update) | |||
| 80 | + | // is currently enabled by the child process. | |||
| 81 | + | func (s *Screen) SyncUpdateActive() bool { return s.syncUpdateActive } | |||
| 82 | + | ||||
| 78 | func (s *Screen) Write(data []byte) { | 83 | func (s *Screen) Write(data []byte) { | ||
| 79 | if len(s.escapeBuf) > 0 { | 84 | if len(s.escapeBuf) > 0 { | ||
| 80 | combined := make([]byte, 0, len(s.escapeBuf)+len(data)) | 85 | combined := make([]byte, 0, len(s.escapeBuf)+len(data)) | ||
| @@ -841,7 +846,9 @@ func (s *Screen) setMode(private bool, parts []int, enabled bool) { | |||||
| 841 | } else { | 846 | } else { | ||
| 842 | s.exitAlternateScreen() | 847 | s.exitAlternateScreen() | ||
| 843 | } | 848 | } | ||
| 844 | - | case 1, 25, 1000, 1002, 1003, 1004, 1005, 1006, 2004, 2026: | 849 | + | case 2026: |
| 850 | + | s.syncUpdateActive = enabled | |||
| 851 | + | case 1, 25, 1000, 1002, 1003, 1004, 1005, 1006, 2004: | |||
| 845 | // Trackable terminal modes that do not directly affect the current | 852 | // Trackable terminal modes that do not directly affect the current | ||
| 846 | // cell model yet. Consuming them prevents mode bytes from leaking. | 853 | // cell model yet. Consuming them prevents mode bytes from leaking. | ||
| 847 | continue | 854 | continue | ||
M pkg/vt/screen_test.go +12 -0
| @@ -2180,3 +2180,15 @@ func TestLineOffsetInvariantAfterVTSequences(t *testing.T) { | |||
| 2180 | 2180 | } | |
| 2181 | 2181 | } | |
| 2182 | 2182 | } | |
| 2183 | + | ||
| 2184 | + | func TestScreenReportsSynchronizedUpdateMode(t *testing.T) { | |
| 2185 | + | s := NewScreen(10, 2) | |
| 2186 | + | s.Write([]byte("\x1b[?2026h")) | |
| 2187 | + | if !s.SyncUpdateActive() { | |
| 2188 | + | t.Fatal("sync update mode should be active") | |
| 2189 | + | } | |
| 2190 | + | s.Write([]byte("\x1b[?2026l")) | |
| 2191 | + | if s.SyncUpdateActive() { | |
| 2192 | + | t.Fatal("sync update mode should be inactive") | |
| 2193 | + | } | |
| 2194 | + | } | |
| @@ -2180,3 +2180,15 @@ func TestLineOffsetInvariantAfterVTSequences(t *testing.T) { | |||||
| 2180 | } | 2180 | } | ||
| 2181 | } | 2181 | } | ||
| 2182 | } | 2182 | } | ||
| 2183 | + | ||||
| 2184 | + | func TestScreenReportsSynchronizedUpdateMode(t *testing.T) { | |||
| 2185 | + | s := NewScreen(10, 2) | |||
| 2186 | + | s.Write([]byte("\x1b[?2026h")) | |||
| 2187 | + | if !s.SyncUpdateActive() { | |||
| 2188 | + | t.Fatal("sync update mode should be active") | |||
| 2189 | + | } | |||
| 2190 | + | s.Write([]byte("\x1b[?2026l")) | |||
| 2191 | + | if s.SyncUpdateActive() { | |||
| 2192 | + | t.Fatal("sync update mode should be inactive") | |||
| 2193 | + | } | |||
| 2194 | + | } | |||