-
Notifications
You must be signed in to change notification settings - Fork 35
⚡ Bolt: Remove blocking XSync from drw_map #185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
Removing `XSync` from `drw_map` in `drw.c` eliminates a blocking round-trip to the X server during every drawing operation (e.g., bar updates). This significantly improves UI responsiveness and reduces latency, as the event loop or `XFlush` handles buffer flushing asynchronously. Also added `.jules/bolt.md` to document this learning. Co-authored-by: paperbenni <15818888+paperbenni@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRemoves a blocking XSync call from the drw_map drawing routine to avoid forced round-trips to the X server, and adds a Bolt journal note documenting the change and rationale. Sequence diagram for drw_map drawing without blocking XSyncsequenceDiagram
participant Client as ClientApp
participant Drw as Drw
participant Xlib as Xlib
participant XServer as XServer
Client->>Drw: drw_map(drw, win, x, y, w, h)
Drw->>Xlib: XCopyArea(drw->dpy, drw->drawable, win, drw->gc, x, y, w, h, x, y)
Xlib-->>Drw: return (request buffered)
Drw-->>Client: return
Client->>Xlib: XNextEvent or XFlush
Xlib->>XServer: send buffered requests
XServer-->>Xlib: process draw requests
Xlib-->>Client: events delivered (async)
rect rgb(240,240,240)
Note over Client,XServer: No XSync call, no blocking round-trip per drw_map
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- If any callers of
drw_mapwere implicitly relying onXSyncfor ordering or avoiding flicker (e.g., before reading back pixels or interacting with other X operations), consider documenting the asynchronous behavior now or adding explicit syncs at those specific call sites instead of globally indrw_map. - Confirm that
.jules/bolt.mdis intended to live in the main repo and not in a local/ephemeral tooling directory; if it’s tooling-specific metadata, it may be better tracked outside the source tree or added to ignore rules.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- If any callers of `drw_map` were implicitly relying on `XSync` for ordering or avoiding flicker (e.g., before reading back pixels or interacting with other X operations), consider documenting the asynchronous behavior now or adding explicit syncs at those specific call sites instead of globally in `drw_map`.
- Confirm that `.jules/bolt.md` is intended to live in the main repo and not in a local/ephemeral tooling directory; if it’s tooling-specific metadata, it may be better tracked outside the source tree or added to ignore rules.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
⚡ Bolt: Remove blocking XSync from drw_map
💡 What: Removed
XSync(drw->dpy, False)fromdrw_mapindrw.c.🎯 Why:
XSyncis a blocking call that forces the client to wait for the X server to process requests. In a drawing loop, this causes unnecessary latency and jank, especially under load or network latency.📊 Impact: Reduced latency for all bar and window drawing operations.
🔬 Measurement: Verify code no longer contains
XSyncindrw_map.PR created automatically by Jules for task 1875585114436176726 started by @paperbenni
Summary by Sourcery
Remove a blocking XSync call from the drawing routine to reduce rendering latency and document the change in Bolt’s journal.
Enhancements:
Documentation: