Skip to content

⚡ Optimize ATSPI window loading by batching N+1 queries - #45

Open
undivisible wants to merge 3 commits into
mainfrom
optimize-atspi-get-children-624884106141225388
Open

⚡ Optimize ATSPI window loading by batching N+1 queries#45
undivisible wants to merge 3 commits into
mainfrom
optimize-atspi-get-children-624884106141225388

Conversation

@undivisible

@undivisible undivisible commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

💡 What: Replaced the child_count() + get_child_at_index() loops with a single get_children() batch RPC call in src/linux_atspi.rs for loading accessibility applications and their active windows.
🎯 Why: The N+1 query pattern generates excessive D-Bus traffic. For an application with 100 windows, the old pattern required 101 blocking IPC calls (child_count + 100 get_child_at_index calls). The new approach requires exactly 1 blocking IPC call.
📊 Measured Improvement: I was unable to show a measured performance improvement because establishing a reliable ATSPI D-Bus mock service environment for benchmark tests requires significant architectural changes and mock definitions that far exceed the scope of the fix itself. However, moving from an N+1 D-Bus RPC pattern to a single batched IPC call is a definitive, unarguable performance optimization for any blocking client.


PR created automatically by Jules for task 624884106141225388 started by @undivisible


Note

Low Risk
Localized IPC optimization on window listing; behavior and safety checks are preserved aside from dropping an explicit negative child_count guard that get_children() does not expose.

Overview
Replaces per-window ATSPI D-Bus round trips when listing an application’s top-level windows. accessibility_focus_sentinel and windows() no longer call child_count() plus get_child_at_index() in a loop; they use one get_children() call and iterate the result.

Window caps still use MAX_WINDOWS_PER_APPLICATION on the returned slice length. Child binding, role/state filtering, and duplicate-surface checks are unchanged.

Reviewed by Cursor Bugbot for commit f1177a6. Configure here.

…lications.

Co-authored-by: undivisible <136312656+undivisible@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f1177a6eb1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/linux_atspi.rs
Comment on lines +1690 to +1691
let children = provider_call!(cancellation, deadline_at_ms, accessible.get_children())?;
if children.len() > MAX_WINDOWS_PER_APPLICATION {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bound child count before fetching the batch

When an application exposes an unusually large or malicious top-level accessibility list, get_children() materializes and deserializes the entire D-Bus response before MAX_WINDOWS_PER_APPLICATION is checked, so the 128-window limit no longer bounds memory or IPC work. The same new pattern also appears in the focus-sentinel path. Retain a bounded preflight such as child_count() before requesting the batch, or otherwise enforce the limit while decoding the response.

Useful? React with 👍 / 👎.

google-labs-jules Bot and others added 2 commits August 27, 2026 05:27
Co-authored-by: undivisible <136312656+undivisible@users.noreply.github.com>
Co-authored-by: undivisible <136312656+undivisible@users.noreply.github.com>
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