Steve
Steve

Reputation: 732

C# WPF Freezing UI

The app takes the text from inputField searches regex matches in it and returns the results into outputField. To put it in a few lines of code I have something like this:

public class Parser {
    public Parser(string _text)
    {
        text = _text;
    }

    private string text { get; set; }

    public string[] find()
    {

        string r1 = "...";
        string r2 = "...";
        string r3 = "...";

        string[] regArray = new string[] { r1, r2, r3 };

        List<string> resL = new List<string>();

        for (int i = 0; i < regArray.Length; i++)
        {
            MatchCollection matchList = Regex.Matches(text, regArray[i]);
            var list = matchList.Cast<Match>().Select(match => match.Value).ToList();
            resL.AddRange(list);
        }

        string[] res = resL.Distinct().ToArray();
        if (res.Length > 0)
            return res;
        return new string[0];
    } }

private async void FindButton_Click(object sender, RoutedEventArgs e) {
    FindButton.Content = "Searching...";
    FindButton.IsEnabled = false;
    await Task.Delay(1);

    try
    {
        string parsed = string.Empty;
        if (string.IsNullOrWhiteSpace(new TextRange(InputField.Document.ContentStart, InputField.Document.ContentEnd).Text)) ;
        {
            OutputField.Document.Blocks.Clear();
            MessageBox.Show("Empty input");
        }
        else
        {
            Parser nOb = new Parser(new TextRange(InputField.Document.ContentStart, InputField.Document.ContentEnd).Text);
            string[] result = nOb.find();

            if (result.Length == 0)
            {
                OutputField.Document.Blocks.Clear();
                MessageBox.Show("Nothing found");
            }
            else
            {
                for (int i = 0; i < result.Length; i++)
                {
                    parsed += result[i] + Environment.NewLine;
                }

                OutputField.Document.Blocks.Clear();
                OutputField.Document.Blocks.Add(new Paragraph(new Run(parsed)));

                MessageBox.Show("Success");
            }
        }
    }
    catch (Exception ex)
    {
        MessageBox.Show("Error: " + ex.Message);
    }

    FindButton.Content = "Default";
    FindButton.IsEnabled = true; 
}

The problem is that when the text from inputField is really large and the program tries to find all matches UI starts to freeze. It's becoming impossible to minimize the program window, Windows says that the app doesn't respond and asks if I want to close it. If I don't click on the program during the work it finishes fine. So is it possible to avoid freezing somehow and make it possible to minimize the app while it works with large inputs? Any help is appreciated.

Upvotes: 0

Views: 1064

Answers (4)

mm8
mm8

Reputation: 169400

Try to execute the find() method on a background thread:

private async void FindButton_Click(object sender, RoutedEventArgs e)
{
    FindButton.Content = "Searching...";
    FindButton.IsEnabled = false;

    try
    {
        if (string.IsNullOrWhiteSpace(new TextRange(InputField.Document.ContentStart, InputField.Document.ContentEnd).Text))
        {
            OutputField.Document.Blocks.Clear();
            MessageBox.Show("Empty input");
        }
        else
        {
            string[] result;
            await Task.Run(() =>
            {
                Parser nOb = new Parser(new TextRange(InputField.Document.ContentStart, InputField.Document.ContentEnd).Text);
                result = nOb.find();
            });

            if (result == null || result.Length == 0)
            {
                OutputField.Document.Blocks.Clear();
                MessageBox.Show("Nothing found");
            }
            else
            {
                StringBuilder sb = new StringBuilder();
                for (int i = 0; i < result.Length; i++)
                {
                    sb.AppendLine(result[i]);
                }

                OutputField.Document.Blocks.Clear();
                OutputField.Document.Blocks.Add(new Paragraph(new Run(sb.ToString())));

                MessageBox.Show("Success");
            }
        }
    }
    catch (Exception ex)
    {
        MessageBox.Show("Error: " + ex.Message);
    }

    FindButton.Content = "Default";
    FindButton.IsEnabled = true;
}

Upvotes: 1

Francesco Bonizzi
Francesco Bonizzi

Reputation: 5302

I have to admit that I didn't understand what was the goal of your code, but I would try with something like this:

I simplified your Parser class:

    public class Parser 
    {
        const string _r1 = "...";
        private string _text;

        public Parser(string text)
        {
            _text = text;
        }

        public IEnumerable<string> Find()
        {
            return Regex.Matches(_text, _r1)
                .Cast<Match>()
                .Select(match => match.Value)
                .Distinct();
        } 
    }

And a part of you button click code, using a StringBuilder:

Parser nOb = new Parser(yourText);
var result = nOb.Find();
string parsed = string.Empty;

if (!result.Any())
{
    // Nothing found
}
else
{
    var stringBuilder = new StringBuilder();
    foreach (var line in result)
    {
        stringBuilder.AppendLine(line);
    }

    parsed = stringBuilder.ToString();

    // ...parsed
}

If you specify better the final goal of your code we can help more precisely.

Upvotes: 0

Ben Voigt
Ben Voigt

Reputation: 283893

This is horrible:

for (int i = 0; i < result.Length; i++)
{
    parsed += result[i] + Environment.NewLine;
}

Because strings are immutable, this creates N strings, each longer than the last, for total memory consumption of O(N2). And time wasted copying is also O(N2).

Best is to use String.Join, but when you need to do string manipulation in a loop, use System.Text.StringBuilder, which modifies its internal buffer instead of throwing it away and making a complete copy for every single operation the way string must.

But, why are you making a multiline string into a single Run in the first place? Probably each of those should be a separate Paragraph object, and let the WPF container manage clipping (and not have to draw Paragraph objects that are completely offscreen). If you do this, though, build one content object disconnected from the UI, then add it to the UI in one step, so that you don't end up with the layout engine running over again for each concatenation.

Upvotes: 1

Ivan I
Ivan I

Reputation: 9990

That's expected behavior as you run your task on the UI thread by default, and as long as it isn't completed no UI operation is possible.

You need to use Task.Run to make the code run on non-UI thread. But you may need to update some controls so then you need to come back to UI thread and then you can use Dispatcher.RunAsync, though it is preferable to use data binding in WPF.

Those are pretty basic things and you should take a further read on MSDN about those topics.

Upvotes: 0

Related Questions