Take the per-keystroke focus refresh off the event-tap path#316
Open
FuJacob wants to merge 1 commit into
Open
Conversation
handleInputEvent ran a full synchronous AX resolve (focusModel.refreshNow) inside the CGEvent tap callback on every keystroke. macOS withholds the keystroke from the focused app until the callback returns, so the resolve time is added directly to typing latency. The cost spikes when the caret is mid-text: the resolver can't get an exact caret from a single BoundsForRange call and falls back to the deep-tree caret walk plus child text-run walks, all cross-process AX IPC, which the user feels as heavy typing lag. Drop the eager refresh from the three event-tap sites. Focus is still refreshed off the hot path: generateFromCurrentFocus re-reads it after the debounce, apply refreshes again as the stale-guard, and the 80ms poll keeps it warm. Requests are still built from a fresh snapshot.
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.
Summary
Typing lagged heavily — especially when editing in the middle of existing text — because
handleInputEventran a full synchronous Accessibility resolve (focusModel.refreshNow()) inside the CGEvent tap callback on every keystroke. macOS withholds the keystroke from the focused app until the tap callback returns, so the resolve time is added directly to typing latency. The cost spikes mid-text: the geometry resolver can't get an.exactcaret from a singleBoundsForRangecall and falls back to the deep-tree caret walk (resolveDeepGeometrySource, up to 200 nodes) plus child text-run walks (up to 300 nodes), all cross-process AX IPC. This change removes the eager refresh from the three event-tap sites so that work no longer blocks each keystroke.Focus is still refreshed off the hot path, so requests are still built from a fresh snapshot:
generateFromCurrentFocusre-reads focus after the debounce (SuggestionCoordinator+Prediction.swift:47)applyrefreshes again as the stale-result guard (:142)FocusTracker.swift)Validation
The change only removes a side effect on the synchronous path; no new logic to unit test. Recommend a manual check: type in the middle of a long Gmail / Slack / Chrome compose box (the worst case) and confirm typing is smooth.
Linked issues
Risk / rollout notes
schedulePredictionnow reads a snapshot that may be up to ~80ms stale (poll interval) instead of freshly resolved;generateFromCurrentFocusre-validates against a fresh snapshot before generating, so this is benign.Greptile Summary
This PR removes three synchronous
focusModel.refreshNow()calls that were executing inside the CGEvent tap callback on every keystroke, adding directly to typing latency. The AX resolve is expensive (potentially tens of milliseconds) and was blocking the OS from delivering each keystroke to the target app.handleInputEvent(_:)path and both the.textMutationand.shortcutMutationbranches of the active-session overload, matching everyschedulePrediction()call site on the event-tap path.generateFromCurrentFocuscallsrefreshNow()after the debounce,apply(result:workID:)calls it again as a stale-result guard, and the 80ms background poll inFocusTrackerkeeps the snapshot warm between events.schedulePrediction()guard inhandleInputEventnow reads a snapshot that may be up to ~80ms stale; sincegenerateFromCurrentFocusre-validates with a fresh snapshot before generating, the worst-case outcome is a redundant debounce task that is silently dropped.Confidence Score: 5/5
Safe to merge. The change removes a redundant side effect on the synchronous event-tap path; every downstream consumer that actually needs a fresh snapshot already obtains one independently.
The removed calls were genuinely redundant: generateFromCurrentFocus calls refreshNow() after the debounce, apply(result:workID:) calls it again before promoting a result, and the 80ms background poll keeps the snapshot warm between events. The only observable difference is that the schedulePrediction() availability gate now reads a snapshot that may be up to ~80ms stale, which at worst causes a debounce task to be scheduled and then silently dropped when the fresher snapshot is obtained. No logic is added or restructured, and the inline comments are accurate and thorough.
No files require special attention. SuggestionCoordinator+Prediction.swift was not changed but contains the two remaining refreshNow() calls that validate the PR's safety claims — those are untouched and behave as before.
Important Files Changed
Sequence Diagram
sequenceDiagram participant OS as macOS participant Tap as CGEvent Tap (handleInputEvent) participant SC as SuggestionCoordinator participant FT as FocusTracker (80ms poll) participant GFC as generateFromCurrentFocus (async, after debounce) OS->>Tap: keystroke (event tap blocked until return) Note over Tap: BEFORE: refreshNow() here → AX IPC, tens of ms lag Tap->>SC: schedulePrediction() reads stale snapshot (≤80ms old) Tap-->>OS: return (keystroke delivered immediately) FT-->>SC: snapshot warm (80ms poll) SC->>GFC: debounce fires (~300ms later) GFC->>GFC: refreshNow() — fresh AX snapshot GFC->>GFC: SuggestionAvailabilityEvaluator check GFC->>GFC: build and send request Note over GFC: apply(result:) calls refreshNow() again as stale-result guardReviews (1): Last reviewed commit: "Take the per-keystroke focus refresh off..." | Re-trigger Greptile