GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||
GeezerVison is showing github.com — workspace(perf,refactor) Bulk set-option helper + 10ms readiness polling by tony · Pull Request #1040 · tmux-python/tmuxp Conversation
Member
SummaryThree small, self-contained workspace-builder changes salvaged from a stalled experimental branch:
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 existsThe bulk-options helper originated on a local-only experiment branch ( 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)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
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@ ## master #1040 +/- ## ========================================== - Coverage 82.55% 82.48% -0.07% ========================================== Files 31 31 Lines 2768 2781 +13 Branches 518 520 +2 ========================================== + Hits 2285 2294 +9 - Misses 346 348 +2 - Partials 137 139 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
9d4e4f3 to
c13b428
Compare
c13b428 to
8b971ed
Compare
why: Workspace loads wait on shell prompt readiness with a 50ms poll, coarser than tmux's 50-150ms shell-ready band, so readiness is noticed late. Separately, the classic builder fires one set-option per loop iteration across four hot loops (session options, global_options, per-window options, options_after) with no shared seam for a future batched dispatch. what: - Tighten _wait_for_pane_ready interval default from 0.05 to 0.01 so prompt readiness is detected sooner (same condition). - Add ClassicWorkspaceBuilder._bulk_set_options(items, *, target, scope_flag): mirrors set_option's True/False -> on/off normalization and propagates errors via handle_option_error. - Route the four set-option loops through the helper: session (-s, session_id), global (-g, None), per-window (-w, window_id), options_after (-w, window_id). - Add tests/workspace/test_builder_bulk_options.py covering scope application, on/off normalization, empty no-op, and OptionError propagation. Salvaged from PR #1040, adapted from the pre-refactor single-file builder.py onto the builder package's classic.py. The helper body issues N round-trips today; the batch-shaped API defers the perf gain to a future libtmux batching API.
|
Automated review finding. Recorded here so it is not lost. Nothing has been pushed to this branch and no fix is applied — this is a note, not a change request. Session-scoped
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…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.
why: Workspace loads wait on shell prompt readiness with a 50ms poll, coarser than tmux's 50-150ms shell-ready band, so readiness is noticed late. Separately, the classic builder fires one set-option per loop iteration across four hot loops (session options, global_options, per-window options, options_after) with no shared seam for a future batched dispatch. what: - Tighten _wait_for_pane_ready interval default from 0.05 to 0.01 so prompt readiness is detected sooner (same condition). - Add ClassicWorkspaceBuilder._bulk_set_options(items, *, target, scope_flag): mirrors set_option's True/False -> on/off normalization and propagates errors via handle_option_error. - Route the four set-option loops through the helper: session (-s, session_id), global (-g, None), per-window (-w, window_id), options_after (-w, window_id). - Add tests/workspace/test_builder_bulk_options.py covering scope application, on/off normalization, empty no-op, and OptionError propagation. Salvaged from PR #1040, adapted from the pre-refactor single-file builder.py onto the builder package's classic.py. The helper body issues N round-trips today; the batch-shaped API defers the perf gain to a future libtmux batching API.
8b971ed to
41ccf9e
Compare
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.