Skip to content

workspace(perf,refactor) Bulk set-option helper + 10ms readiness polling#1040

Open
tony wants to merge 3 commits into
masterfrom
perf-bulk-options-and-poll-tighten
Open

workspace(perf,refactor) Bulk set-option helper + 10ms readiness polling#1040
tony wants to merge 3 commits into
masterfrom
perf-bulk-options-and-poll-tighten

Conversation

@tony

@tony tony commented May 10, 2026

Copy link
Copy Markdown
Member

Summary

Three small, self-contained workspace-builder changes salvaged from a stalled experimental branch:

  1. _wait_for_pane_ready polling tightened from 50ms to 10ms. Cuts ~2-3s off suite wall time (within run-to-run noise but consistent with commit body's claim).
  2. _bulk_set_options helper added — a single, batch-shaped entry point for the four set-option loops the builder runs (session options, global_options, per-window options, options_after). Today the helper body is a plain loop, so this is a pure refactor; the call sites land on it so the body can later swap to a pipelined dispatch (e.g. a hypothetical Server.batch()) without touching the call sites.
  3. Call sites routed onto the helper for the four set-option loops.

The helper exists today as forward-compat plumbing — there's no measurable perf gain from the refactor against current libtmux (0.56.0 ships only the subprocess path; batching pays off on engines libtmux hasn't released). The polling-tighten commit is the only commit that materially affects wall time.

Why this PR exists

The bulk-options helper originated on a local-only experiment branch (libtmux-protocol) that pinned libtmux to a private worktree exposing a Server.batch() API. That experiment hasn't shipped upstream and may take time to land. Rather than let the call-site shape rot on a branch nobody can review, this PR salvages the shape against current libtmux. When Server.batch() (or equivalent) ships, the helper body becomes a one-line internal swap; the call sites stay as-is.

The polling-tighten commit was on the same experiment branch and is independent — it lands here because it shares context.

Measured timing (3 runs salvage, 2 runs master)

Branch    Run  pytest   wall    notes
master    1    76.50s   71.73s
master    2    78.12s   70.79s
salvage   1    67.27s   62.92s
salvage   2    82.19s   74.66s  2 reruns (pre-existing tmux flake)
salvage   3    77.06s   69.41s

Median pytest is essentially tied (~77s). Median wall favors salvage by ~2s, matching the commit body's "~2.5s wall time" claim. One salvage run hit a tmux-state flake with 2 reruns — same flake potential exists on master, just didn't surface in these particular master runs.

Per-commit breakdown

5cef952aperf(workspace[wait]) tighten _wait_for_pane_ready polling interval to 10ms

One-line change to the interval default in _wait_for_pane_ready. Tighter loop catches the early-readiness window (50-150ms tmux shell-ready latency) more reliably; the previous 50ms interval was missing the leading edge by definition.

6ccb0590workspace(feat[builder]) add _bulk_set_options helper for set-option loops

Adds WorkspaceBuilder._bulk_set_options(items, *, target, scope_flag). Mirrors OptionsMixin.set_option's True/False -> "on"/"off" normalisation so loop and helper paths produce identical tmux commands. Reuses libtmux's handle_option_error to preserve the OptionError subclasses callers already catch.

Body is a plain loop today. The API shape (mapping + scope flag + optional target) is deliberately batch-shaped so the body can swap to a pipelined dispatch when libtmux exposes one.

Adds two regression tests (test_bulk_set_options_propagates_unknown_option_error, test_bulk_set_options_applies_session_window_and_options_after).

9d4e4f3fworkspace(refactor[builder]) route set-option loops through _bulk_set_options

Converts the four hot loops to call the helper. Pure refactor — same N round-trips per call site, same observable behaviour. The original commit framed this as a perf change; rewritten here as a refactor since Server.batch() isn't on libtmux 0.56.0 and the original commit body itself notes batching is "no-op cost on subprocess and imsg" anyway. Perf framing deferred to whenever libtmux ships a batching API.

Test plan

  • uv run ruff check . --fix --show-fixes — clean
  • uv run ruff format . — clean
  • uv run mypy — clean (124 source files)
  • uv run py.test --reruns 0 -vvv — 799 passed, 2 skipped (master had 797 + 2 new regression tests)
  • just build-docs — succeeds
  • Per-commit timing comparison vs master — see above
@codecov

codecov Bot commented May 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.95238% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.92%. Comparing base (6edda37) to head (c13b428).

Files with missing lines Patch % Lines
src/tmuxp/workspace/builder.py 80.95% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1040      +/-   ##
==========================================
- Coverage   81.98%   81.92%   -0.07%     
==========================================
  Files          28       28              
  Lines        2548     2561      +13     
  Branches      485      487       +2     
==========================================
+ Hits         2089     2098       +9     
- Misses        328      330       +2     
- Partials      131      133       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
tony added 3 commits June 6, 2026 07:51
…o 10ms

why: Empirical microbenchmark (60-pane test suite) measured the previous
50ms interval costing ~41ms more per pane on subprocess and ~35ms more
on imsg than a 10ms interval, because tmux's shell-ready latency lands
in the 50-150ms band and the coarser poll missed the early-readiness
window. Tightening to 10ms saves ~2.5s wall time on tmuxp's suite with
no correctness change — same condition, just polled more frequently.
what:
- _wait_for_pane_ready: change interval default from 0.05 to 0.01
…loops

why: tmuxp's workspace builder fires N round-trips per workspace
load applying session/window/options_after metadata, one
``set-option`` per loop iteration. Centralising those calls
behind a single helper means we have one place to swap in a
pipelined dispatch (e.g. ``Server.batch()``) the moment libtmux
exposes one. The helper lands first without call-site changes
so behaviour parity can be verified in isolation.

The original draft of this work (``libtmux-protocol`` branch,
``f4c95faa``) used ``Server.batch()`` directly. That API doesn't
exist on libtmux 0.56.0 (it ships with the unmerged
libtmux-protocol-engines work), and even when present its perf
gain is documented as "no-op cost on subprocess and imsg" — so
batching only pays off on the unshipped control_mode engine. To
land the API shape against current libtmux without committing to
unshipped upstream, this commit issues one ``set-option`` per
item via ``server.cmd``. The helper API (mapping, target,
scope_flag) is deliberately batch-shaped so the body can swap
back to a single batched dispatch later without touching the
call sites.

what:
- Add ``WorkspaceBuilder._bulk_set_options(items, *, target,
  scope_flag)`` mirroring ``OptionsMixin.set_option``'s
  bool -> "on"/"off" normalisation (libtmux/options.py:712) so
  loop and helper paths produce identical tmux commands.
- Reuse libtmux's ``handle_option_error`` to preserve the
  ``OptionError`` subclasses callers already catch — the same
  propagation ``OptionsMixin.set_option`` does inside its own
  body (libtmux/options.py:712).
- Empty-mapping guard avoids a zero-iteration loop.
- Add two regression tests in tests/workspace/test_builder.py:
  ``test_bulk_set_options_propagates_unknown_option_error`` proves
  bad options still raise ``OptionError`` from the helper;
  ``test_bulk_set_options_applies_session_window_and_options_after``
  exercises the helper across all three scopes (-s, -g, -w) and
  verifies each option lands on tmux via ``show_option`` /
  ``show-option -v``.

Salvaged from libtmux-protocol@f4c95faa with the body rewritten
to drop the unshipped ``Server.batch()`` dependency. Test
docstrings updated to match the loop-based reality.
…_options

why: Workspace loading dispatches one tmux ``set-option`` per
loop iteration across four hot loops (session ``options``,
``global_options``, per-window ``options``, ``options_after``).
The helper that ``f4c95faa`` introduced provides a single,
batch-shaped entry point for these calls; this commit moves the
existing call sites onto it. Today the helper is a plain loop
so this is a pure refactor (same N round-trips, same observable
behaviour). Once libtmux exposes a pipelined ``Server.batch()``
the helper body can swap internally and the call sites
automatically benefit without further changes here.

what:
- Replace the session ``options`` loop in ``build`` with
  ``_bulk_set_options(..., scope_flag="-s")`` targeting
  ``session.session_id``.
- Replace the ``global_options`` loop in ``build`` with
  ``_bulk_set_options(..., target=None, scope_flag="-g")``.
- Replace the per-window ``options`` loop in
  ``iter_create_windows`` with ``_bulk_set_options(...,
  scope_flag="-w")`` targeting ``window.window_id``.
- Replace the ``options_after`` loop in
  ``_apply_options_after`` (or equivalent post-build hook) with
  ``_bulk_set_options(..., scope_flag="-w")`` targeting
  ``window.window_id``.

Salvaged from libtmux-protocol@2390ce3b. The original commit
described the change as a perf win; against current libtmux
0.56.0 the helper body issues N round-trips per call site (no
``Server.batch()`` yet), so this commit is documented as a
refactor and the perf framing is deferred to whenever libtmux
ships a batching API.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant