perf(cmux-tui-core): a pane or tab list inside one workspace reads only that workspace (CLI-1)

cmux-tui/crates/cmux-tui-core/src/mux/tests/read_paths.rs

@@ -176,3 +176,85 @@ fn session_ping_reads_only_the_topology_frame() {
         .collect::<Vec<_>>();
     assert!(extra.is_empty(), "session.ping read {extra:?}");
 }
+
+/// Tables a pane or tab list of one pane, screen or workspace has no use
+/// for: terminal hosts, projections and the journal (a tab row joins its
+/// terminal's resource row for the host id).
+const NOT_A_TAB_TABLE: &[&str] = &[
+    "terminal_hosts",
+    "resource_agent_projections",
+    "resource_effect_receipts",
+    "frontend_projections",
+    "session_journal",
+];
+
+/// CLI-1 (TYPING UNDER LOAD, STATUS 6237): `pane.list` and `tab.list` built
+/// the whole session snapshot under the registry and state locks, then kept
+/// one pane's rows; under the L1 CLI storm that held both locks 37 % of the
+/// time. A scoped list reads only its workspace's rows, the same values.
+#[test]
+fn scoped_pane_and_tab_lists_read_only_their_workspace() {
+    let mux = test_mux();
+    for _ in 0..2 {
+        let first = mux.new_workspace(None, Some((80, 24))).unwrap();
+        let pane = mux.with_state(|state| state.pane_of(first.id).unwrap());
+        mux.split(pane, crate::SplitDir::Right, Some((80, 24))).unwrap();
+        mux.new_tab(Some(pane), None, Some((80, 24))).unwrap();
+    }
+    let snapshot = resource(&mux, "session.snapshot", json!({}));
+    let rows = |collection: &str| snapshot[collection].as_array().unwrap().clone();
+    let pane = rows("panes")[0]["id"].as_str().unwrap().to_string();
+    let screen = rows("panes")[0]["screen_id"].as_str().unwrap().to_string();
+    let workspace = rows("screens")
+        .into_iter()
+        .find(|value| value["id"] == screen.as_str())
+        .unwrap()["workspace_id"]
+        .as_str()
+        .unwrap()
+        .to_string();
+    let screens = rows("screens")
+        .into_iter()
+        .filter(|value| value["workspace_id"] == workspace.as_str())
+        .map(|value| value["id"].as_str().unwrap().to_string())
+        .collect::<BTreeSet<_>>();
+    let panes = rows("panes")
+        .into_iter()
+        .filter(|value| screens.contains(value["screen_id"].as_str().unwrap()))
+        .collect::<Vec<_>>();
+    let pane_ids = panes
+        .iter()
+        .map(|value| value["id"].as_str().unwrap().to_string())
+        .collect::<BTreeSet<_>>();
+    let tabs_of = |keep: &dyn Fn(&str) -> bool| {
+        Value::Array(
+            rows("tabs")
+                .into_iter()
+                .filter(|value| keep(value["pane_id"].as_str().unwrap()))
+                .collect(),
+        )
+    };
+    let cases = [
+        ("pane.list", json!({"workspace": workspace}), Value::Array(panes.clone())),
+        ("tab.list", json!({"pane": pane}), tabs_of(&|id| id == pane)),
+        ("tab.list", json!({"workspace": workspace}), tabs_of(&|id| pane_ids.contains(id))),
+    ];
+    assert!(pane_ids.len() >= 2, "the workspace has two panes: {snapshot}");
+    for (operation, scope, expected) in cases {
+        assert!(expected.as_array().is_some_and(|rows| !rows.is_empty()), "{operation} {scope}");
+        let reads = crate::workspace_registry::WorkspaceRegistry::full_topology_reads_for_test();
+        let mut listed = Value::Null;
+        let touched = touched(&mux, || listed = resource(&mux, operation, scope.clone()));
+        assert_eq!(listed, expected, "{operation} {scope} equals the snapshot's rows");
+        assert_eq!(
+            crate::workspace_registry::WorkspaceRegistry::full_topology_reads_for_test(),
+            reads,
+            "{operation} {scope} read the whole topology"
+        );
+        let extra = NOT_A_TAB_TABLE
+            .iter()
+            .filter(|table| touched.read.contains(**table))
+            .collect::<Vec<_>>();
+        assert!(extra.is_empty(), "{operation} {scope} read {extra:?}");
+        assert!(touched.written.is_empty(), "{operation} {scope} wrote {:?}", touched.written);
+    }
+}

cmux-tui/crates/cmux-tui-core/src/resource_api.rs

@@ -21,7 +21,8 @@ use serde_json::{Map, Value, json};
 
 use crate::resource::{
     ContentPublicId, FrontendProjectionPublicId, MachinePublicId, PanePublicId, ResourceError,
-    ResourceOperation, Selector, SessionPublicId, TabPublicId, TerminalPublicId,
+    ResourceOperation, ScreenPublicId, Selector, SessionPublicId, TabPublicId, TerminalPublicId,
+    WorkspacePublicId,
 };
 use crate::resource_screen::{public_screen_value, tabs_by_pane};
 use crate::sidebar_resource::{sidebar_snapshot, sidebar_view_id};
@@ -589,7 +590,6 @@ pub(crate) fn public_session_snapshot_with_journal_head(
             .iter()
             .map(|screen| (&screen.public_id, screen))
             .collect::<HashMap<_, _>>();
-        let active_screens = topology.active_screens.iter().cloned().collect::<HashMap<_, _>>();
         let terminals_by_id = terminal_registry
             .terminals
             .iter()
@@ -610,38 +610,7 @@ pub(crate) fn public_session_snapshot_with_journal_head(
             .map(|screen| public_screen_value(&topology, screen, &tabs_by_pane, &panes_by_id))
             .collect::<anyhow::Result<Vec<_>>>()?;
 
-        let panes = topology
-            .panes
-            .iter()
-            .map(|pane| {
-                let screen = screens_by_id
-                    .get(&pane.screen_id)
-                    .ok_or_else(|| anyhow::anyhow!("pane references a missing screen"))?;
-                let screen_focused = topology.active_workspace.as_ref()
-                    == Some(&screen.workspace_id)
-                    && active_screens.get(&screen.workspace_id).and_then(Option::as_ref)
-                        == Some(&screen.public_id);
-                Ok(json!({
-                    "id": pane.public_id,
-                    "screen_id": pane.screen_id,
-                    "name": pane.name,
-                    "focused": screen_focused && screen.active_pane == pane.public_id,
-                    "zoomed": screen.zoomed_pane.as_ref() == Some(&pane.public_id),
-                }))
-            })
-            .collect::<anyhow::Result<Vec<_>>>()?;
-
-        let tabs = topology
-            .tabs
-            .iter()
-            .map(|tab| {
-                let pane = panes_by_id
-                    .get(&tab.pane_id)
-                    .ok_or_else(|| anyhow::anyhow!("tab references a missing pane"))?;
-                checked_index(tab.position)?;
-                Ok(tab.public_value(pane.active_tab.as_ref() == Some(&tab.public_id)))
-            })
-            .collect::<anyhow::Result<Vec<_>>>()?;
+        let (panes, tabs) = public_pane_and_tab_values(&topology, &screens_by_id, &panes_by_id)?;
 
         let mut terminal_order = Vec::new();
         let mut seen_terminals = HashSet::new();
@@ -888,6 +857,100 @@ pub(crate) fn public_workspaces(mux: &Mux) -> Result<Vec<Value>, ResourceError>
     .map_err(operation_failed)
 }
 
+/// The public panes and tabs of `topology`, in its order: one builder for
+/// the session snapshot and the scoped lists.
+fn public_pane_and_tab_values(
+    topology: &crate::workspace_registry::ResourceTopologySnapshot,
+    screens_by_id: &HashMap<&ScreenPublicId, &crate::workspace_registry::RegistryScreen>,
+    panes_by_id: &HashMap<&PanePublicId, &crate::workspace_registry::RegistryPane>,
+) -> anyhow::Result<(Vec<Value>, Vec<Value>)> {
+    let active_screens = topology.active_screens.iter().cloned().collect::<HashMap<_, _>>();
+    let panes = topology
+        .panes
+        .iter()
+        .map(|pane| {
+            let screen = screens_by_id
+                .get(&pane.screen_id)
+                .ok_or_else(|| anyhow::anyhow!("pane references a missing screen"))?;
+            let screen_focused = topology.active_workspace.as_ref() == Some(&screen.workspace_id)
+                && active_screens.get(&screen.workspace_id).and_then(Option::as_ref)
+                    == Some(&screen.public_id);
+            Ok(json!({
+                "id": pane.public_id,
+                "screen_id": pane.screen_id,
+                "name": pane.name,
+                "focused": screen_focused && screen.active_pane == pane.public_id,
+                "zoomed": screen.zoomed_pane.as_ref() == Some(&pane.public_id),
+            }))
+        })
+        .collect::<anyhow::Result<Vec<_>>>()?;
+    let tabs = topology
+        .tabs
+        .iter()
+        .map(|tab| {
+            let pane = panes_by_id
+                .get(&tab.pane_id)
+                .ok_or_else(|| anyhow::anyhow!("tab references a missing pane"))?;
+            checked_index(tab.position)?;
+            Ok(tab.public_value(pane.active_tab.as_ref() == Some(&tab.public_id)))
+        })
+        .collect::<anyhow::Result<Vec<_>>>()?;
+    Ok((panes, tabs))
+}
+
+/// The session snapshot's `screens` (id and workspace only), `panes` and
+/// `tabs` of one workspace, for a `pane.list` or `tab.list` scoped to a
+/// workspace, screen or pane (CLI-1): the same values in the same order,
+/// from the workspace's stored subtree. It reads no other workspace, no
+/// terminals, projections or journal, and holds only the registry lock, not
+/// state, so a list costs the workspace's size, not the session's.
+pub(crate) fn public_workspace_topology(
+    mux: &Mux,
+    workspace: &WorkspacePublicId,
+) -> Result<Value, ResourceError> {
+    let registry = mux.workspace_registry.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
+    // One connection hold for the whole read, as `public_workspaces`.
+    let _connection = registry.connection.get();
+    (|| -> anyhow::Result<Value> {
+        let mut topology = registry.resource_topology_scoped(std::slice::from_ref(workspace))?;
+        // The full load's order (topology_load.rs): panes by screen id, then
+        // creation; tabs by pane id, then position. The scoped load walks
+        // screens by position, so sort to match.
+        topology.panes.sort_by(|left, right| {
+            (&left.screen_id, left.creation_ordinal, &left.public_id).cmp(&(
+                &right.screen_id,
+                right.creation_ordinal,
+                &right.public_id,
+            ))
+        });
+        topology.tabs.sort_by(|left, right| {
+            (&left.pane_id, left.position).cmp(&(&right.pane_id, right.position))
+        });
+        let screens_by_id = topology
+            .screens
+            .iter()
+            .map(|screen| (&screen.public_id, screen))
+            .collect::<HashMap<_, _>>();
+        let panes_by_id =
+            topology.panes.iter().map(|pane| (&pane.public_id, pane)).collect::<HashMap<_, _>>();
+        let (panes, mut tabs) =
+            public_pane_and_tab_values(&topology, &screens_by_id, &panes_by_id)?;
+        registry.read_state(|connection| {
+            for tab in &mut tabs {
+                crate::state::values::decorate_value(connection, "tab", tab)?;
+            }
+            Ok(())
+        })?;
+        let screens = topology
+            .screens
+            .iter()
+            .map(|screen| json!({"id": screen.public_id, "workspace_id": screen.workspace_id}))
+            .collect::<Vec<_>>();
+        Ok(json!({"screens": screens, "panes": panes, "tabs": tabs}))
+    })()
+    .map_err(operation_failed)
+}
+
 /// The session snapshot's `cursor` alone (`session.ping`): the topology
 /// generation and resource revision, from the topology frame.
 pub(crate) fn public_session_cursor(mux: &Mux) -> Result<Value, ResourceError> {

cmux-tui/crates/cmux-tui-core/src/resource_router/topology.rs

@@ -128,7 +128,16 @@ fn list_resources(
     operation: &str,
 ) -> Result<Value, ResourceError> {
     let path = mux.resolve_resource_path(scope, selectors)?;
-    let snapshot = public_session_snapshot(mux)?;
+    // A pane or tab list inside one workspace reads that workspace's rows
+    // only, under the registry lock alone (CLI-1: the whole snapshot held
+    // both global locks for each CLI `tab list`); every other list reads
+    // the session snapshot.
+    let snapshot = match &path.workspace {
+        Some(workspace) if matches!(collection, "panes" | "tabs") => {
+            crate::resource_api::public_workspace_topology(mux, workspace)?
+        }
+        _ => public_session_snapshot(mux)?,
+    };
     let path_index = SnapshotPathIndex::new(&snapshot);
     let values = snapshot[collection]
         .as_array()