Skip to content

perf: Reduce PTY read latency and fix lock contention #53

Description

@datasciencemonkey

Problem

The current PTY read loop and session management have latency and contention issues that affect terminal responsiveness, especially under high-volume output (e.g., seq 1 10000).

Proposed Changes (cherry-pick from PR #52)

PR #52 includes performance fixes that are independent of the OpenCode fork and should be extracted:

1. select() timeout reduction

  • Current: 500ms timeout in the PTY read loop
  • Proposed: 50ms — reduces worst-case latency for output delivery by 10x

2. Lock contention in get_output_batch()

  • Current: Single global sessions_lock held for the entire operation
  • Proposed: 3-step resolve/swap/join pattern — grab reference under lock, swap buffer outside lock, join strings outside lock. Matches the pattern already used in get_output().

3. Lock contention in cleanup_stale_sessions()

  • Current: Iterates and cleans up under the global lock
  • Proposed: Snapshot session dict under lock, then iterate/cleanup outside lock

4. Poll-worker interval

  • Current: 100ms poll interval in static/poll-worker.js
  • Proposed: 50ms — improves responsiveness for HTTP polling fallback

Why separate from PR #52

These are changes to app.py and static/poll-worker.js only. They don't require the OpenCode fork, spawner app, or any other PR #52 components. Extracting them keeps the review focused and the blast radius small.

Files affected

  • app.py — select timeout, lock patterns
  • static/poll-worker.js — poll interval

Activity

  1. datasciencemonkey commented on Mar 11, 2026

    @datasciencemonkey
    OwnerAuthor

    These perf fixes are still valuable and should be cherry-picked from PR #52 as a standalone change. Independent of the OpenCode fork — just changes to app.py and static/poll-worker.js.

  2. datasciencemonkey commented on Mar 12, 2026

    @datasciencemonkey
    OwnerAuthor

    Resolved by PR #48 — per-session locks replaced global lock, WebSocket I/O eliminated polling latency, thread pool increased to 16.

  3. datasciencemonkey commented on Mar 12, 2026

    @datasciencemonkey
    OwnerAuthor

    Reopening — PR #48 addressed item 2 (per-session locks for output batch) but 3 items remain:

    • select() timeout still 500ms (app.py:490) — should be 50ms
    • cleanup_stale_sessions() still holds global lock during iteration (app.py:571) — should snapshot-then-iterate
    • Poll-worker interval still 100ms (poll-worker.js:24) — should be 50ms for HTTP fallback responsiveness
  4. datasciencemonkey commented on Mar 12, 2026

    @datasciencemonkey
    OwnerAuthor

    Closing — the meaningful work was completed in PR #48. The 3 remaining items don't warrant changes:

    select() timeout 500ms → 50ms — select returns immediately when data is available. The 500ms is only the idle sleep before checking if the process died. Reducing it doesn't improve output latency at all — just checks for dead processes more often, burning CPU for no user-visible gain.

    cleanup_stale_sessions() lock pattern — The global lock is held for microseconds while iterating 1-3 sessions (single-user app). No I/O inside the lock, termination already happens outside it. No contention exists to fix.

    Poll-worker interval 100ms → 50ms — HTTP polling is now a fallback only (PR #48 made WebSocket primary). When WS is active, the poll-worker is stopped entirely. Optimizing the degraded fallback path by 50ms isn't worth 2x request volume.

    PR #48 shipped the changes that mattered: per-session locks (real contention fix) and WebSocket I/O (eliminated polling latency).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions