JBR-10147 Optimize performace of CAccessibility.getChildrenAndRolesRecursive for JTree#599
Open
dmitrii-drobotov wants to merge 1 commit intoJetBrains:jbr25from
Open
Conversation
…cursive for JTree * Use tree.getSelectionPaths() for JAVA_AX_SELECTED_CHILDREN * For JAVA_AX_ALL_CHILDREN and JAVA_AX_VISIBLE_CHILDREN also use linear iteration to avoid expensive operations with arrays on each level * Flip comparison order for accessible states to not call getAccessibleStateSet() when not needed because it can be expensive
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.
This PR speeds up the
CAccessibility.getChildrenAndRolesRecursivemethod for JTrees.From my testing, it may be called in the following cases:
JAVA_AX_ALL_CHILDRENwhen VoiceOver or Full Keyboard Access are enabled and interacting with a tree, usually on only the first focusJAVA_AX_SELECTED_CHILDRENwhen one of interactive assistive tools or settings is enabled, such as VoiceOver, Zoom, VoiceControl, Hover Text, etc. It's called on focus of the tree and on every selection change. Also, for me when all known assistive tools were turned off, it was sometimes called on mouse clicks on nodes, on tree focus, and node expansion - it could be coming from a non-accessibility related OS tool or setting, or from a third-party app that uses accessibility APIs.JAVA_AX_VISIBLE_CHILDRENwhen Voice Control was enabled and listening.The approach is based on ffa33d7 with the following changes:
AccessibleJTreeNodefromTreePath(which had issues described in https://bugs.openjdk.org/browse/JDK-8329667?focusedId=14668819&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14668819), I usedgetPathBoundsandgetAccessibleAt.allChildren.add(String.valueOf((tree.isRootVisible() ? path.getPathCount() : path.getPathCount() - 1)));toallChildren.add(String.valueOf(tree.isRootVisible() ? path.getPathCount() - 1 : path.getPathCount() - 2));. This way it matches the level from the code path below. For example, if the root is visible and the path is [Root], its level should be 0 (== count - 1), for path is [Root, Node1] the level needs to be 1 and so on. If root is not visible, it's still included in the path, so levels are decreased by 2 from the count.JAVA_AX_VISIBLE_CHILDRENbecause it's used by Voice Control.The main improvement between the existing code is with
JAVA_AX_SELECTED_CHILDRENargument. Previously, the code was iterating the whole tree to find selected nodes, now it gets them instantly. ForJAVA_AX_ALL_CHILDRENandJAVA_AX_VISIBLE_CHILDRENthere is also an improvement because we avoid a lot ofclear,add,addAlloperations with arrays. From my basic benchmark on 50k nodes tree, forJAVA_AX_SELECTED_CHILDRENthe time went from 6s to <1ms, forJAVA_AX_ALL_CHILDRENfrom 6s to 1.5s and forJAVA_AX_VISIBLE_CHILDRENfrom 6s to 2s. If we still get reports of freezes after this fix, we can try adding a limit (e.g., 1000) to the the returned children count, but for now I think this is already a good improvement.The
allowIgnoredargument is not handled because for trees (NSAccessibilityOutlineRole) it's always passed as true: https://github.com/JetBrains/JetBrainsRuntime/blob/jbr25/src/java.desktop/macosx/native/libawt_lwawt/awt/a11y/CommonComponentAccessibility.m#L686Another change is flipping the order of comparison of accessible states in the code path below (for non-JTrees).
cac.getAccessibleStateSet()can be expensive and this way we skip it ifwhichChildren == JAVA_AX_ALL_CHILDREN.