Skip to content

perf(recording): reuse one X11 connection and unchanged cursor PNGs - #2622

Open
celeroncoder wants to merge 2 commits into
CapSoftware:mainfrom
celeroncoder:perf/x11-cursor-image-source
Open

celeroncoder wants to merge 2 commits into
CapSoftware:mainfrom
celeroncoder:perf/x11-cursor-image-source

Conversation

@celeroncoder

@celeroncoder celeroncoder commented Oct 15, 2026 •

Copy link
Copy Markdown

Stacked on #2621. The first commit is #2621. Review 5c655ea only, or wait for #2621 to merge.

On X11 the cursor recorder calls get_x11_cursor_data() every tick (about 60/s). Each call:

  • opened a new X connection with x11rb::connect (socket, auth, setup)
  • ran xfixes_query_version
  • fetched GetCursorImage
  • converted ARGB to RGBA and encoded a PNG, even though the cursor image almost never changes between ticks

Change (Linux only)

get_cursor_data()                      thread_local X11CursorSource, one per recorder thread
  cursor_image()
    kept connection -> GetCursorImage
    request failed  -> drop it, connect + query_version once, retry in the same call
    connect failed  -> None (fallback cursor, as on main)
  same width, height and pixels as last tick?
    yes -> reuse the stored PNG
    no  -> x11_cursor_png() (main's encoder, moved), store it
  hotspot read from every reply
  • X11CursorSource (crates/recording/src/cursor.rs:857) owns the connection and does the XFixes handshake once. It lives in a thread_local! inside the Linux get_cursor_data(), so each recorder thread has its own and closes it when the thread exits.
  • If a request on the kept connection fails, the source reconnects once within the same call, so a dead connection never produces a fallback-cursor tick. If that connect also fails, the call returns None like main did. No retry loop.
  • GetCursorImage is still requested every tick, so a shape change shows up on the next tick as before. PNG bytes are identical to main, so cursor ids (SHA-256 of the PNG), saved cursor_N.png files and hotspots don't change.
  • The shared recorder loop, macOS and Windows are untouched.
  • New Linux-only tests in cursor::x11_cursor_tests that need no X server:
    • cached_cursor_pngs_match_a_fresh_encode covers the same image, changed pixels, swapped width/height and switching back. It fails if either the dimension or the pixel check is dropped from the cache key.
    • cursor_pixels_are_encoded_as_rgba
    • cursor_with_mismatched_pixel_count_is_skipped

Results (Linux X11)

main @ 83237e9 #2621 @ b4f88a6 This PR @ 5c655ea
X11 connect() per tick, moving pointer 2.00 1.00 0 (3 at startup, then none)
Recorder CPU per 10 s, idle, 16 px core cursor 240 ms [233-256] 150 ms [140-155] 69 ms [68-75]
Recorder CPU per 10 s, idle, 24 px ARGB cursor 259 ms [240-270] 166 ms [160-185] 76 ms [70-89]
Recorder CPU per 10 s, idle, 48 px ARGB cursor 306 ms [301-310] 222 ms [222-232] 80 ms [78-81]
Recorder CPU per tick, moving pointer, 24 px ARGB 414 µs [407-470] 266 µs [265-269] 115 µs [114-115]
Xvfb CPU per 10 s, idle, 24 px ARGB cursor 240 ms [220-260] 140 ms [140-150] 50 ms [40-70]

The real spawn_cursor_recorder ran for 10 s from release builds against Xvfb 1920x1080 on a shared 2 vCPU aarch64 VM. CPU is the recorder's user+sys from getrusage, median of 5 interleaved runs with the range in brackets. Connection counts come from one strace run.

  • Bigger cursors cost more to encode. At 48 px, recorder user time drops from 139 to 39 ms per 10 s compared to perf(recording): take the cursor position from the recorder's mouse sample #2621.
  • The table was taken just before the reconnect retry was added, which only touches the error path. A re-run on 5c655ea gave 71 ms [67-73] per 10 s idle at 24 px and 113 µs [113-119] per moving tick.
  • The idle runs shared the VM with a background build (load1 1.13-3.74). The moving runs were at load1 0.20-0.96. Xvfb has no compositor, so a real X.Org or XWayland session will show different absolute numbers.

Output equivalence

  • A throwaway Xvfb test compared this source with main's get_x11_cursor_data (copied verbatim) across 10 root-cursor changes over 6 shapes, including repeats and switching back. Every PNG and hotspot was byte-identical. The cache re-encoded on every shape change and reused the PNG only when the image was unchanged.
  • A 10 s recording ran on main, perf(recording): take the cursor position from the recorder's mouse sample #2621 and this branch while the root cursor cycled through 6 shapes every 400 ms. All three registered the same 6 cursors with identical PNG-hash keys, hotspots and saved files. The move timeline switched cursor id 25 times on each, so no stale cursor was served.
  • Killing the kept connection from the server side (KillClient, 3 times): each next call reconnected within the call (about 0.35 ms) and returned the real cursor. With an unreachable DISPLAY the call returned None once, then reconnected after DISPLAY was restored.

Scope and overlap

How this was made

This PR is agent-generated.

  • Gemini 4 Argon (Antigravity) and space-bunny-free (OpenCode) found this during an audit of the repo and wrote the first patch.
  • Claude Opus 5.5 reviewed that patch, rewrote it on current main, added the tests and took the measurements above.
  • Gemini 4 Argon reviewed the final diff and numbers over two rounds. The in-call reconnect came out of its first round, and it approved the second.

🤖 Generated with Claude Code

RetriggerView in GreptileConfidence Score: 5/5

The reviewed commit appears safe to merge.

Summary

Reviewed commit 5c655eab as requested. The earlier coordinate changes belong to the preceding stacked PR and are excluded.

  • Linux X11 cursor capture keeps a connection and reuses unchanged cursor PNGs.
  • The recorder builds cursor positions from the mouse sample it already has.

No actionable issues found. This review used code inspection; tests were not run.

Reviews (1) · Last reviewed commit: "perf(recording): keep one X connection f..." · Reviewed by Greptile

@vercel

vercel Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

@celeroncoder is attempting to deploy a commit to the Cap Software Inc Team on Vercel.

A member of the Team first needs to authorize it.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant