Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating many UI elements allocates unreasonable amount of memory and consumes too much cpu #17552

Open
JakeIn2l opened this issue Nov 18, 2024 · 14 comments
Labels

Comments

@JakeIn2l
Copy link

JakeIn2l commented Nov 18, 2024

Describe the bug

Consider the following markup

<ItemsControl ItemsSource="{CompiledBinding TestChannels}">
		<ItemsControl.ItemsPanel>
			<ItemsPanelTemplate>
				<WrapPanel/>
			</ItemsPanelTemplate>
		</ItemsControl.ItemsPanel>
		<ItemsControl.ItemTemplate>
			<DataTemplate>
				<Border Padding="8" Margin="8" Classes="panel3">
					<StackPanel>
						<TextBlock Classes="h3 centerH" Text="{CompiledBinding Channel}"/>
						<StackPanel Margin="0,8,0,0" Orientation="Horizontal">
							<Button Margin="1"  Content="1"/>
							<Button Margin="1"  Content="-"/>
							<Button Margin="1" Content="0"/>
						</StackPanel>
						<TextBox Margin="0,8,0,0"/>
					</StackPanel>
				</Border>
			</DataTemplate>
		</ItemsControl.ItemTemplate>
	</ItemsControl>

The collection TestChannels has over 500 elements. When this view is loaded, over 300MB is allocated. It also takes upwards of 6 seconds for the view to load with a 5ghz cpu core pegged at 100%.

To Reproduce

Create a view as above and create a simple view model of any type with a collection of basic objects bounded to the view.

Expected behavior

View should load nearly instantly and not consume so many resources.

Avalonia version

11.2.1

OS

No response

Additional context

A variety of layouts were tested including stackpanels and just normal panels, all exhibited the issue. Removing style classes does not help. I've reduced the view model code down to basic objects, to confirm it wasn't a impact. I'm aware I can use virtualization but in this case these elements span an ultra wide screen and all need to be displayed at once. Doing the equivalent in a web stack does not yield any complications.

After doing some memory profiling, looks like all the objects allocated are style related instances.

@JakeIn2l JakeIn2l added the bug label Nov 18, 2024
@thevortexcloud
Copy link
Contributor

Have you tested a virtualised panel?

but in this case these elements span an ultra wide screen and all need to be displayed at once.

I don't understand. Why does screen size make a difference? What do you think virtualisation does?

@maxkatz6
Copy link
Member

WrapPanel is not virtualized. Each control has to be in the memory for it to work, high memory consumption is expected.
VirtualizedStackPanel will reuse components, and consume as little memory, as necessary to display currently visible elements.

@maxkatz6
Copy link
Member

I couldn't find whether we already had a feature request for VirtualizedWrapPanel (or VirtualizedUniformGrid). But that's a solution we would want to have.

@thevortexcloud
Copy link
Contributor

There was actually a PR to add one ages ago. But it was rejected for not handling different sized items.

#11251

@JakeIn2l
Copy link
Author

JakeIn2l commented Nov 18, 2024

Have you tested a virtualised panel?

but in this case these elements span an ultra wide screen and all need to be displayed at once.

I don't understand. Why does screen size make a difference? What do you think virtualisation does?

@thevortexcloud You are misunderstanding, sorry if I wasn't clear. All 500 or so elements will be visible all at once. The application is intended to be used on a large screen where this is possible and also the desired end product for our customers. Virtualization does not help here. Virtualization is for when elements scroll out of view. In this case, there is no scrolling because all the elements are visible on a large screen.

Also, your comment comes off a bit rude, like a superiority complex. Please be more civil next time.

@JakeIn2l
Copy link
Author

JakeIn2l commented Nov 18, 2024

@maxkatz6 Yes I would expect a hit to memory, but not a hit this big. The memory hit is however, the least of our concerns. The loading time is the killer. Our requirements are that all 500 elements are visible at once. I was undecided originally whether to mark this as a bug or enhancement, but after having no issue when testing the same layout in react/web and also NoesisGui, (instant load time) I decided to mark as bug. Sorry if that is not the best classification. I would also be curious of the performance for such a use case in WPF. I've had issues with other xaml frameworks displaying this many items at once, but this seems like the low end of what I've seen.

@maxkatz6
Copy link
Member

maxkatz6 commented Nov 18, 2024

As a possible optimization, you should avoid adding TextBox in every cell by default. It's a relatively heavy control, which doesn't make sense to be added by default everywhere. Our TextBox template is also more complex compared to WPF, as it includes errors visualization, left and right inner contents, floating watermarks, most of which is included in the XAML tree, even when not needed (which is definitely a room for improvements).

Instead, you can keep it as a TextBlock, and replace with TextBox when user focuses this cell or click on it. This technique is used in DataGrids for both WPF and Avalonia too.

@JakeIn2l
Copy link
Author

As a possible optimization, you should avoid adding TextBox in every cell by default. It's a relatively heavy control, which doesn't make sense to be added by default everywhere.

Instead, you can keep it as a TextBlock, and replace with TextBox when user focuses this cell or click on it. This technique is used in DataGrids for both WPF and Avalonia too.

Good advice, we tried that and it definitely helped but its still too slow. Basically the layout can only reach acceptable performance if only textblocks are present.

As an aside and at the risk of being off topic, we've already made considerable efforts to have Avalonia fit in our use case, we've for example implemented our own custom scroll panel to have better touch support and our own virtualization panels (not for this issue fyi), so we've been hammering at the optimizations for a while now. I think we are just hitting the limits of Avalonia for what we are trying to do. I think the issue is just overall performance.

@maxkatz6
Copy link
Member

There shouldn't be as big difference in overall performance between WPF and Avalonia. Layout engine is similar, compositing engine is similar (but in C#), many virtualization parts were ported directly from WPF wit adjustments, as well as some controls like Grid. There definitely can be specific areas and controls with worse performance, with TextBox as a good example.

Another big difference, that many developers might hit, is that WPF assemblies are (or at least were in .NET Framework) pre-jitted with NGen/R2R. Which typically solved by using R2R or AOT in Avalonia.

@maxkatz6
Copy link
Member

Other than that, my other recommendations, without going way too deep into custom scrolls/virtualization:

  1. You can avoid at least one nesting layer per each item, by avoiding ItemTemplate infrastructure. And instead override items control and override containers logic. Templates infrastructure adds overhead on its own. See ListBox and recycle container content and layout performance #17511 (comment) recent discussion on this topic.

  2. Aside from TextBox, your next relatively complex control is Button, depending on customizations and interactions you need, you can replace ControlTheme with a simple TextBlock inside.

  3. Any samples and or dotMemory/dotTrace profiling would greatly help too, so we won't blindly search for performance issues, and could pinpoint specific areas.

@JakeIn2l
Copy link
Author

Thanks Max. I will dive deeper tomorrow and report back. Quick profiling revealed that most of the memory allocations were from classes pertaining to styling be instantiated. I need to either tweak the cpu profilier / debug settings or include from source to get proper cpu profiling because my profiler isn't showing where the cpu is being used up.

The first thing I will try is removing the item template infrastructure, that is something I haven't thought of.

@maxkatz6
Copy link
Member

maxkatz6 commented Nov 19, 2024

Quick profiling revealed that most of the memory allocations were from classes pertaining to styling be instantiated

Do you have many styles in your app that might be applied to Button/TextBox/Border? Even if it's not always matched. It could be a reason. Especially something like ":is(Control).my-class" will force styling system to go through every control in the app.

@JakeIn2l
Copy link
Author

JakeIn2l commented Nov 22, 2024

The app does use complex styles, but only for custom controls. We have custom textboxes, custom buttons, ect. So for this issue in our app, all our styles apply only to those custom controls. The built-in controls thus only whatever the basic theme provide. This issue is reproducible with the built-in controls with no styling. The code I included in this issue is with our custom controls substituted with the built-in ones. There was no noticeable change is performance.

I'm not sure when I can get you more information.

@maxkatz6
Copy link
Member

maxkatz6 commented Nov 22, 2024

We have custom textboxes, custom buttons, ect. So for this issue in our app, all our styles apply only to those custom controls.

Do you use ControlTheme for these custom controls? It would be our recommendation.

This issue is reproducible with the built-in controls with no styling

I see. It's still useful to reduce noise in the profiler for this repro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants