Bug 253612 - AX: Stop caching 9 isolated object properties unnecessarily
Summary: AX: Stop caching 9 isolated object properties unnecessarily
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tyler Wilcock
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-03-08 15:46 PST by Tyler Wilcock
Modified: 2023-03-10 10:27 PST (History)
10 users (show)

See Also:


Attachments
Patch (35.14 KB, patch)
2023-03-08 15:52 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (35.22 KB, patch)
2023-03-09 10:26 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tyler Wilcock 2023-03-08 15:46:01 PST
We have many properties that can be expressed purely in terms of AXCoreObject methods.
Comment 1 Radar WebKit Bug Importer 2023-03-08 15:46:12 PST
<rdar://problem/106456887>
Comment 2 Tyler Wilcock 2023-03-08 15:52:15 PST
Created attachment 465365 [details]
Patch
Comment 3 chris fleizach 2023-03-08 16:03:54 PST
Comment on attachment 465365 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=465365&action=review

did you measure any improvements from this one?

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:-1054
> -    AXCoreObject::AccessibilityChildrenVector tabs;

how come we don't need this?
Comment 4 Tyler Wilcock 2023-03-08 16:05:45 PST
(In reply to chris fleizach from comment #3)
> Comment on attachment 465365 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=465365&action=review
> 
> did you measure any improvements from this one?
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:-1054
> > -    AXCoreObject::AccessibilityChildrenVector tabs;
> 
> how come we don't need this?
The code builds `tabs` but never uses it, so I believe this is just wasted work unless you know of some reason to do this.
Comment 5 chris fleizach 2023-03-08 16:36:39 PST
Comment on attachment 465365 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=465365&action=review

>>> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:-1054
>>> -    AXCoreObject::AccessibilityChildrenVector tabs;
>> 
>> how come we don't need this?
> 
> The code builds `tabs` but never uses it, so I believe this is just wasted work unless you know of some reason to do this.

does tabChildren do some work in the background?
Comment 6 Tyler Wilcock 2023-03-08 17:53:18 PST
(In reply to chris fleizach from comment #5)
> Comment on attachment 465365 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=465365&action=review
> 
> >>> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:-1054
> >>> -    AXCoreObject::AccessibilityChildrenVector tabs;
> >> 
> >> how come we don't need this?
> > 
> > The code builds `tabs` but never uses it, so I believe this is just wasted work unless you know of some reason to do this.
> 
> does tabChildren do some work in the background?
The only side-effect it could have is calling AXCoreObject::children(updateChildrenIfNecessary = true), but I don't think that side-effect is needed here (and if it were, a comment should be added, and a test should fail if the side effect is removed).
Comment 7 Tyler Wilcock 2023-03-09 10:26:48 PST
Created attachment 465377 [details]
Patch
Comment 8 EWS 2023-03-10 00:20:34 PST
commit-queue failed to commit attachment 465377 [details] to WebKit repository. To retry, please set cq+ flag again.
Comment 9 EWS 2023-03-10 10:27:30 PST
Committed 261508@main (ae20ce99baa2): <https://commits.webkit.org/261508@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 465377 [details].