4
\$\begingroup\$

I've been experiencing this performance issue for about 3 days and really need your help. I'm a novice at c# and cannot understand what I can do to optimize my application.

The problem is that the UI is not as responsive as it must be while many items are loading.

I decided to write an application on learning purpose where we can calculate factorial of the input big number. It's also important to make calculations concurrently in several threads.

I also want to bind a progress bar to the progress of calculations and show interim calculations.

For example: Input number: 5

Interim calculations:

0!=1
1!=1
2!=2
3!=6
4!=24
5!=120

Result: 5!=120

Everything is fine when the input number is less then 1000. If it's more then freezes are coming.

What I've tried:

  1. At first I've tried to write my own observable concurrent collection where I add my items from other threads.
  2. Then I've tried to push interim results, which are calculated by other threads, into ConcurrentQueue from which I will dequeue them into my observable collection.
  3. I've tried to turn on virtualization but it also didn't help. Actually, I'm pretty sure that the problem is in my collection that ItemSource is binded to is growing and when I add more elements to this collection then it starts redrawing completly.

CalculationProcessView:

<UserControl x:Class="FactorialCalculator.WPF.Views.CalculationProcessView"
         xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
         xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
         xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
         xmlns:d="http://schemas.microsoft.com/expression/blend/2008" 
         xmlns:local="clr-namespace:FactorialCalculator.WPF.Views"
         mc:Ignorable="d" 
         d:DesignHeight="450" d:DesignWidth="800">

<StackPanel>
    <Label Content="Calculation:"/>

    <Border BorderBrush="Gainsboro" BorderThickness="0,2">
        <ListBox 
            x:Name="CalculationProcessListBox"
            Height="100"
            ItemsSource="{Binding Path=Calculations, Mode=OneWay, IsAsync=True}"
            VirtualizingPanel.IsContainerVirtualizable="True"
            VirtualizingPanel.IsVirtualizing="True"
            VirtualizingPanel.VirtualizationMode="Recycling"
            VirtualizingPanel.CacheLengthUnit="Page"
            VirtualizingPanel.CacheLength="2,2">

            <ListBox.Template>
                <ControlTemplate>
                    <ScrollViewer>
                        <ItemsPresenter />
                    </ScrollViewer>
                </ControlTemplate>
            </ListBox.Template>

            <ListBox.ItemsPanel>
                <ItemsPanelTemplate>
                    <VirtualizingStackPanel />
                </ItemsPanelTemplate>
            </ListBox.ItemsPanel>

        </ListBox>
    </Border>

    <ProgressBar Minimum="0"
                 Maximum="{Binding ProgressBarMaxValue}"
                 Value="{Binding Calculations.Count, Mode=OneWay}"
                 Height="28" />
</StackPanel>

CalculatorFormViewModel:

namespace FactorialCalculator.WPF.ViewModels
{
    public class CalculatorFormViewModel : BaseViewModel, ICalculatorFormViewModel
    {
        private string _progressBarMaxValue;
        private ICommand _calculateCommand;
        private IModelFactory _modelFactory;
        private SystemTimer _dispatcherTimer;
        private ICalculationModel _calculation;
        private ICommandFactory<RelayCommand> _commandFactory;
        private IFactorialCalculationService _factorialCalculationService;
        private static object _syncLock = new object();

        public CalculatorFormViewModel(Dispatcher uiDispatcher,
                                       IModelFactory modelFactory,
                                       ICommandFactory<RelayCommand> commandFactory,
                                       IFactorialCalculationService factorialCalculationService) : base(uiDispatcher)
        {
            _modelFactory = modelFactory;
            _commandFactory = commandFactory;
            _factorialCalculationService = factorialCalculationService;
            _calculation = _modelFactory.CreateCalculationModel();
            Calculations = new ConcurrentSortedList<ICalculationModel>();
            BindingOperations.EnableCollectionSynchronization(Calculations, _syncLock);
        }

        public ConcurrentSortedList<ICalculationModel> Calculations { get; set; }

        public string ProgressBarMaxValue
        {
            get { return _progressBarMaxValue; }
            set
            {
                _progressBarMaxValue = value;
                NotifyPropertyChanged("ProgressBarMaxValue");
            }
        }

        public ICalculationModel Calculation
        {
            get
            {
                return _calculation;
            }
            set
            {
                _calculation = value;
                NotifyPropertyChanged("Calculation");
            }
        }

        #region ICalculatorFormViewModel implementation

        public ICommand CalculateCommand
        {
            get
            {
                return _calculateCommand ??
                  (
                      _calculateCommand
                            = _commandFactory.CreateCommand(
                                 _ => this.Calculate().FireAndForgetSafeAsync(null))
                  );
            }
        }

        public async Task Calculate()
        {
            if (_factorialCalculationService.IsInPorcess)
            {
                _factorialCalculationService.StopCalculation();
            }

            if (BigInteger.TryParse(Calculation.InputNumber, out BigInteger number))
            {
                try
                {
                    Calculations.Clear();

                    ProgressBarMaxValue = Calculation.InputNumber;
                    StartMonitoringCalculationProgress();

                    await _factorialCalculationService.StartCalculation(number);
                }
                finally
                {
                    StopMonitoringCalculationProgress();
                }
            }
            else
            {
                throw new ArgumentException("Incorrect input value!");
            }
        }

        #endregion

        protected void StartMonitoringCalculationProgress()
        {
            _dispatcherTimer = new SystemTimer() { Interval = 500 };

            _dispatcherTimer.Elapsed += (o, args) =>
            {

                if (_factorialCalculationService
                            .TryGetInterimResults(out IDictionary<BigInteger, BigInteger> interimResults))
                {
                    lock (_syncLock)
                    {
                        Calculations.AddItems(
                            interimResults.Select(
                                    (item) =>
                                    {
                                        var calculationModel = _modelFactory.CreateCalculationModel();

                                        calculationModel.InputNumber = item.Key.ToString();
                                        calculationModel.Factorial = item.Value.ToString();

                                        return calculationModel;
                                    }
                                ).ToList()
                        );
                    }
                }
            };

            _dispatcherTimer.Start();
        }

        protected void StopMonitoringCalculationProgress()
        {
            _dispatcherTimer.Stop();
            _dispatcherTimer.Dispose();
        }
    }
}

FactorialCalculationService:

namespace FactorialCalculator.Application.Services
{
    public class FactorialCalculationService : IFactorialCalculationService
    {
        private bool _isCanceled;
        private bool _isInPorcess;
        private CancellationTokenSource _tokenSourse;
        private ConcurrentQueue<IDictionary<BigInteger, BigInteger>> _calculationQueue;
        private readonly IFactorialCalculatorService _factorialCalculatorService;

        public FactorialCalculationService(IFactorialCalculatorService factorialCalculatorService)
        {
            _isCanceled = false;
            _isInPorcess = false;
            _factorialCalculatorService = factorialCalculatorService;
            _calculationQueue = new ConcurrentQueue<IDictionary<BigInteger, BigInteger>>();
        }

        #region IFactorialCalculationService implementation

        public bool IsInPorcess => _isInPorcess;

        public bool TryGetInterimResults(out IDictionary<BigInteger, BigInteger> interimResults)
        {
            return _calculationQueue.TryDequeue(out interimResults);
        }

        public Task StartCalculation(BigInteger number)
        {
            return 
                Task.Run(
                    () =>
                    {
                        try
                        {
                            _isCanceled = false;
                            _isInPorcess = true;
                            _tokenSourse = new CancellationTokenSource();

                            _factorialCalculatorService.Calculate(
                                                             number,
                                                             (dictionary) => _calculationQueue.Enqueue(dictionary),
                                                             _tokenSourse.Token
                                                         );


                            //Wait while all the results will be consumed
                            while (!_calculationQueue.IsEmpty);
                        }
                        catch (AggregateException ax)
                        {
                            ax.Handle(ex => {
                                OperationCanceledException exception = ex as OperationCanceledException;

                                if (exception != null)
                                {
                                    _isCanceled = true;
                                }

                                return exception != null;
                            });

                        }
                        finally
                        {
                            _isInPorcess = false;
                        }
                    }
                );
        }

        public void StopCalculation()
        {
            if (!_isCanceled)
            {
                _calculationQueue = new ConcurrentQueue<IDictionary<BigInteger, BigInteger>>();
                _tokenSourse.Cancel();
            }
        }

        #endregion
    }
}

FactorialCalculatorService:

namespace FactorialCalculator.Domain.Services
{
    public class FactorialCalculatorService : IFactorialCalculatorService
    {
        private int _maxTasks;

        #region IFactorialCalculatorService implementation

        public BigInteger Calculate(BigInteger number,
                                    Action<IDictionary<BigInteger, BigInteger>> callback = null,
                                    CancellationToken cancellationToken = default(CancellationToken))
        {
            if (number < 0)
                throw new ArgumentOutOfRangeException(nameof(number));

            BigInteger countOfTasks = number / Environment.ProcessorCount;
            countOfTasks = countOfTasks > int.MaxValue ? int.MaxValue : (int)countOfTasks;
            _maxTasks = countOfTasks == 0 ? 1 : (int)countOfTasks;

            var tasks = ParallelizeCalculation(number, callback, cancellationToken);

            Task.WaitAll(tasks);

            return GetFinalResult(tasks);
        }

        #endregion


        protected virtual BigInteger CalculateClass(BigInteger upperBound,
                                                    BigInteger startFrom,
                                                    CancellationToken cancellationToken = default(CancellationToken),
                                                    Action<IDictionary<BigInteger, BigInteger>> callback = null)
        {
            cancellationToken.ThrowIfCancellationRequested();

            SortedDictionary<BigInteger, BigInteger> calculationHistory = new SortedDictionary<BigInteger, BigInteger>();

            for (var i = startFrom; i <= upperBound; i += _maxTasks)
            {
                //Thread.Sleep(50);
                cancellationToken.ThrowIfCancellationRequested();

                var internalResult = Calculate(i);
                calculationHistory.Add(i, internalResult);
            }

            callback?.Invoke(calculationHistory);

            return calculationHistory.Last().Value;
        }

        protected virtual BigInteger Calculate(BigInteger number)
        {
            BigInteger result = 1;

            for (BigInteger i = result; i <= number; i++)
            {
                result *= i;
            }

            return result;
        }

        protected virtual Task<BigInteger>[] ParallelizeCalculation(BigInteger number,
                                                        Action<IDictionary<BigInteger, BigInteger>> callback = null,
                                                        CancellationToken cancellationToken = default(CancellationToken))
        {
            var tasks = Enumerable.Range(0, _maxTasks)
                                  .Select(
                                            i => Task.Factory.StartNew(
                                                () => CalculateClass(number, i, cancellationToken, callback),
                                                TaskCreationOptions.AttachedToParent
                                            )
                                         )
                                  .ToArray();


            return tasks;
        }

        private BigInteger GetFinalResult(Task<BigInteger>[] tasks)
        {
            BigInteger finalResult = 1;

            foreach(var task in tasks)
            {
                finalResult *= task.Result;
            }

            return finalResult;
        }
    }
}

ConcurrentSortedList:

namespace FactorialCalculator.WPF.Infrastructure.Collections
{
    public class ConcurrentSortedList<T> : ObservableCollection<T>
    {
        private readonly ReaderWriterLockSlim _lock;
        private bool suspendCollectionChangeNotification;

        public ConcurrentSortedList()
            : base()
        {
            this.suspendCollectionChangeNotification = false;
            _lock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);
        }

        public override event NotifyCollectionChangedEventHandler CollectionChanged;


        public void AddItems(IList<T> items)
        {
            try
            {
                _lock.EnterWriteLock();

                this.SuspendCollectionChangeNotification();

                foreach (var i in items)
                {
                    Add(i);
                }
            }
            finally
            {
                _lock.ExitWriteLock();
            }

            this.NotifyChanges(); 

        }
        public new void Add(T item)
        {
            try
            {
                _lock.EnterWriteLock();

                int i = 0;
                while (i < Items.Count && Comparer<T>.Default.Compare(Items[i], item) < 0)
                    i++;

                Items.Insert(i, item);
            }
            finally
            {
                _lock.ExitWriteLock();
            }

            OnPropertyChanged(new System.ComponentModel.PropertyChangedEventArgs("Count"));
        }


        public void NotifyChanges()
        {
            this.ResumeCollectionChangeNotification();
            var arg = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset);
            this.OnCollectionChanged(arg);
        }

        public void RemoveItems(IList<T> items)
        {
            try
            {
                _lock.EnterWriteLock();

                this.SuspendCollectionChangeNotification();

                foreach (var i in items)
                {
                    Remove(i);
                }
            }
            finally
            {
                _lock.ExitWriteLock();
            }

            this.NotifyChanges();
        }

        public void ResumeCollectionChangeNotification()
        {
            this.suspendCollectionChangeNotification = false;
        }

        public void SuspendCollectionChangeNotification()
        {
            this.suspendCollectionChangeNotification = true;
        }

        protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
        {

            using (BlockReentrancy())
            {
                if (!this.suspendCollectionChangeNotification)
                {
                    NotifyCollectionChangedEventHandler eventHandler =
                          this.CollectionChanged;
                    if (eventHandler == null)
                    {
                        return;
                    }

                    Delegate[] delegates = eventHandler.GetInvocationList();

                    foreach (NotifyCollectionChangedEventHandler handler in delegates)
                    {
                        DispatcherObject dispatcherObject
                             = handler.Target as DispatcherObject;

                        if (dispatcherObject != null
                               && !dispatcherObject.CheckAccess())
                        {
                            dispatcherObject.Dispatcher.BeginInvoke(DispatcherPriority.DataBind, handler, this, e);
                        }
                        else
                        {
                            handler(this, e);
                        }
                    }
                }
            }
        }


    }
}

BaseViewModel

public abstract class BaseViewModel : INotifyPropertyChanged
{
    protected readonly Dispatcher _uiDispatcher;

    protected BaseViewModel(Dispatcher uiDispatcher)
    {
        _uiDispatcher = uiDispatcher;
    }

    #region INotifyPropertyChanged implementation

    public event PropertyChangedEventHandler PropertyChanged;

    protected void NotifyPropertyChanged([CallerMemberName] string propertyName = null)
    {
        var propertyChanged = PropertyChanged;
        if (propertyChanged == null)
        {
            return;
        }

        _uiDispatcher.Invoke(() => propertyChanged.Invoke(this, new PropertyChangedEventArgs(propertyName)));
    }

    #endregion
}

App:

enter image description here

Source code: enter link description here

\$\endgroup\$
  • 1
    \$\begingroup\$ @dfhwze mhmmm it looks like this code is working correctly for small numbers but might have performance/memory problems with larger ones... I'd say this is on-topic. \$\endgroup\$ – t3chb0t Jun 25 at 17:06
  • \$\begingroup\$ @t3chb0t You have convinced me. The choice of words in the OP was unfortunate. \$\endgroup\$ – dfhwze Jun 25 at 17:09
  • 1
    \$\begingroup\$ @dfhwze yep, but we've managed to save the world for today - from -2 to +2 ;-) There is only a small part that is still off-topic... the creation of the progress bar.. but I think we can ignore it... \$\endgroup\$ – t3chb0t Jun 25 at 17:14
  • 1
    \$\begingroup\$ @dfhwze, Added BaseViewModel and source files/ \$\endgroup\$ – Ivan Radiant Jun 25 at 17:26
  • 2
    \$\begingroup\$ I would strongly suspect that a large part of your problem is using wpf. I can load 1000! and all intermediate results into a datagridview in winform, using only a single thread, in less than 1 sec.. Which to me isn't a large freeze time for a UI. \$\endgroup\$ – tinstaafl Jun 25 at 19:55
1
\$\begingroup\$

The problem is a combination of very large numbers and rendering in ItemsControlss in WPF.

And if you think about it: No human can use the full view of a number with 10000 digits to anything. Therefore you need a view model:

  public class FactorialVM : INotifyPropertyChanged
  {
    public FactorialVM(long value, BigInteger factorial)
    {
      Value = value;
      Factorial = factorial;
    }

    public long Value { get; }
    public BigInteger Factorial { get; }


    public string Display { get; private set; }

    public event PropertyChangedEventHandler PropertyChanged;
    void OnPropertyChanged(string name)
    {
      PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(name));
    }

    async internal Task SetDisplay()
    {
      await Task.Run(() =>
      {
        string temp = Factorial.ToString();

        if (Factorial >= 1000)
        {
          Display = $"{Value}! = {temp[0]}.{temp.Substring(1, 3)}e{temp.Length - 1}";
        }
        else
        {
          Display = $"{Value}! = {temp}";
        }

        OnPropertyChanged("Display");
      });
    }

    public string FullView => $"{Value}! = {Factorial}";

    public override string ToString()
    {
      return Display;
    }
  }

that reduces the view of each number to something understandable for humans and maintainable for the rendering system.

A test view with a button, a text box for the count of numbers to add and the result list could look like:

  <DockPanel>
    <ToolBarTray DockPanel.Dock="Top">
      <ToolBar>
        <Button Content="Next" Name="NextCm" Click="Button_Click" />
        <TextBox Name="CountOfNumbers" Text="100" Width="100" TextAlignment="Right"/>
      </ToolBar>
    </ToolBarTray>
    <ListBox Name="NumberList"  
              VirtualizingPanel.IsContainerVirtualizable="True"
              VirtualizingPanel.IsVirtualizing="True"
              VirtualizingPanel.VirtualizationMode="Recycling"
              VirtualizingPanel.CacheLengthUnit="Item"
              VirtualizingPanel.CacheLength="10, 10"
              >
      <ListBox.ItemTemplate>
        <DataTemplate>
          <TextBlock Text="{Binding Display}">
            <TextBlock.ContextMenu>
              <ContextMenu>
                <MenuItem Header="Copy" Name="ValueCopyCmd" Click="ValueCopyCmd_Click" />
              </ContextMenu>
            </TextBlock.ContextMenu>
          </TextBlock>
        </DataTemplate>
      </ListBox.ItemTemplate>
    </ListBox>
  </DockPanel>

The NumberList List box is initialized as follows:

ObservableCollection<FactorialVM> numbers = new ObservableCollection<FactorialVM>();
public MainWindow()
{
  InitializeComponent();

  NumberList.ItemsSource = numbers;
}

The click event for the button could then be something like:

long currentValue = 1;
BigInteger currentFac = 1;
async private void Button_Click(object sender, RoutedEventArgs e)
{
  try
  {
    NextCm.IsEnabled = false;
    int nums = int.Parse(CountOfNumbers.Text);

    long stop = currentValue + nums;

    for (; currentValue < stop; currentValue++)
    {
      currentFac *= currentValue;
      FactorialVM vm = new FactorialVM(currentValue, currentFac);
      await vm.SetDisplay();
      numbers.Add(vm);
      NumberList.ScrollIntoView(vm);
    }

  }
  finally
  {
    NextCm.IsEnabled = true;
  }
}

And you could provide a copy command to copy a number to the clipboard:

private void ValueCopyCmd_Click(object sender, RoutedEventArgs e)
{
  string text = ((sender as MenuItem).DataContext as FactorialVM).FullView;
  Clipboard.SetText(text);

}

You'll of course have to make properly validation of input and make a proper main View Model etc.

You can try the above in a standard WPF Window and you'll see it works smoothly. But be aware that the calculation of the factorial for numbers takes longer and longer time as well as the formatting of the display string will.

There may be possibilities for performance improvements, but the above is a way to go.

\$\endgroup\$
  • \$\begingroup\$ No horizontal scrollbars in the code blocks. This alone is a +1. \$\endgroup\$ – dfhwze Jun 26 at 6:44
  • \$\begingroup\$ Thanks a lot for your help I will take it into account. Actually, What I'm looking for is the way to step around the problem when we have big data that is splitted into several portions and we need to load it concurrently. The user doesn't need to press "load more" button. I know that this is weird, but I strive to solve this task this way. It's only on learning purpose. Now I'm about to try to implement somethin like this codeproject.com/Articles/34405/WPF-Data-Virtualization. \$\endgroup\$ – Ivan Radiant Jun 26 at 7:29
  • 1
    \$\begingroup\$ I'm so stupid. Let's have a look at what I did in Calculate method of FactorialCalculatorService. If the input number is 10 000, then I will schedule 1666 Tasks (in my case I have six cores). Thead pool probably will create many threads and the most important thing is that too many switches will occur. I reduced it and UI became way more responsive \$\endgroup\$ – Ivan Radiant Jun 26 at 7:58
  • 1
    \$\begingroup\$ @Henrik Hansen, And I also display the numbers that way that you do and everything works and looks nice. Even factorial of 50 000 calculates pretty good \$\endgroup\$ – Ivan Radiant Jun 26 at 8:05
  • 1
    \$\begingroup\$ @Henrik Hansen on my computer about 30 minutes. But if I get rid of logging inerim caculations and use more tasks, It will be much faster \$\endgroup\$ – Ivan Radiant Jun 26 at 11:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.