wpf
wpf copied to clipboard
dotnet
Fix ListView/GridView scroll performance when automation peers are created
Fixes #9181
Description
I was running some tests with ListView (using GridView) and when I loaded 1 000 000 items (yes, an obscure use case but that's why we got virtualization, no?), the scrolling performance would degrade and keyboard scrolling felt almost unusable.
During a quick rundown, I've discovered the culprit is in override of GetChildrenCore of GridViewItemAutomationPeer, which performs IndexOf (linear) search in the Items collection of the underlying ItemsSource to find the respective item's Row.
Reproduction Steps
The easiest step to reproduce is to add a ListView with GridView plus a ComboBox, after opening the ComboBox's dropdown (clicking it), Accessibility.dll gets loaded and the party gets started. You can find a demo repository on Github - here.
Behavior
The scrolling performance becomes worse to unusable with increasing number of items.
Known Workarounds
The "workaround" is to subclass GridView, overriding GetAutomationPeer to return NULL. Not really a solution though.
Risk
None.
Microsoft Reviewers: Open in CodeFlow
Tested?
Code changes look good to me. Would it make sense to have a reference to something holding the cached row number so that it doesn't have to be searched for each cell in a row?
@miloush Yeah, tested, it works as expected.
It would definitely make more sense to cache the row index for all the cells, however I don't see an easy way how to do it here, given that we would load the index lazily.
@miloush I have actually thought of a much simpler solution that didn't occur to me before, we can just retrieve the index from the realized item in ItemGenerator. That way the change is pretty much a one-liner.
I have also thought about your caching proposal, and my original solution does actually ruin the UAI performance with multiple columns indeed. This solution on the other hand is rather smooth.
For one liner I think it would appreciated if the PR didn't contain all the other stylistic changes.
Agreed, I'll push a commit with only the one-liner.
No, ItemGenerator contains always just the realized items (block) and it does a linear search but only in the bunch, so in case of virtualization that's only a handful few that are currently realized (it has a starting index). I use it a lot in other projects to draw over controls so I'm aware of the performance. I ran it with the build of the new PresentationFramework as well just to be sure before committing.
Hello, i'm following this conversation since the start because i ran into this issue. I was very glad when @h3xds1nz came up with a solution and seeing the progress on the reviews. Now the downside, there is not much activity since 30th may. Is there a way upvote / make the owner aware of this issue please? When "Accessibility.dll" load it greatly impact the datagrid /listview negatively and it doesn't require 1 000 000 items to notice it. In my usecase, it happen with100
@h3xds1nz I was trying to check your sample repro by replacing the locally built PresentationFramework.dll and could experience similar scrolling performance. Couldn't observe something noticeable. I might be missing a step here, Could you please share the exact steps to verify?
@singhashish-wpf Thanks for checking, I've included sample videos for you with the repro that I've originally recorded. First I'm regularly scrolling when peers are not being created.
Notice what happens in the first/original video when Accessibility gets loaded (VS output) - e.g. clicking dropdown, and automation peers start being created. The scrolling up that takes like 250ms for each item (because for each item in the view it does linear search (IndexOf) in the entire Items collection) is me actually holding Up arrow key.
Same goes for the scrollbar being laggy after I reach the bottom.
By the way, the fact that automation peers are created after interacting in with dropdowns, popups, even though there's no client subbed, that's a long-standing issue since the beginning, I believe there's a tracking for this as well in this repo but that's just a side note.
@harshit7962 Thanks for including it, finally can use UIA again in the upcoming project.