Repository navigation
Conversation
|
Looks great! That's one big performance improvement 👌🏻 |
This branch has not been deployed
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.
Querying the message list through accessibility freezes the app for seconds once a few thousand messages are loaded.
HistoryInner::accessibleElements()walks every history block and builds a new vector on each call.accessibilityChildCount(),accessibilityChildIdentity(),accessibilityChildState()and the other per-row methods each call it, andaccessibilityUnreadBarIndex()builds it once more.Ui::Accessible::Itemasks several of these for every row while the platform lists the children, so listing n rows costs O(n²) on the main thread.HistoryView::ListWidgetgot the same parallel implementation in #30948 and has the same cost.To fix this, this PR keeps the built list until the rows change.
HistoryInnerdrops it inviewRemoved()andrecountHistoryGeometry(), andHistoryView::ListWidgetinviewReplaced()andupdateItemsGeometry(), so the list never holds a destroyed view. The unread bar index is cached for the current bar element. Methods that move focus or scroll still take a copy, because scrolling can load or unload messages while they run.Tested on macOS 26 (arm64) with v7.2.9 (the patch applies to dev as is), in a group with 6705 loaded messages, querying the message list with the macOS accessibility API:
AXChildrencountAXChildrenThe rows returned match before and after: the same count, and the same hash over the role, description and selection of the newest 5000 rows.
To reproduce, load a few thousand messages in a chat and ask its message list for
AXChildren. Any client that lists the window's children does this. With an app switcher reading windows in the background (AltTab 11.1.0), the app froze for about 2 s every few seconds while typing.