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

[Bug] Button memory leak #245

Closed
1 task done
gubard opened this issue Jul 7, 2024 · 12 comments
Closed
1 task done

[Bug] Button memory leak #245

gubard opened this issue Jul 7, 2024 · 12 comments
Labels
bug Something isn't working

Comments

@gubard
Copy link

gubard commented Jul 7, 2024

Check the following items

  • I have looked up relevant Issue

Description of the issue

I will create an application in the interface in which many buttons are used. While running the application, I noticed that every time the interface is updated, the amount of RAM required increases. After some research, I found that whenever the application needs to render a button, the necessary amount of memory increases. Also, I checked FluentTheme, it doesn't have similar behavior. I have created a repository with a project that reproduces this problem https://github.com/gubard/AvaloniaItemsControlMemoryLeak. Just run the project and press the start button.

Package Version

6.0.0-beta7

Environment

Windows 11 Pro 23H2 22631.3810 and Garuda Linux Bird of Prey x86_64

Expected Behavior

The amount of required RAM should not increase each time the button is rendered

Reproduction

Rendering the button and then clearing it. And so constantly

Additional Information

No response

@gubard gubard changed the title Button memory leak [Bug] [Bug] Button memory leak Jul 7, 2024
@AuroraZiling AuroraZiling added the bug Something isn't working label Jul 7, 2024
@YJammak
Copy link
Contributor

YJammak commented Jul 31, 2024

The memory leak is caused by the use of animation,There is a Loading inside the Button, and Loading uses animation. This should be a bug in Avalonia.

@JETomi
Copy link

JETomi commented Aug 1, 2024

I get the same issue when testing with borders.

        [ObservableProperty]
        ObservableCollection<Control> controls = new ObservableCollection<Control>();

        public MainViewModel()
        {
            System.Timers.Timer aTimer = new System.Timers.Timer();
            aTimer.Elapsed += new ElapsedEventHandler(OnTimedEvent);
            aTimer.Interval = 500;
            aTimer.Enabled = true;
        }
        private void OnTimedEvent(object source, ElapsedEventArgs e)
        {
            Dispatcher.UIThread.InvokeAsync(() => {
                Controls.Clear();
                for (int i = 0; i < 100; i++)
                {
                    //Controls.Add(new Button() { Content = "Button" });
                    Controls.Add(new Border() { Width = 100, Height = 100, Background = Brushes.Blue });
                }
            });
        }

@YJammak
Copy link
Contributor

YJammak commented Aug 1, 2024

I get the same issue when testing with borders.

        [ObservableProperty]
        ObservableCollection<Control> controls = new ObservableCollection<Control>();

        public MainViewModel()
        {
            System.Timers.Timer aTimer = new System.Timers.Timer();
            aTimer.Elapsed += new ElapsedEventHandler(OnTimedEvent);
            aTimer.Interval = 500;
            aTimer.Enabled = true;
        }
        private void OnTimedEvent(object source, ElapsedEventArgs e)
        {
            Dispatcher.UIThread.InvokeAsync(() => {
                Controls.Clear();
                for (int i = 0; i < 100; i++)
                {
                    //Controls.Add(new Button() { Content = "Button" });
                    Controls.Add(new Border() { Width = 100, Height = 100, Background = Brushes.Blue });
                }
            });
        }

I don't know what control you use to display the Border list. It is possible that the control uses animation when displaying the Item.

@JETomi
Copy link

JETomi commented Aug 1, 2024

This is my XAML code.

<suki:SukiWindow xmlns="https://github.com/avaloniaui"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
             xmlns:vm="clr-namespace:**.UI.Views"
             x:DataType="vm:MainViewModel"
				 xmlns:suki="clr-namespace:SukiUI.Controls;assembly=SukiUI"
		mc:Ignorable="d" d:DesignWidth="800" d:DesignHeight="450"
        x:Class="**.UI.Views.MainView"
        Icon="/Assets/avalonia-logo.ico"
        Title="**.UI">
	<Design.DataContext>
		<!-- This only sets the DataContext for the previewer in an IDE,
         to set the actual DataContext for runtime, set the DataContext property in code (look at App.axaml.cs) -->
		<vm:MainViewModel />
	</Design.DataContext>
	<StackPanel>
		<ItemsControl ItemsSource="{Binding Controls}"/>
	</StackPanel>
</suki:SukiWindow>

@JETomi
Copy link

JETomi commented Aug 1, 2024

I tried the same approach on a 'vanilla avalonia project' and the same issue exists.
So doesn't look like a theme specific problem.

@AuroraZiling AuroraZiling added the upstream Issues happened on upstreams (Avalonia, Skiasharp...) label Aug 6, 2024
@AuroraZiling
Copy link
Collaborator

Avalonia 11.1.3 with latest SukiUI CI dll

Testing reveals that the vanilla Avalonia project(repo in issue description) now does not suffer from memory leaks

@AuroraZiling AuroraZiling removed the upstream Issues happened on upstreams (Avalonia, Skiasharp...) label Aug 14, 2024
@kikipoulet
Copy link
Owner

@AuroraZiling

Can we consider the issue fixed ?

@AuroraZiling
Copy link
Collaborator

This issue still exists (6.0.0-beta8)
With SukiUI:
image

With Fluent:
image

@kikipoulet
Copy link
Owner

@YJammak was completely right. It seems to be indeed the Loading :

With Loading :

{41210947-749E-4423-A076-70AC171CCBF5}

Without Loading :

{4E91A7B9-7C83-4422-A6A3-A16378C6BAE4}

Fluent Theme :

{DD50F133-4717-47F0-A3FC-4E191610A82B}

Obviously the little difference between Suki and Fluent Theme is caused by the more complex tree + effect, but at least memory is cleaned correctly when there is no Loading. Let's investigate the Loading control then 👍

@kikipoulet
Copy link
Owner

kikipoulet commented Oct 4, 2024

The animation is 100% causing this.

<Style Selector="suki|Loading">
            <Style.Animations>
                <Animation FillMode="None"
                           IterationCount="Infinite"
                           PlaybackDirection="Normal"
                           Duration="0:0:1.5">
                    <Animation.Easing>
                        <QuadraticEaseInOut />
                    </Animation.Easing>
                    <KeyFrame Cue="0%">
                        <Setter Property="RotateTransform.Angle" Value="0" />
                    </KeyFrame>
                    <KeyFrame Cue="30%">
                        <Setter Property="RotateTransform.Angle" Value="0" />
                    </KeyFrame>

                    <KeyFrame Cue="100%">
                        <Setter Property="RotateTransform.Angle" Value="360" />
                    </KeyFrame>
                </Animation> 

            </Style.Animations>
   </Style>

Removing this infinite animation resolve the bug. So .. It seems to mean that using infinite animation trigger a memory leak. As this is a standard Avalonia use, I suppose we should raise an issue in the Avalonia repo.

Behavior in an empty avalonia project :

memoryleak


However, it is quite critical as a Loading control is embedded in every button. @sirdoombox, can you replace the Loading control by your LoadingTest ? I already tested it and it works like a charm an solve the bug, the only difficulty is to be able to set the foreground property and I don't want to make a mess in your "SukiEffect" code 🥲 there's no hurry though

@sirdoombox
Copy link
Collaborator

I should have some time to look into this tomorrow. It should be relatively easy to add some extra uniforms for setting colours, though I didn't mess around with that loading control much beyond just showing it was possible.

@sirdoombox
Copy link
Collaborator

Fixed in #297

(Tested with 1000 buttons, memory usage is flat)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants