AXPosition::GetAnchor
Daniel Libby
It looks like AXPosition has always stored an AXNodeID and AXTreeID. When looking at some recent traces of Edge and Narrator (which is pretty heavy on TextRange/AXPosition usage), I noticed that over half the CPU samples were spent in AXPosition::GetAnchor (two hashtable lookups per call).
There’s probably some clean up we can do to remove redundant calls within the same function/stashing pointers in local variables/etc., but I was curious if there would be any issues with just storing an AXNode pointer directly on AXPosition? If we’re worried about lifetime issues, we can probably have it be a WeakPtr. WDYT? Am I missing any other concerns?
Daniel Libby
Found a couple of things that make this slower than it needs to be.
- We’re actually doing 5 hash table lookups for each GetAnchor call, 4 to the tree map, 1 for the node map. Each call to AXTreeManagerMap does two lookups (base::Contains + .at()) and could probably be improved to just do one (find + return iterator value). We’re also using the wrong flavor of BrowserAccessibilityManager::GetNodeFromTree where AXTreeManagerMap::GetManager is called again even though we’ve already looked up the tree to `this`
- AXTreeID operator== (to validate the hash lookup) is somewhat slow due to CRYTPO_memcmp for UnguessableToken (compares all 16 bytes, byte-by-byte presumably to harden against timing attacks)
Overall, GetAnchor in Edge on a recent Dev channel build takes 1375 instructions to execute:
I’ll put up a CL to reduce the overhead per the first point. I don’t think we can do anything about base::Token operator==.
Since WeakPtr access should be on the order of 10 instructions, I still want to explore converting AXPosition to have WeakPtr<AXNode> (it’ll also help debuggability), unless nektar@ has reservations.
From: Aaron Leventhal <aleve...@google.com>
Sent: Wednesday, December 15, 2021 11:16 AM
To: Daniel Libby <dli...@microsoft.com>
Cc: nek...@chromium.org; aleve...@chromium.org; chromium-ac...@chromium.org
Subject: Re: AXPosition::GetAnchor
Thanks for researching where we spend a lot of time! That's a shocking result.
Yes, lifetime issues are a concern. We've had crashes recently, and IIRC switched some of the Unserialize code to use AXIDs.
I guess we need nektar's input, but feel free to experiment with weak pointers.
... Shouldn't hash lookups be extremely fast? Are there some hash options that can make it faster?
On Wed, Dec 15, 2021 at 1:44 PM 'Daniel Libby' via Chromium Accessibility <chromium-ac...@chromium.org> wrote:
It looks like AXPosition has always stored an AXNodeID and AXTreeID. When looking at some recent traces of Edge and Narrator (which is pretty heavy on TextRange/AXPosition usage), I noticed that over half the CPU samples were spent in AXPosition::GetAnchor (two hashtable lookups per call).
There’s probably some clean up we can do to remove redundant calls within the same function/stashing pointers in local variables/etc., but I was curious if there would be any issues with just storing an AXNode pointer directly on AXPosition? If we’re worried about lifetime issues, we can probably have it be a WeakPtr. WDYT? Am I missing any other concerns?
--
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-accessib...@chromium.org.
