DataGrid with Fluent theme and lots of columns is 10x slower removing from view

[garyhertel]

Describe the bug

When the Fluent theme is used, a DataGrid with lots of columns (20+) is very slow when removed from the UI. (about 20x slower than adding) Using the base Light theme is about 70x faster when removing the control.

To Reproduce

1. Clone https://github.com/garyhertel/AvaloniaDataGridPerfTest.git
2. Start App
3. Click "Toggle" button to toggle DataGrid on and off
4. Look at the Debug output for times
5. You can switch the theme in the App.xaml

A few notes:

• Hardcoding or resizing the MainWindow Width cuts the performance hit by 10x, but this is still significantly slower than the base theme. My app also relies on the DataGrid autosizing itself so I'm not sure this can be permanently hard coded.
• I'm using a widescreen monitor so the default window width is ~2800 pixels

Expected behavior

Using the Fluent theme and lots of DataGrid columns, the DataGrid removal should match the performance of the base light theme.

Alternatively, some workaround to speed up removal would help. (although other people will probably run into this problem too)

Screenshots

Desktop (please complete the following information):

• OS: Windows 10 Pro
• Version: Avalonia 0.10.15

Additional context

Timings for the sample app using using different themes:

Width=2864

Light Theme, resized and half of columns showing

DataGrid added, [time = 52ms ]
DataGrid removed, [time = 56ms ]

DataGrid added, [time = 0ms ]
DataGrid removed, [time = 40ms ]

DataGrid added, [time = 0ms ]
DataGrid removed, [time = 71ms ]

Light Theme, all columns showing

DataGrid added, [time = 57ms ]
DataGrid removed, [time = 152ms ]

DataGrid added, [time = 0ms ]
DataGrid removed, [time = 137ms ]

DataGrid added, [time = 0ms ]
DataGrid removed, [time = 134ms ]

Fluent Light, resized and half of columns showing

DataGrid added, [time = 58ms ]
DataGrid removed, [time = 669ms ]

DataGrid added, [time = 0ms ]
DataGrid removed, [time = 662ms ]

DataGrid added, [time = 0ms ]
DataGrid removed, [time = 661ms ]

Fluent Light, all columns showing

DataGrid added, [time = 57ms ]
DataGrid removed, [time = 9876ms ]

DataGrid added, [time = 0ms ]
DataGrid removed, [time = 9788ms ]

DataGrid added, [time = 0ms ]
DataGrid removed, [time = 9789ms ]

[maxkatz6]

Don't put your DataGrid or any other control with virtualization inside of the StackPanel.
DataGrid can't virtualize rows, when there is no limit by height. Replacing StackPanel with DockPanel solves the issue for me.

Additionally, I would still say that it's a bug that removing hundreds of non-virtualized rows freezes the app. But it was previously fixed and can't be reproduced on nightly build (0.10.999-cibuild0021377-beta). So I am going to close this issue.

[maxkatz6]

By the way, TreeDataGrid might be better choice for you, if you need to display lots of data fast, without editing it.
https://github.com/AvaloniaUI/Avalonia.Controls.TreeDataGrid

[garyhertel]

Thanks for the tip. It looks like TreeDataGrid has come a long ways since early Avalonia! I'll see if it can handle everything I can throw at it. :)

I also should have used a Grid in this demo since I don't really use StackPanel anywhere. It was just easier to use in this example. But it sounds like 0.11 might fix it.

[garyhertel]

So I updated my test and the repo to use a Grid instead of a StackPanel. (that was my bad) After running some more tests with the nightly nuget package it appears the issue is still there with the DataGrid and Fluent theme. The most interesting thing is the nightly throws an exception when removing the DataGrid, which I don't see in the current 0.10.15. I'm not sure if that's because there's extra debug logging with the nightly, or if it's a new regression in the nightly:

DataGrid added, [ time = 91ms ]
[Binding] Error in binding to 'Avalonia.Controls.Border'.'IsVisible': 'Null value in expression '!.$parent[DataGrid, 0]'.' (Border #60890569)
DataGrid removed, [ time = 1140ms ]

Here's some more numbers. I also added in some TreeDataGrid numbers, which does appear to be faster removing than the DataGrid with Fluent, but not faster than the DataGrid with the base Light Theme. There's probably quite a few features needed before I could switch to the TreeDataGrid, but it looks promising as an eventual option.

Avalonia 0.10.15, Fluent Theme:

Width=2864

DataGrid added, [ time = 87ms ]
DataGrid removed, [ time = 1008ms ]

DataGrid added, [ time = 0ms ]
DataGrid removed, [ time = 992ms ]

DataGrid added, [ time = 0ms ]
DataGrid removed, [ time = 1001ms ]

TreeDataGrid added, [ time = 51ms ]
TreeDataGrid removed, [ time = 251ms ]

TreeDataGrid added, [ time = 3ms ]
TreeDataGrid removed, [ time = 224ms ]

TreeDataGrid added, [ time = 2ms ]
TreeDataGrid removed, [ time = 187ms ]

Avalonia 0.10.15, Light Theme:
(TreeDataGrid only works with Fluent)

DataGrid added, [ time = 81ms ]
DataGrid removed, [ time = 55ms ]

DataGrid added, [ time = 0ms ]
DataGrid removed, [ time = 41ms ]

DataGrid added, [ time = 0ms ]
DataGrid removed, [ time = 45ms ]

Avalonia Nightly (0.10.999-cibuild0014036-beta), Fluent Theme:
(TreeDataGrid can't be used with nightly without extra work)

DataGrid added, [ time = 91ms ]
[Binding] Error in binding to 'Avalonia.Controls.Border'.'IsVisible': 'Null value in expression '!.$parent[DataGrid, 0]'.' (Border #60890569)
DataGrid removed, [ time = 1140ms ]

DataGrid added, [ time = 0ms ]
[Binding] Error in binding to 'Avalonia.Controls.Border'.'IsVisible': 'Null value in expression '!.$parent[DataGrid, 0]'.' (Border #37195075)
DataGrid removed, [ time = 1051ms ]

DataGrid added, [ time = 0ms ]
[Binding] Error in binding to 'Avalonia.Controls.Border'.'IsVisible': 'Null value in expression '!.$parent[DataGrid, 0]'.' (Border #38228382)
DataGrid removed, [ time = 989ms ]

DataGrid added, [ time = 0ms ]
[Binding] Error in binding to 'Avalonia.Controls.Border'.'IsVisible': 'Null value in expression '!.$parent[DataGrid, 0]'.' (Border #56165358)
DataGrid removed, [ time = 987ms ]

Avalonia Nightly, Light Theme:

DataGrid added, [ time = 106ms ]
DataGrid removed, [ time = 59ms ]

DataGrid added, [ time = 0ms ]
DataGrid removed, [ time = 55ms ]

DataGrid added, [ time = 0ms ]
DataGrid removed, [ time = 33ms ]

[grokys]

Profiling this on master I see the following obvious bottleneck:

30% of the time taken when removing the DataGrid from the tree is unsubscribing to Classes.CollectionChanged events. @MarchingCube and I have actually spoken about this bottleneck before: event subscribe/unsubscribe in .net is actually kinda slow beacuse it needs to be threadsafe. We only need to do this on the UI thread so we can speed this up pretty easily.

I'm going to write a few benchmarks and open a PR for this. An initial POC shows a reduction of time taken to detach the DataGrid on my machine from 1.2s to 0.273s.

[grokys]

Seems that the reason the above method is showing up so prominently in profiling is that the Window.DottedLineFocusAdorner :is(Control) style is adding thousands of handlers to Classes.CollectionChanged:

There are nearly 8000 class listeners being added to the window's Classes collection:

[maxkatz6]

As per discussion, out adorners aren't perf-optimal. But DottedLineFocusAdorner specifically is not used by many applications, disabled by defaul. And still causes perf degradation even if not used. Create PR to remove it.

[grokys]

Opened a PR to improve class selector performance #8433

[garyhertel]

Thanks for all the quick fixes everyone! This was the biggest blocker for the Fluent upgrade, so hopefully I can get that updated soon!