Bug 247735 - AX: Add logging of the AX trees on a timer for debugging purpose.
Summary: AX: Add logging of the AX trees on a timer for debugging purpose.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Andres Gonzalez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-11-10 08:04 PST by Andres Gonzalez
Modified: 2022-11-15 05:37 PST (History)
15 users (show)

See Also:


Attachments
Patch (26.16 KB, patch)
2022-11-10 08:42 PST, Andres Gonzalez
andresg_22: review?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andres Gonzalez 2022-11-10 08:04:03 PST
Useful to compare live and isolated trees.
Comment 1 Radar WebKit Bug Importer 2022-11-10 08:04:17 PST
<rdar://problem/102194134>
Comment 2 Andres Gonzalez 2022-11-10 08:42:41 PST
Created attachment 463478 [details]
Patch
Comment 3 Tyler Wilcock 2022-11-14 10:38:14 PST
My two concerns with this patch are:

  1. It dumps the isolated tree on the secondary thread (rather than main-thread) unconditionally. This is a problem because I find that most webpages are way too slow to be operable with AXLoggingOptions::OffMainThread enabled. My non-upstreamed timer dump patch forces a tree dump on the main-thread when it has detected an idle state (meaning that the tree dump isn't *strictly* thread-safe, but thread-safe enough for what it needs to do).

  2. It requires something to run AXIsolatedTree::applyPendingChanges to dump the isolated tree, rather than the aforementioned forced tree dump directly based on the timer. My personal workflow is to get pages into a state where they are broken, and then let the forced dump run without any other VO actions to ensure I don't further change the page state or otherwise muddy the logging.

My tree dump patch is obviously not upstreamable, but keeping it around locally[1] and having to rebase it every now and then is not a big deal for the utility it provides.

[1]: I keep it as a git stash, and git stash apply as needed
Comment 4 Andres Gonzalez 2022-11-15 05:37:35 PST
(In reply to Tyler Wilcock from comment #3)
> My two concerns with this patch are:
> 
>   1. It dumps the isolated tree on the secondary thread (rather than
> main-thread) unconditionally. 

That's where the iso tree should be accessed, should not be accessed on the main thread.

> This is a problem because I find that most
> webpages are way too slow to be operable with
> AXLoggingOptions::OffMainThread enabled. 

We should address that by providing finer granularity for what to log, i.e., don't log object wrapper stuff, not just on/off main thread.

> My non-upstreamed timer dump patch
> forces a tree dump on the main-thread when it has detected an idle state
> (meaning that the tree dump isn't *strictly* thread-safe, but thread-safe
> enough for what it needs to do).

We can add that hack to this but it is just an illusion that we can detect any kind of idleness in the iso tree. An actual solution here could be to dispatch the dump request to the secondary thread.

> 
>   2. It requires something to run AXIsolatedTree::applyPendingChanges to
> dump the isolated tree, rather than the aforementioned forced tree dump
> directly based on the timer. 

We care about the state of the iso tree after applying pending changes, so that's where we want to dump the tree.

> My personal workflow is to get pages into a
> state where they are broken, and then let the forced dump run without any
> other VO actions to ensure I don't further change the page state or
> otherwise muddy the logging.

We can make that work with the right approach of dispatching the dump to the secondary thread. This is a step in that direction.