[WPF] Threading - Parsing script in background thread

[NickThissen]

Hi,

I am building a simple script editor application in WPF which parses the text in the script into constructs such as 'classes', 'events', etc (these don't really relate to .NET classes and events, it's a game script).

Every time the script text changes (eg: the user types into the editor), the script needs to be parsed again so that any possible new classes/events can be picked up. The parsed classes and events are shown in two ComboBoxes above the editor, similar to Visual Studio.

The problem is that large scripts take a slight amount of time to parse, and this causes the UI to feel sluggish when typing. I am sure I cannot improve the parsing speed any more (and I'm not going to dump the code here and leave you to figure it out), so the obvious approach (at least to me) seems multithreading.

I want to run the parsing in a background thread, so that the UI does not freeze up for a few milliseconds when the user is typing. The logic seems easy enough; if the user types 1 letter, a background thread is started that starts parsing the script. When that finishes, it sends its results back to the window so it can show the new classes/events. Should the user type again during the time that the background thread is parsing (which will happen if someone is typing at a moderate to fast pace), then the current thread is simply aborted (any results it comes up with are out of date anyway) and a new thread is started.

I cannot figure out how to do this though, I've never been any good at threading. I keep getting problems where some object tries to access some other object from a different thread, which is of course not possible.

The most important code is the Script class:

public class Script : NotifyObject
    {
        // The container class that holds the thread and the parsing logic
        private ScriptParseThreadContainer _ScriptParseThread;
 
        public Script(string text)
        {
            _ParsedWords = new ObservableCollection<string>();
            this.Text = text;
        }
 
        private string _Text;
        public string Text
        {
            get { return _Text; }
            set
            {
                _Text = value;
                this.ParseText();
                this.OnPropertyChanged(() => this.Text);
            }
        }
 
        private readonly ObservableCollection<string> _ParsedWords;
        public ObservableCollection<string> ParsedWords { get { return _ParsedWords; } }
 
        private void ParseText()
        {
            // Create a new script parse thread container
            if (_ScriptParseThread != null) _ScriptParseThread.Dispose();
            _ScriptParseThread = new ScriptParseThreadContainer(this.Text);
 
            // Hook up event and start it
            _ScriptParseThread.ParsingComplete += ScriptParsingComplete;
            _ScriptParseThread.Start();
        }
 
        private void ScriptParsingComplete(object sender, EventArgs e)
        {
            // When the thread is done parsing, load the resulting words into this ParsedWords collection so the Window can display them
            this.ParsedWords.Clear();
            foreach (var word in _ScriptParseThread.ParsedWords)
            {
                this.ParsedWords.Add(word);
            }
            _ScriptParseThread.Dispose();
        }
 
        public class ScriptParseThreadContainer : IDisposable
        {
            public ScriptParseThreadContainer(string text)
            {
                _Thread = new Thread(ParseText);
                this.Text = text;
                _ParsedWords = new ObservableCollection<string>();
            }
 
            public event EventHandler ParsingComplete;
 
            public Thread _Thread;
            public string Text { get; set; }
 
            private readonly ObservableCollection<string> _ParsedWords;
            public ObservableCollection<string> ParsedWords { get { return _ParsedWords; } }
 
            public void Start()
            {
                _Thread.Start();
            }
 
            public void ParseText()
            {
                this.ParsedWords.Clear();
                var words = this.Text.Split(' ');
                foreach (var word in words)
                {
                    this.ParsedWords.Add(word);
                }
 
                if (this.ParsingComplete != null)
                {
                    this.ParsingComplete(this, EventArgs.Empty);
                }
            }
 
            public void Dispose()
            {
                _Thread.Abort();
                _Thread = null;
            }
        }
    }

It has properties 'Text' (just the script text) and 'ParsedWords'. For the sake of example I am just extracting the separate words from the script, as the parsing details are not important.

When the Text property changes, any running parsing thread is aborted and a new one is started. The thread is encapsulated in a class ScriptParseThreadContainer (so that I can pass values such as the Text and retrieve the resulting list of words when it is finished) which extracts all separate words ('parses the text'), puts them in an ObservableCollection<string> (I suppose any collection/array could have done) and then raises the ParsingComplete event.

The event handler for this ParsingComplete event simply reads the ParsedWords collection and puts each word in its own ParsedWords collection. The MainWindow has a ComboBox that is bound to this property so that it displays that collection.


This doesn't seem to work. It seems that the event handler for ParsingComplete is still raised on the background thread, so that I am accessing the Script.ParsedWords collection from a different thread that created it resulting in an error.



How can I do this? It shouldn't be that hard, right? I just need to run a thread that puts some strings into a collection, then I need it to notify me when it is finished, at which point I put those words in my own collection where the Window can read them...

Any help? Thanks!


EDIT
I might have gotten a bit further, not there yet though...

I suppose I have to somehow put the words from one collection to the other in a method which should then be called from the UI thread. Similar to how accessing controls from background threads works in Winforms? In that case I create a delegate and invoke that via the Control.Invoke method. I don't have any control here to call Invoke on though, so I found the next best thing: Dispatcher.CurrentDispatcher?

private void ScriptParsingComplete(object sender, EventArgs e)
        {
            // When the thread is done parsing, load the resulting words into this ParsedWords collection so the Window can display them
            var del = new LoadWordsDelegate(LoadWords);
            Dispatcher.CurrentDispatcher.BeginInvoke(del);
            _ScriptParseThread.Dispose();
        }
 
        private delegate void LoadWordsDelegate();
        private void LoadWords()
        {
            this.ParsedWords.Clear();
            foreach (var word in _ScriptParseThread.ParsedWords)
            {
                this.ParsedWords.Add(word);
            }
        }

This seems more appropriate, but still doesn't work. When using Dispatcher.CurrentDispatcher.BeginInvoke, the LoadWords method is never called. I can now type into the textbox without errors, but the ComboBox is never filled and stays empty. When I replace that line by Dispatcher.CurrentDispatcher.Invoke, the method is called, but still on the background thread it seems... It results in the same error as if I don't use the delegate invoking at all.

[Evil_Giraffe]

First point, you may not need multithreading.

The problem is that large scripts take a slight amount of time to parse, and this causes the UI to feel sluggish when typing. I am sure I cannot improve the parsing speed any more (and I'm not going to dump the code here and leave you to figure it out), so the obvious approach (at least to me) seems multithreading.

Don't be so sure.

// Whole load of script here (many many lines)

[CURSOR HERE TO WRITE A NEW LINE]
Script.DoSomething();

// Whole load of script (many many lines)

Imagine the user now types a letter (e.g. 'd'). Why do you need to reparse the whole file? You know that what is being written can only impact the current expression, so all you need to reparse is:

d
Script.DoSomething();

(Nitpick corner: this isn't really increasing the 'speed' of parsing. It's doing less parsing at the same speed, so takes less time.)

[Evil_Giraffe]

EDIT
I might have gotten a bit further, not there yet though...

I suppose I have to somehow put the words from one collection to the other in a method which should then be called from the UI thread. Similar to how accessing controls from background threads works in Winforms? In that case I create a delegate and invoke that via the Control.Invoke method. I don't have any control here to call Invoke on though, so I found the next best thing: Dispatcher.CurrentDispatcher?

private void ScriptParsingComplete(object sender, EventArgs e)
        {
            // When the thread is done parsing, load the resulting words into this ParsedWords collection so the Window can display them
            var del = new LoadWordsDelegate(LoadWords);
            Dispatcher.CurrentDispatcher.BeginInvoke(del);
            _ScriptParseThread.Dispose();
        }
 
        private delegate void LoadWordsDelegate();
        private void LoadWords()
        {
            this.ParsedWords.Clear();
            foreach (var word in _ScriptParseThread.ParsedWords)
            {
                this.ParsedWords.Add(word);
            }
        }

This seems more appropriate, but still doesn't work. When using Dispatcher.CurrentDispatcher.BeginInvoke, the LoadWords method is never called. I can now type into the textbox without errors, but the ComboBox is never filled and stays empty. When I replace that line by Dispatcher.CurrentDispatcher.Invoke, the method is called, but still on the background thread it seems... It results in the same error as if I don't use the delegate invoking at all.

Your problem is probably (I haven't checked this) that you're calling Dispatcher.CurrentDispatcher from the background thread. That gets the dispatcher for the background thread, which obviously is not the same as the UI dispatcher. In your form constructor, take a reference to the Dispatcher.CurrentDispatcher property and place it in a field, then use that instance from your background thread.

As to the difference between BeginInvoke and Invoke, BeginInvoke is asynchronous, meaning it will drop the message on the message queue and return, and leave something else to pump the messages off and respond to them. Since you dispatched to the background Dispatcher, the message will only get processed when the background thread yields and since you have a processor bound thread going on the only time it yields is when it shuts down, so nothing ever pumps that message.

Invoke is synchronous, meaning the thread will suspend until that message is processed. Because the thread yields, it is now free to process messages from the Dispatcher - the only one being the message you just posted. It processes the message and (because we're still on the background thread) goes pop on accessing the UI controls.

[NickThissen]

First point, you may not need multithreading.



Don't be so sure.

// Whole load of script here (many many lines)

[CURSOR HERE TO WRITE A NEW LINE]
Script.DoSomething();

// Whole load of script (many many lines)

Imagine the user now types a letter (e.g. 'd'). Why do you need to reparse the whole file? You know that what is being written can only impact the current expression, so all you need to reparse is:

d
Script.DoSomething();

(Nitpick corner: this isn't really increasing the 'speed' of parsing. It's doing less parsing at the same speed, so takes less time.)

I get what you're saying, but I don't think that's possible in my case, at least not without completely rewriting the parser. It's not a very complicated parser, but it derives its simplicity from the fact that it just parses the whole text and cannot parse only parts of a script. So I'd have to take a real good look at that, but I cannot see it working out any time soon.
Furthermore, the fact that the parses takes some time doesn't matter at all. Even if it took 2 seconds that would be acceptable, as long as the UI doesn't freeze up for 2 seconds after typing something. The parsed classes/events will be displayed in a ComboBox, and if it is parsed in the background the user will always leave some time between typing and opening the ComboBox anyway (maybe not 2 seconds but definitely the few milliseconds it takes now) so the list will always be current when opened.

Your problem is probably (I haven't checked this) that you're calling Dispatcher.CurrentDispatcher from the background thread. That gets the dispatcher for the background thread, which obviously is not the same as the UI dispatcher. In your form constructor, take a reference to the Dispatcher.CurrentDispatcher property and place it in a field, then use that instance from your background thread.

As to the difference between BeginInvoke and Invoke, BeginInvoke is asynchronous, meaning it will drop the message on the message queue and return, and leave something else to pump the messages off and respond to them. Since you dispatched to the background Dispatcher, the message will only get processed when the background thread yields and since you have a processor bound thread going on the only time it yields is when it shuts down, so nothing ever pumps that message.

Invoke is synchronous, meaning the thread will suspend until that message is processed. Because the thread yields, it is now free to process messages from the Dispatcher - the only one being the message you just posted. It processes the message and (because we're still on the background thread) goes pop on accessing the UI controls.

Thanks, I understand now. I passed along the CurrentDispatcher and invoke the method on that, that seems to work. There's something else going 'wrong' now though; it is clear that the parsing is now done on the background thread, typing is much less sluggish, but when I now open the ComboBox (which is bound to the Script.ParsedWords collection) it takes quite a long time to open it, almost as if it doesn't add any items until it needs to display them. I don't get this, adding the items wasn't this slow when I did it in the UI thread without any multithreading, why does it suddenly take such a long time?

So what I'm doing now is this:
- Start a background thread that parses the script into a collection of words
- When this parsing is complete, transfer the parsed words into a different collection of words in the window, to which the ComboBox is databound.

What I did before was just:
- Parse the script and for each word found immediately add it to the collection of words in the window.

I can't see the second option being faster?

[Evil_Giraffe]

Nope, I've been thinking about it and I can't think why that change should speed things up.

Maybe post the before/after code and something might jump out?