diff --git a/docs/changes/unreleased/2466-idle-app-saving-helper-exits.md b/docs/changes/unreleased/2466-idle-app-saving-helper-exits.md new file mode 100644 index 0000000000..a3be2c2bcb --- /dev/null +++ b/docs/changes/unreleased/2466-idle-app-saving-helper-exits.md @@ -0,0 +1,13 @@ +--- +kind: changed +title: An idle desktop app lets its saving helper exit instead of keeping it awake +pr: 2466 +surface: [app, engine] +invalidates: + - "The desktop engine sent its saving helpers a health request every 20 seconds for as long as it ran, so a quiet app kept the same helper alive. It now sends them only while a reply is running, a save is held or running, or a seal awaits its engine sync; an idle app's helper exits after about one minute and the next save starts it again." +--- + +A kept-alive `furrow serve` woke about 28 times a second on an otherwise idle +app. Starting it again for the next save takes about 4 ms on an Apple silicon +Mac, plus the first open of the store. Touches still never start or restart a +helper, and terminal and command-line helper lifetimes are unchanged. diff --git a/internal/cellstore/daemon_app.go b/internal/cellstore/daemon_app.go index 8a087d2e86..1d42a6a6d9 100644 --- a/internal/cellstore/daemon_app.go +++ b/internal/cellstore/daemon_app.go @@ -14,8 +14,11 @@ const appDaemonTouchEvery = 20 * time.Second // SetAppDaemonLifetime selects a one-minute idle limit for helpers this process // starts. Call it once, before starting helpers. Existing helpers keep their -// limit. Until ctx ends, health requests keep this process's own helpers warm; -// neither cancellation nor process exit sends a shutdown request. +// limit. Until ctx ends, health requests keep this process's own helpers warm +// while it has saving work (appDaemonsWanted). An idle app sends none, so its +// helpers take their own idle exit and stop waking; the next save starts one +// again (Daemon.connect). Neither cancellation nor process exit sends a +// shutdown request. func SetAppDaemonLifetime(ctx context.Context) { if appDaemonLifetime.Swap(true) { return @@ -28,12 +31,59 @@ func SetAppDaemonLifetime(ctx context.Context) { case <-ctx.Done(): return case <-ticker.C: - touchAppDaemons(ctx) + touchBusyAppDaemons(ctx) } } }) } +// touchBusyAppDaemons touches this process's helpers only while it has saving +// work. An idle app sends nothing. +func touchBusyAppDaemons(ctx context.Context) { + if appDaemonsWanted() { + touchAppDaemons(ctx) + } +} + +// appDaemonsWanted reports saving work a warm helper serves: a chat turn is +// running, a capture is held or running, or a seal still owes its durability +// fence. Anything else is an idle app. Its helper exits after the idle limit, +// which costs the next save a cold start and never the save: Daemon.Do starts +// a missing helper, and the WAL keeps every call until its seal holds. +func appDaemonsWanted() bool { + return liveTurns.Load() > 0 || capturesOwed() || engineDebtOwed() +} + +// capturesOwed reports held calls or a capture still running. A finished +// capture that failed stays registered for its error; it is not work. +func capturesOwed() bool { + captures.Lock() + defer captures.Unlock() + if len(captures.held) > 0 { + return true + } + for _, state := range captures.cells { + select { + case <-state.done: + default: + return true + } + } + return false +} + +// engineDebtOwed reports a seal whose engine sync has not yet confirmed it. +func engineDebtOwed() bool { + engineCommits.Lock() + defer engineCommits.Unlock() + for _, debt := range engineCommits.cells { + if debt.generation != 0 { + return true + } + } + return false +} + func touchAppDaemons(ctx context.Context) { ctx, cancel := context.WithTimeout(ctx, healthWait) defer cancel() diff --git a/internal/cellstore/daemon_app_lifetime_test.go b/internal/cellstore/daemon_app_lifetime_test.go index f8e6201e1e..c828f768a4 100644 --- a/internal/cellstore/daemon_app_lifetime_test.go +++ b/internal/cellstore/daemon_app_lifetime_test.go @@ -132,9 +132,12 @@ func TestDesktopSavingHelperLifetime(t *testing.T) { client := startIdleClient(t, socket, binary, true) pid := client.command(t, "health") assertIdleFlag(t, pid, "1") + if got := client.command(t, "turn"); got != "turn" { + t.Fatal(got) + } time.Sleep(70 * time.Second) if got := client.command(t, "health"); got != pid { - t.Fatalf("quiet app restarted helper: pid %s became %s", pid, got) + t.Fatalf("app with a running turn restarted helper: pid %s became %s", pid, got) } if killed { if err := client.cmd.Process.Kill(); err != nil { @@ -144,28 +147,56 @@ func TestDesktopSavingHelperLifetime(t *testing.T) { t.Fatal(got) } stopped := time.Now() - deadline := stopped.Add(75 * time.Second) - for { - args, err := os.ReadFile("/proc/" + pid + "/cmdline") - if os.IsNotExist(err) || (err == nil && len(args) == 0) { - break - } - if err != nil { - t.Fatal(err) - } - if time.Now().After(deadline) { - t.Fatal("helper outlived touching by 75 seconds without Stop") - } - time.Sleep(time.Second) - } - if time.Since(stopped) < 45*time.Second { - t.Fatal("helper closed before its idle limit") - } - t.Logf("same pid %s survived quiet app; idle exit after %.1fs without Stop", pid, time.Since(stopped).Seconds()) + awaitHelperExit(t, pid, stopped) + t.Logf("same pid %s survived a running turn; idle exit after %.1fs without Stop", pid, time.Since(stopped).Seconds()) }) } } +// An app with no saving work stops touching its helper, so the helper exits +// on its own idle limit; the next request starts a new one. +func TestDesktopIdleAppLetsHelperExit(t *testing.T) { + if testing.Short() { + t.Skip("real one-minute idle exit") + } + binary := pinnedIdleBinary(t) + socket := idleSocket(t) + client := startIdleClient(t, socket, binary, true) + pid := client.command(t, "health") + assertIdleFlag(t, pid, "1") + quiet := time.Now() + awaitHelperExit(t, pid, quiet) + t.Logf("idle app's helper %s exited after %.1fs", pid, time.Since(quiet).Seconds()) + next := client.command(t, "health") + if next == pid { + t.Fatalf("helper %s answered after it exited", pid) + } + assertIdleFlag(t, next, "1") +} + +// awaitHelperExit waits for pid to exit by its own one-minute idle limit, +// measured from the last request at from. +func awaitHelperExit(t *testing.T, pid string, from time.Time) { + t.Helper() + deadline := from.Add(75 * time.Second) + for { + args, err := os.ReadFile("/proc/" + pid + "/cmdline") + if os.IsNotExist(err) || (err == nil && len(args) == 0) { + break + } + if err != nil { + t.Fatal(err) + } + if time.Now().After(deadline) { + t.Fatal("helper outlived touching by 75 seconds without Stop") + } + time.Sleep(time.Second) + } + if time.Since(from) < 45*time.Second { + t.Fatal("helper closed before its idle limit") + } +} + func TestDesktopTouchNeverStartsHelper(t *testing.T) { if testing.Short() { t.Skip("real twenty-second touch interval") @@ -173,6 +204,11 @@ func TestDesktopTouchNeverStartsHelper(t *testing.T) { binary := pinnedIdleBinary(t) socket := idleSocket(t) client := startIdleClient(t, socket, binary, true) + // A running turn makes every tick touch, so only dial-only touching keeps + // the absent and the killed helper from starting. + if got := client.command(t, "turn"); got != "turn" { + t.Fatal(got) + } time.Sleep(25 * time.Second) if _, err := os.Stat(socket); !os.IsNotExist(err) { t.Fatalf("touch started absent helper: %v", err) diff --git a/internal/cellstore/daemon_app_test.go b/internal/cellstore/daemon_app_test.go new file mode 100644 index 0000000000..b5a6c748eb --- /dev/null +++ b/internal/cellstore/daemon_app_test.go @@ -0,0 +1,144 @@ +package cellstore + +import ( + "bufio" + "context" + "encoding/json" + "errors" + "net" + "testing" +) + +// An idle desktop app must leave its saving helper alone so the helper reaches +// its own idle exit and stops waking. Saving work in flight keeps it warm. +func TestAppTouchesHelpersOnlyForSavingWork(t *testing.T) { + isolateSavingWork(t) + socket := testSocket(t) + healths := healthRequests(t, socket) + ownerExited := make(chan struct{}) + owner := &spawnedDaemon{socket: socket, exited: ownerExited} + daemonOwners.Store(socket, owner) + t.Cleanup(func() { daemonOwners.CompareAndDelete(socket, owner) }) + + // One tick's work runs here, synchronously: the helper counts a health + // before answering it, so a touch that returned has already been counted. + tick := func() int { + touchBusyAppDaemons(context.Background()) + n := len(healths) + for len(healths) > 0 { + <-healths + } + return n + } + quiet := func(when string) { + t.Helper() + if n := tick(); n != 0 { + t.Fatalf("%s: an idle app touched its saving helper %d times", when, n) + } + } + touched := func(when string) { + t.Helper() + if n := tick(); n != 1 { + t.Fatalf("%s: saving work touched the helper %d times, want 1", when, n) + } + } + + quiet("before any chat") + var chat SealWatch + chat.Waiting() + quiet("a chat waiting for input") + chat.TurnStarted() + chat.TurnStarted() + touched("a running turn") + chat.TurnEnded() + quiet("after the turn ended") + chat.TurnEnded() + if n := liveTurns.Load(); n != 0 { + t.Fatalf("repeated turn words left %d live turns", n) + } + + root := t.TempDir() + captures.Lock() + captures.held[root] = &Recorder{} + captures.Unlock() + touched("held calls") + captures.Lock() + delete(captures.held, root) + running := &captureState{done: make(chan struct{})} + captures.cells[root] = running + captures.Unlock() + touched("a running capture") + captures.Lock() + running.err = errors.New("seal failed") + close(running.done) + captures.Unlock() + quiet("a finished capture kept only for its error") + + engineCommits.Lock() + engineCommits.cells[root] = engineDebt{root: root, generation: 1} + engineCommits.Unlock() + touched("a seal owing its engine sync") + engineCommits.Lock() + engineCommits.cells[root] = engineDebt{root: root} + engineCommits.Unlock() + quiet("a synced seal") + + close(ownerExited) + chat.TurnStarted() + defer chat.TurnEnded() + quiet("a helper that already exited") +} + +// isolateSavingWork gives the test empty registries of saving work and returns +// what earlier tests left when it ends, so only this test's work is read. +func isolateSavingWork(t *testing.T) { + t.Helper() + turns := liveTurns.Swap(0) + captures.Lock() + held, cells := captures.held, captures.cells + captures.held, captures.cells = map[string]*Recorder{}, map[string]*captureState{} + captures.Unlock() + engineCommits.Lock() + debts := engineCommits.cells + engineCommits.cells = map[string]engineDebt{} + engineCommits.Unlock() + t.Cleanup(func() { + liveTurns.Add(turns) + captures.Lock() + captures.held, captures.cells = held, cells + captures.Unlock() + engineCommits.Lock() + engineCommits.cells = debts + engineCommits.Unlock() + }) +} + +// healthRequests serves socket as a helper and signals each health request +// before answering it, so a finished touch has always been counted. +func healthRequests(t *testing.T, socket string) <-chan struct{} { + t.Helper() + ln, err := net.Listen("unix", socket) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { ln.Close() }) + got := make(chan struct{}, 16) + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + var req wireRequest + line, _ := bufio.NewReader(conn).ReadBytes('\n') + _ = json.Unmarshal(line, &req) + if req.Verb == "health" { + got <- struct{}{} + } + reply, _ := json.Marshal(wireResponse{ID: req.ID, Ok: json.RawMessage(`{}`)}) + _, _ = conn.Write(append(reply, '\n')) + conn.Close() + } + }() + return got +} diff --git a/internal/cellstore/sealwatch.go b/internal/cellstore/sealwatch.go index ea99185be9..1f5d247ec5 100644 --- a/internal/cellstore/sealwatch.go +++ b/internal/cellstore/sealwatch.go @@ -6,6 +6,7 @@ import ( "net/url" "os" "sync" + "sync/atomic" "time" "github.com/Agent-Field/codeaf/internal/cell" @@ -158,10 +159,17 @@ func (w *SealWatch) Waiting() { } } +// liveTurns counts this process's watches with a turn running. The desktop +// app keeps its saving helpers warm only while one is (daemon_app.go). +var liveTurns atomic.Int64 + // TurnStarted is the session's word that a turn began: the turn writes the // record itself and every call seals it, so no tail seal is owed. func (w *SealWatch) TurnStarted() { w.mu.Lock() + if !w.started || w.idle { + liveTurns.Add(1) + } w.started = true w.idle = false w.stopTail() @@ -214,6 +222,9 @@ func (w *SealWatch) sealTail() { // forwards, so a watch without a listener costs nothing. func (w *SealWatch) TurnEnded() { w.mu.Lock() + if w.started && !w.idle { + liveTurns.Add(-1) + } w.started = true w.idle = true w.requireTranscript() diff --git a/internal/cellstore/testdata/daemon-lifetime/main.go b/internal/cellstore/testdata/daemon-lifetime/main.go index 1031aaedd0..982fc3d275 100644 --- a/internal/cellstore/testdata/daemon-lifetime/main.go +++ b/internal/cellstore/testdata/daemon-lifetime/main.go @@ -27,6 +27,8 @@ func main() { cellstore.SetAppDaemonLifetime(ctx) } d := cellstore.Daemon{Socket: os.Getenv("CODEAF_TEST_IDLE_SOCKET"), Binary: os.Getenv("CODEAF_TEST_IDLE_BINARY")} + // A running turn is the saving work that keeps an app's helper warm. + var chat cellstore.SealWatch fmt.Println("ready") lines := bufio.NewScanner(os.Stdin) for lines.Scan() { @@ -42,6 +44,9 @@ func main() { panic(err) } fmt.Println(health.PID) + case "turn": + chat.TurnStarted() + fmt.Println("turn") case "quiet": cancel() fmt.Println("quiet") diff --git a/internal/manual/chat/running-from-the-terminal.md b/internal/manual/chat/running-from-the-terminal.md index d06cd57a6e..02a7827ee1 100644 --- a/internal/manual/chat/running-from-the-terminal.md +++ b/internal/manual/chat/running-from-the-terminal.md @@ -2061,10 +2061,13 @@ processes; finished logs remain available on demand without an ended count. ## Desktop app quit and saving helpers -Saving helpers started by the desktop app use a one-minute idle limit. While -the app's engine is alive, it sends a light health request about every 20 seconds -to the helpers it started, so a quiet app keeps the same saving helper ready. -These requests never start or restart a helper, and failures are ignored. +Saving helpers started by the desktop app use a one-minute idle limit. While a +reply is running or a save is still in progress, the app's engine sends a light +health request about every 20 seconds to the helpers it started, so they stay +ready. An idle app sends none: its saving helper exits after about one minute +and stops using the processor, and the next save starts it again, which can add +a short delay to that save. These requests never start or restart a helper, and +failures are ignored. After the engine quits or is killed, the requests stop. Its saving helpers exit by themselves after about one minute without activity. Quit sends no shutdown