dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.1k stars 1.17k forks source link

WPF TextBox stutters on update #5887

Open JensNordenbro opened 2 years ago

JensNordenbro commented 2 years ago

image

The Go button starts an update of the value Data bound to all TextBox:es according to:

image

The UI freezes quite quickly although I have ample of system resources:

Processor Intel(R) Core(TM) i7-10700 CPU @ 2.90GHz 2.90 GHz Installed RAM 64,0 GB (63,8 GB usable) System type 64-bit operating system, x64-based processor

During net framework era we have employed a trick that adds memory pressure and garbage collects "all the time" so that a bug garbage collect that seems to cause the freeze does not need to happen. This however is not a good long term solution: (The "solution" was initially described in http://connect.microsoft.com/VisualStudio/feedback/details/376495/wpf-sporadically-slow-textbox-text-anystring that is no longer available. )

image

This can be tested by checking the checkbox "UseUglyMitigation" in the first image.

Repro by running the attached application.

WpfTextBoxPerfIssue.zip

miloush commented 2 years ago

You can get slightly better results by using background/server GC.

TextBox is a heavy control, why do you need to use them in this scenario as opposed to TextBlocks? You can't really have users select and edit values if they keep changing every 100ms...

Otherwise if being interrupted by GC is not acceptable than perhaps managed runtime is not the best tool for the task.

JensNordenbro commented 2 years ago

Well, I lost my time machine to be able to unselect the choice of a Managed language often times used to code games without stutter.

It is true that you practically can't edit while values are changing but the components are not always changing rapidly and the end user needs to be able to change them manually from time to time and I guess that is why a textbox was choosen. However, The ability to track a click to Edit should not mean bad performance. My scenario is not a performance intensive one considering all games etc being written in c#. I do NOT even have a lot components in the example . Obviously something should be considered wrong and it would benefit a lot of People to fix them!

cwensley commented 1 year ago

We are also now running into this issue (we've migrated some MFC code over to WPF) and is causing our 3D view to stutter when a user pans/rotates/etc since the camera locations that are being tracked in a few text boxes (which they can edit manually at any time). The whole application can lock up for upwards of 2-4 seconds every so often which is not a very good user experience.

In doing a little debugging I found it was due to an apparent deadlock that occurs every so often with the how WPF interacts with the thread input manager. I did find that the following registry setting can avoid the issue (which seems better than a GC hack, that actually causes worse performance in our case):

[HKEY_CURRENT_USER\Software\Microsoft\CTF]
"Disable Thread Input Manager"=dword:00000001

With this setting there is no stutter at all, perhaps @JensNordenbro you can verify this on your end too.

What I'd like to know is, if our software sets the above registry value when installed what are the consequences? What feature(s) would be lost? Is there any progress on this issue, any hints at what we can do to try to fix it ourselves, or any way to bump its priority?

Here's what it looks like:

https://github.com/dotnet/wpf/assets/367446/ddca12bc-8d68-46ce-ace7-d092c5ba05f2

Thanks!

cwensley commented 1 year ago

Just posting an update here, I've been poking away at this for a while and found that using GC.TryStartNoGCRegion while setting the TextBox.Text property fixes the issue. I'm not sure whether it would be considered as something to change in WPF itself to improve performance, but at least we have a way forward without disabling features. To see the (quite dramatic) results:

https://github.com/dotnet/wpf/assets/367446/0fe05305-0c63-4b2f-9689-a1dd34d8fb8c

Here's the test code:

    public partial class MainWindow : Window
    {
        List<TextBox> textBoxes = new List<TextBox>();
        public MainWindow()
        {
            InitializeComponent();

            for (int y = 0; y < 10; y++)
            {
                grid.RowDefinitions.Add(new RowDefinition {  Height = new GridLength(1, GridUnitType.Star) });

                for (int x = 0; x < 10; x++)
                {
                    if (y == 0)
                    {
                        grid.ColumnDefinitions.Add(new ColumnDefinition {  Width = new GridLength(1, GridUnitType.Star)});
                    }
                    var tb = new TextBox();
                    Grid.SetColumn(tb, x);
                    Grid.SetRow(tb, y);
                    textBoxes.Add(tb);

                    grid.Children.Add(tb);
                }
            }

            var timer = new DispatcherTimer();
            timer.Interval = TimeSpan.FromSeconds(0.01);
            timer.Tick += Timer_Tick;
            timer.Start();
        }

        private void Timer_Tick(object? sender, EventArgs e)
        {
            var rnd = new Random();
            foreach (var tb in textBoxes)
            {
                var ret = noGCRegion.IsChecked == true && GC.TryStartNoGCRegion(10000000);
                tb.Text = rnd.Next(int.MaxValue).ToString();
                if (ret)
                    GC.EndNoGCRegion();
            }
        }
    }
lindexi commented 1 year ago

The TextStore in TextBox should interop with native code, such as IME and so on. The GC need cost more to keep the memory safe. That means the tb.Text = rnd.Next(int.MaxValue).ToString(); will take more time, and it will cause a missed UI refresh. Because it may intermittently miss UI refresh, resulting in what looks like @cwensley video shown.

@cwensley Another possible approach is to configure DispatcherPriority in DispatcherTimer, see:

        // DispatcherPriority.Loaded = 6
        // DispatcherPriority.Render = 7
        // DispatcherPriority.Loaded < DispatcherPriority.Render
        var timer = new DispatcherTimer(DispatcherPriority.Loaded);
JensNordenbro commented 1 year ago

We are also now running into this issue (we've migrated some MFC code over to WPF) and is causing our 3D view to stutter when a user pans/rotates/etc since the camera locations that are being tracked in a few text boxes (which they can edit manually at any time). The whole application can lock up for upwards of 2-4 seconds every so often which is not a very good user experience.

In doing a little debugging I found it was due to an apparent deadlock that occurs every so often with the how WPF interacts with the thread input manager. I did find that the following registry setting can avoid the issue (which seems better than a GC hack, that actually causes worse performance in our case):

[HKEY_CURRENT_USER\Software\Microsoft\CTF]
"Disable Thread Input Manager"=dword:00000001

With this setting there is no stutter at all, perhaps @JensNordenbro you can verify this on your end too.

What I'd like to know is, if our software sets the above registry value when installed what are the consequences? What feature(s) would be lost? Is there any progress on this issue, any hints at what we can do to try to fix it ourselves, or any way to bump its priority?

Here's what it looks like:

Screen.Recording.2023-06-23.at.10.19.47.AM.mp4 Thanks!

I can confirm that the registry setting makes the stutter go away ! I tested on a Windows Sandbox on Windows 11 so it is a clean environment. Not sure what it means for the behavior of the system but surely something is not right wrt interaction with this "subsystem". @miloush ; are you a Microsofty? Can you explain?

cwensley commented 1 year ago

@lindexi thanks for the feedback, it is very much appreciated.

The TextStore in TextBox should interop with native code, such as IME and so on. The GC need cost more to keep the memory safe.

I'm not sure I follow here, temporarily disabling any GC from happening while setting the Text should still allow interop with native code, IME, etc. I'm not sure how this would also not allow the memory to be safe, from what I understand is it would simply defer the GC to sometime outside of setting that property.

will take more time, and it will cause a missed UI refresh. Because it may intermittently miss UI refresh, resulting in what looks like @cwensley video shown.

I'm not sure that this is just a missed UI refresh, the whole application hangs and is unresponsive.

Another possible approach is to configure DispatcherPriority in DispatcherTimer, see:

The sample is using a DispatchTimer to simulate many events happening in quick succession, this is not the case in our real-world example. If you look at my previous video this happens when using the mouse to manipulate a 3D view which in turn updates only 6 TextBox controls. I have tried to minimize the rate in which they are updated, but even limiting it to 5 updates per second vs 20-30 (which is actually horrible from a user perspective) still yields the same performance/hang issue.

lindexi commented 1 year ago

@cwensley Thank you for your reply. I can't answer your question until I know more.

JensNordenbro commented 3 months ago

@lindexi What happened to the @cwensley proposal?

lindexi commented 3 months ago

@JensNordenbro I do not know what happen...

JensNordenbro commented 3 months ago

Ok. @cwensley?

cwensley commented 3 months ago

@JensNordenbro All I can say is that the GC.TryStartNoGCRegion approach is working well for us. I do not have the knowledge of where/how that might be applied to WPF in general, so am unable to submit a PR with changes. Ideally there just wouldn't be the deadlock to begin with.

JensNordenbro commented 3 months ago

Ok, Thanks guys.