jtheisen / old-moldinium

Let a framework figure out what property needs PropertyChanged fired for.
MIT License
2 stars 0 forks source link

Multithreading not supported: Run-time crash - implement safeguards #1

Open Shiloovr opened 7 years ago

Shiloovr commented 7 years ago

I wrote a simple code that demonstrates a bug that occurs at runtime. The bug occurs during the run. Sometimes immediately and sometimes after a while. Attached video.

https://drive.google.com/file/d/0B0C_KheExAN7ZnU0ZG5yZHRKYzg/view?usp=sharing

https://drive.google.com/file/d/0B0C_KheExAN7dnF0ZTB2WnR3bkk/view?usp=sharing

jtheisen commented 7 years ago

Moldinium isn't thread-safe, and the way it works, it can't ever be.

However, I could do a much better job at protecting against use from multiple threads. Users should really get an exception when accessing properties from a non-ui thread.

Your sample works (in principle) when you change the UserInfo ctor to:

        public UserInfo()
        {
            Cycle();
        }

        public async void Cycle()
        {
            while (true)
            {
                BurthDate = BurthDate.AddYears(1);
                Num2++;
                await Task.Delay(1000);
            }
        }

(In general, you can schedule updates in the UI thread with Dispatcher.Invoke.)

However, that turns out to be excessively slow for 2500 ui elements. I'm not yet certain how the multithreaded version can be (seemingly) fast, but if you need performance for this kind of massive-ui-element scenario, I do some experimentation and profiling.

Shiloovr commented 7 years ago

Hi Jens,

First, thanks for the quick response.

I used Dispatcher.BeginInvoke. Indeed, not a crash occurs. This is the code I wrote: public UserInfo() { Task.Run(() => { while (true) { Application.Current.Dispatcher.BeginInvoke(new Action(() =>
{ BurthDate = BurthDate.AddYears(1); Num2++; } )); Thread.Sleep(1000); } }); }

But there is a big performance issue. Initially, the app works great and you can move the window (the GUI thread is available to receive events). As time passes, the application consumes more CPU resources and the GUI thread Taken - You can not move the window. This occurs within 2-3 minutes. However, even though at that time I do not do anything except feed values. Which means that it depends on the moldinium library.

jtheisen commented 7 years ago

I tried that myself and I

Shiloovr commented 7 years ago

You add conditions that have not been the original source:                  if (i <10)                      Users.Add (user);

This of course makes the collection contain 250 instead of 2500.

Remove the condition. Run the application and move the window. Look within 2-3 minute window can not moved.

jtheisen commented 7 years ago

You're right that was stupid.

I think I have an idea about what's going on.

The tasks you create don't start right away. If you log something at the start, like this:

        static int count = 0;

        public void Cycle()
        {
            Task.Run(() => {
                Debug.WriteLine("task #" + ++count);
                ...

Then you will see that they only start a handful, and then each second or so there comes another one in.

I think the Task.Run schedules tasks to run on one of the thread pool threads, of which there are only a handful, and you then go on putting each to sleep - and that's why new ones don't get started right away.

Why new ones keep slowly coming (even though you don't end any of the tasks as the while loops never terminate) I don't quite understand. My theory is that .NET realizes thread starvation and increases the the thread pool size.

In any case: As more and more threads come up the load increases as more and more UI components start updating on changes.

So I don't think it's due to something in Moldinium why the test app gets slower, although I will make some additional research on what exactly is taking up the time here (I think it's the UI components themselves, updating 2500 TextBlocks per second is something), but I won't get around to do it right away.

Shiloovr commented 7 years ago

I set the listbox control with virtualization. This practice is not created in 2500 controls. Only a few controls, actually created. So it does not seem a problem of updating the GUI.

In general, the fact that you need to use the GUI Thread to update properties of the objects in the ViewModel is not easy. The GUI Thread is "expensive". In addition, using Invoke makes the code awkward

jtheisen commented 7 years ago

You're right, it's not the UI controls.

The problem is that MainVM.TotalAges is re-calculated each time anything it depends on, which means at least 5000 times per second on a cursory glance, and it's summing over 2500 elements. Add to that that currently, Moldinium's property access is computationally a good deal more expensive than normal property access, and that gives the performance problems we see.

So this is to some extent simply the nature of dependency tracking and you'd see the same effect with something like Knockout.

However, there is a way to get better performance in many circumstances. For one, Moldinium has a lot of room to improve property access speed, especially if Fody's technique is used.

More importantly, there's another Javascript dependency tracking library called mobx that allows modifications to be grouped together. Within those so-called actions, dependent calculations are not updated until either the action terminates or the respective calculated properties are requested from within the action. In C#, that could look like this:

using (BeginAction())
{
    // do multiple modifications
}
// only here INotifyPropertyChanged gets called

However, it does require some thinking on the users side and in your sample, you'd have to group the many distinct per-second updates together.

If you really want a sum over thousands of items that change fast at random, what you probably need is a more sophisticated kind of dependency tracking than either Knockout, mobx or Moldinium provides - at the moment anyway. (You'd need to maintain a sum that gets updated incrementally rather than recalculated. Possible, but much more challenging.)

About multithreading:

When I started writing I planned to say that multithreading has no place in UI logic and that the problem should be solved with techniques hinted at about (the incremental update thing).

However, now that I think about it, there may be a way to use it in a safe way in the following form

In any case, I'd strongly recommend against modifying your state from different threads simultaneously, with or without Moldinium. Even if every library you use is thread-safe, it's still often a debugging nightmare.