-
Notifications
You must be signed in to change notification settings - Fork 35
β‘ Bolt: Optimize drw_map drawing performance #192
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
Replacing XSync with XFlush in drw_map avoids unnecessary blocking round-trips to the X server during frequent drawing operations (like bar updates), significantly improving responsiveness. 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
β¨ Finishing touchesπ§ͺ Generate unit tests (beta)
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 GuideOptimize X11 bar drawing by replacing a blocking XSync call with a non-blocking XFlush in drw_map, and document the learning in a new Jules bolt note. Sequence diagram for optimized drw_map XFlush-based drawingsequenceDiagram
actor User
participant WM as WindowManager
participant Drw as Drw
participant X11 as XServer
User->>WM: Trigger bar update (focus change, clock tick, tag switch)
WM->>Drw: drawbar
Drw->>Drw: prepare drawable buffer
Drw->>X11: XCopyArea(drw_drawable, win, gc, x, y, w, h, x, y)
Drw->>X11: XFlush(dpy)
X11-->>X11: Process requests asynchronously
X11-->>User: Updated bar appears when processed
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 found 1 issue, and left some high level feedback:
- In
.jules/bolt.mdthe "XSync vs XFlush" section is duplicated and the first entry is missing backticks/formatting aroundXSyncanddrw_map, so it would be good to consolidate this into a single correctly formatted section.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `.jules/bolt.md` the "XSync vs XFlush" section is duplicated and the first entry is missing backticks/formatting around `XSync` and `drw_map`, so it would be good to consolidate this into a single correctly formatted section.
## Individual Comments
### Comment 1
<location> `.jules/bolt.md:2-3` </location>
<code_context>
+## 2026-02-04 - XSync vs XFlush in Drawing
+**Learning:** forces a blocking round-trip to the X server, which is catastrophic for frequent drawing operations like bar updates ().
+**Action:** Use for drawing operations where the client doesn't need to wait for the server's response.
+## 2026-02-04 - XSync vs XFlush in Drawing
+**Learning:** `XSync` forces a blocking round-trip to the X server, which is catastrophic for frequent drawing operations like bar updates (`drw_map`).
</code_context>
<issue_to_address>
**issue (typo):** The first "Learning" and "Action" lines appear incomplete and missing function names.
In the first pair of lines, the text after "**Learning:**" is missing, and "bar updates ()" has empty parentheses. The "**Action:** Use for drawing..." line also lacks the function name. Since the second pair of lines already correctly mentions `XSync`, `XFlush`, and `drw_map`, please either complete these first two lines to match the intended wording or remove them if they were an earlier draft.
</issue_to_address>Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
| **Learning:** forces a blocking round-trip to the X server, which is catastrophic for frequent drawing operations like bar updates (). | ||
| **Action:** Use for drawing operations where the client doesn't need to wait for the server's response. |
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.
issue (typo): The first "Learning" and "Action" lines appear incomplete and missing function names.
In the first pair of lines, the text after "Learning:" is missing, and "bar updates ()" has empty parentheses. The "Action: Use for drawing..." line also lacks the function name. Since the second pair of lines already correctly mentions XSync, XFlush, and drw_map, please either complete these first two lines to match the intended wording or remove them if they were an earlier draft.
π‘ What: Replaced
XSync(drw->dpy, False)withXFlush(drw->dpy)indrw_map(drw.c).π― Why:
XSyncforces the client to wait for the X server to process the request, creating a blocking round-trip. In high-frequency paths likedrawbar(which callsdrw_mapon every focus change, clock tick, or tag switch), this adds unnecessary latency and CPU overhead.π Impact: Reduces latency for every bar redraw. The UI will feel snappier, especially under load or with higher network latency (if remote).
π¬ Measurement: Verified build with
make clean && make. No visual regressions expected as X11 guarantees request ordering.PR created automatically by Jules for task 14763089325153825185 started by @paperbenni
Summary by Sourcery
Optimize X11 bar drawing by replacing a blocking XSync call with a non-blocking XFlush in drw_map to reduce redraw latency and CPU overhead.
Enhancements:
Documentation: