Ask the listing's questions together

CHANGELOG.md

@@ -148,6 +148,52 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **`dl --ls --json` asks its `devpod status` round trips together, and `Runner`
+  now requires `Sync`.** The `--json` is the whole of which command this is about:
+  the human table `dl --ls` prints has no state column and costs one `devpod list`
+  and nothing per row. The document is the surface carrying a `state` for every
+  workspace, and `devpod list` does not answer that, so the document asks devpod
+  about every workspace in it, including the ones devlaunch did not make. Those
+  trips are required and none has been removed. What the document no longer does
+  is wait for each answer before asking the next question: nothing devpod says
+  about one workspace changes what is asked about another. They go out in batches
+  of eight, so a forty workspace
+  machine pays five batches rather than forty trips end to end. The number of
+  trips is unchanged, which is why the test that pins that cost reads exactly as
+  before.
+
+  Measured against real devpod on one docker host: ten workspaces went from 5.13s
+  to 1.40s. That is about 3.7x rather than the 8x the width suggests, because a
+  trip costs about 38% more when eight of them are in flight (0.465s alone against
+  0.641s in a batch). `docs/performance.md` carries the per-chunk figures.
+
+  **The seam change is the part with consequences beyond this repository.**
+  `devlaunch_runner::Runner` gains `Sync` as a supertrait, which is what lets one
+  `&dyn Runner` be handed to several threads. Any out-of-tree implementation
+  holding a `RefCell`, `Rc` or `Cell` no longer compiles. In tree it cost
+  nothing: `ProcessRunner` is a unit struct, and three test wrappers took the
+  change from `RefCell` to `Mutex` that a shared recorder wants anyway. The
+  alternative, a `Sync` bound written at each call site that needs one, was
+  rejected because it puts the requirement in the callers rather than in the
+  contract and so permits an implementation that satisfies some callers and not
+  others. One row of `devlaunch-runner/public-api.txt` moves.
+
+  `devlaunch-core/public-api.api.txt` does not move at all, and that is the part
+  to read twice rather than the reassurance it looks like. The promised tier hands
+  out `CommandContext::new(&'r dyn Runner)`, `ColdPath::new`, `Refresh::ask` and
+  `Provision::provision_tools`, and every one of them names a `dyn Runner` that
+  has just narrowed to `dyn Runner + Sync`. They render exactly as before, so the
+  promised contract tightened without a single row changing. The snapshot guards
+  compare rendered rows and cannot see a supertrait reach the promised surface
+  through a `dyn` it names, which is why this paragraph is the migration note and
+  the diff is not.
+
+  A timing document for `dl --ls --json` reports smaller `devpod-up` **stage**
+  seconds as a result, since that stage is now the wall time of the batch loop
+  rather than the sum of the per-row status times. The spans themselves, and their
+  count, are unchanged, so the stage now reports less than the spans inside it add
+  up to.
+
 - **A workspace id is derived once, and the three signatures that had a triple in
   hand stopped flattening it into loose strings.** `WorkspaceId::value()` ran the
   whole derivation on every call — a SHA-256 over the triple, three slug passes

docs/performance.md

@@ -20,6 +20,68 @@ naming it and then the tools probe, rides a single setup pass. So an
 interactive `dl <ws>` and a one-shot
 `dl <ws> -- <cmd>` cost the same trips.
 
+## The listing's questions are asked together
+
+`dl --ls --json` is the one command whose cost grows with the machine, and the
+`--json` is load-bearing: the human table `dl --ls` prints has no state column, so
+it costs the single `devpod list` and nothing per row. The document is the surface
+that carries a `state` for every workspace, and a state is a question `devpod list`
+does not answer, so the document asks `devpod status` once per workspace,
+including about the ones devlaunch did not make. That is one round trip per row
+and there is no way around it: a document of forty workspaces asks forty
+questions. `the_table_asks_devpod_for_the_list_and_nothing_else` holds the two
+apart.
+
+What it no longer does is wait for each one before asking the next. The questions
+are independent, so they go out in batches of eight and the waiting overlaps:
+forty workspaces cost five batches rather than forty trips end to end. The trips
+themselves are unchanged in number, which is why
+`the_listing_costs_one_list_and_one_status_per_workspace` still reads the same;
+what changed is only how much of the waiting happens at once.
+
+**A batch costs the slowest trip in it, not the average.** The eight are started
+together and all eight are waited for before the next eight begin, so this is a
+barrier rather than a pool of eight permits: one slow answer leaves seven threads
+idle until it lands. It is never worse than asking serially, which is what it
+replaced, but a work queue that started the ninth trip the moment any of the first
+eight returned would be better on a machine where one workspace is much slower to
+answer than the rest. Worth knowing before reading a slow `--ls --json` as
+something else.
+
+**A trip also costs more when eight are in flight, so the win is not the width.**
+Measured on 2026-09-01, on one host with ten workspaces on the local docker
+provider and devpod 0.26.1: asked serially the ten trips averaged 0.465s each and
+the `devpod-up` stage took 4.656s. Batched, the same ten averaged 0.641s each,
+about 38% more, and the stage took 1.724s. The command went from 5.132s to 1.395s.
+So read the win as roughly 3.7x rather than the 8x the width suggests, and read
+"forty workspaces cost five batches" as five batches whose trips are each slower
+than a lone one would be.
+
+That run shows the barrier's cost rather than describing it. The first chunk's
+eight trips landed between 0.593s and 0.656s, and the second chunk, holding the
+two rows left over, took 0.443s and 1.066s. The slower of those two is 62% of the
+whole stage, and the work queue above would have spent the faster one's thread on
+something instead of idling it.
+
+Eight is chosen for the shape of the wait rather than for the core count. A trip
+is one process blocking on devpod's own work rather than arithmetic, so the useful
+width is set by how many of those the machine will schedule. It is bounded rather
+than unlimited because a row costs a devpod process, and the cost of starting
+sixty at once is a real one. What the figures above settle is that the contention
+is real rather than hypothetical and that eight still pays; what they do not
+settle is where the curve turns, since nothing has been measured at four or at
+sixteen, and eight is a conservative pick rather than a tuned one.
+
+One thing the change does move: the `devpod-up` **stage** seconds a timing
+document reports for `dl --ls --json` used to be the sum of the per-row status
+times and are now the wall time of the batch loop, which is smaller. The span
+count, and every individual span, are unchanged. So the stage now reports less
+than the spans inside it add up to, which is the one place this page's arithmetic
+stops being addition.
+
+This is also why the document is something somebody asks for rather than something
+on the launch path. A launch asks about one workspace, and pays one trip for it.
+
 ## One connection per workspace
 
 The trip that carries `dl <ws> -- <cmd>` at a terminal is OpenSSH, over the host

rust/devlaunch-core/src/flows/agent_worktrees/tests.rs

@@ -13,7 +13,7 @@
 //! the absence of a path — a plan that contains no unit for it, a spawn log that
 //! contains no invocation naming it — rather than a guard firing.
 
-use std::cell::RefCell;
+use std::sync::Mutex;
 
 use devlaunch_runner::{
     CapturedText, DetachOutcome, Invocation, Outcome, ProcessRunner, Runner, SpawnSpec,
@@ -156,7 +156,7 @@ impl Clone {
         plan: &CloneWorktrees,
         forgets_must_be_absent: bool,
     ) -> (WorktreeReport, Vec<Vec<String>>) {
-        let calls = RefCell::new(Vec::new());
+        let calls = Mutex::new(Vec::new());
         let runner = Recording {
             real: ProcessRunner::new(),
             calls: &calls,
@@ -165,7 +165,7 @@ impl Clone {
         let git = Git::new(&runner);
         let mut report = WorktreeReport::default();
         reclaim(&git, plan, Some(&self.bare), &mut report);
-        (report, calls.into_inner())
+        (report, calls.into_inner().expect("the recorded calls"))
     }
 
     fn listing(&self) -> String {
@@ -214,7 +214,7 @@ impl OtherRepository {
 /// forget is invoked, which is P2 asserted directly (devlaunch#462).
 struct Recording<'a> {
     real: ProcessRunner,
-    calls: &'a RefCell<Vec<Vec<String>>>,
+    calls: &'a Mutex<Vec<Vec<String>>>,
     /// Assert P2 at every forget: the argument must not exist when the spawn
     /// happens. Off for the one fixture whose recorded path deliberately
     /// resolves into another repository, where the point is git's refusal.
@@ -239,7 +239,7 @@ impl Runner for Recording<'_> {
                  invoked, and {target} does"
             );
         }
-        self.calls.borrow_mut().push(argv);
+        self.calls.lock().expect("the recorded calls").push(argv);
         self.real.capture(spec)
     }
 
@@ -1106,7 +1106,7 @@ fn a_foreign_leaf_colliding_with_our_admin_name_is_not_probed_through_our_index(
     let theirs = other.worktree_at(&worktrees_dir(&outer).join("agent-outer"), "agent-outer");
     world.containerise();
 
-    let calls = RefCell::new(Vec::new());
+    let calls = Mutex::new(Vec::new());
     let runner = Recording {
         real: ProcessRunner::new(),
         calls: &calls,
@@ -1139,11 +1139,12 @@ fn a_foreign_leaf_colliding_with_our_admin_name_is_not_probed_through_our_index(
     let theirs_spelled = format!("--work-tree={}", theirs.display());
     assert!(
         !calls
-            .borrow()
+            .lock()
+            .expect("the recorded calls")
             .iter()
             .any(|argv| argv.iter().any(|arg| arg == &theirs_spelled)),
         "the foreign site must never be probed: {:?}",
-        calls.borrow()
+        calls.lock().expect("the recorded calls")
     );
 }