power: coalesce backlight helper writes, stop blocking on every step - #469
Open
rumours86 wants to merge 1 commit into
Open
power: coalesce backlight helper writes, stop blocking on every step#469rumours86 wants to merge 1 commit into
rumours86 wants to merge 1 commit into
Conversation
Every Screen.StepUp/StepDown call ran up to three synchronous g_spawn_command_line_sync() round-trips (get-brightness, get-max-brightness, then 'pkexec csd-backlight-helper --set-brightness' with a polkit round-trip), blocking the daemon's main loop for 60-400 ms per step. Keyboard autorepeat produces steps every ~33 ms, so holding a brightness key piled the calls up in the daemon's D-Bus queue: brightness kept 'running' for seconds after the key was released, and pressing the opposite key had no effect until the whole tail of the queue was processed. (Single presses were fine - one cycle completes before the next press.) Fix, following the approach gnome-settings-daemon took in gsd-backlight: - keep the last requested value (backlight_helper_target) and step from it instead of re-reading the hardware, answering the D-Bus call immediately; - write asynchronously (g_spawn_async + child watch) with a one-slot queue: at most one helper runs at a time, and when it finishes only the latest requested value is written. A burst of N steps now costs one or two helper invocations instead of N; - cache get-max-brightness - the maximum never changes at runtime, yet it was re-read through the helper on every single step; - GetPercentage reports the target while writes are in flight, so OSD and applet sliders stay in sync with what the user requested; - fix backlight_set_percentage falling through to the helper even after a successful xrandr write (the step functions already had the corresponding goto). Measured on a Mint 22.3 laptop (intel_backlight): a burst of 15 StepDown calls used to take ~1.4 s of serialized pkexec spawns and left the queue draining long after; now every call answers in ~1 ms, the burst results in exactly two helper writes (first + coalesced final value), and an opposite step right after the burst takes effect immediately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Author
|
Following the same standard requested on the cinnamon PRs: now verified against git master as well. Built master csd-power (with master schemas) and ran it live in place of the system daemon on Mint 22.3 / intel_backlight:
Previously posted numbers were from the same patch built at tag 6.6.4; behavior is identical on master. 🤖 Addressed by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug. Holding a brightness key makes brightness keep changing for seconds after the key is released, and pressing the opposite key has no effect until the backlog drains. Single presses behave fine.
Why. Every
Screen.StepUp/StepDownruns up to three synchronousg_spawn_command_line_sync()round-trips —csd-backlight-helper --get-brightness,--get-max-brightness, thenpkexec csd-backlight-helper --set-brightnesswith a polkit round-trip — blocking the daemon's main loop for 60–400 ms per step. Keyboard autorepeat produces a step every ~33 ms, so the calls pile up in the daemon's D-Bus queue and are processed strictly FIFO: brightness "runs on" after release, and an opposite press waits behind the whole accumulated tail.The fix (same approach gnome-settings-daemon took in
gsd-backlight):backlight_helper_target) and step from it instead of re-reading hardware; answer the D-Bus call immediately;g_spawn_async+ child watch) with a one-slot queue: at most one helper in flight, and on completion only the latest requested value is written — a burst of N steps costs 1–2 helper invocations instead of N;get-max-brightness(never changes at runtime, was re-read via the helper on every step);GetPercentagereports the target while a write is in flight, keeping OSD/applet sliders in sync with what the user asked for;backlight_set_percentagefalling through to the helper even after a successful xrandr write (step functions already had the correspondinggoto).Measured on Mint 22.3 (intel_backlight, max=120000), built at tag 6.6.4 and run live:
Helper-write log from the live test showing the coalescing:
(the g_message debug lines are not part of this PR)
🤖 Generated with Claude Code