patterncsharpMinor
RubberduckParser 2.0 - Asynchronous Parsing
Viewed 0 times
asynchronousparsingrubberduckparser
Problem
The
This new interface does more than breaking all the existing unit tests and pretty much every single feature: it flips things around and centralizes parser state, which can now be shared amongst all features... and instead of being fired up by the many features, it's now activated by a keyhook that captures keypresses in the VBE and starts parser tasks asynchronously - features can be disabled while the parser is working in the background, and re-enabled when it's in a "ready" state... without ever annoying the user with a blocking call.
Here's the implementation:
```
public class RubberduckParser : IRubberduckParser
{
private readonly VBE _vbe;
private readonly Logger _logger;
public RubberduckParser(VBE vbe, RubberduckParserState state)
{
_vbe = vbe;
_state = state;
_logger = LogManager.GetCurrentClassLogger();
}
private readonly RubberduckParserState _state;
public RubberduckParserState State { get { return _state; } }
public async Task ParseAsync(VBComponent vbComponent, CancellationToken token)
{
var component = vbComponent;
var parseTask = Task.Run(() => ParseInternal(component, token), token);
try
{
await parseTask;
}
catch (SyntaxErrorException exception)
{
State.SetModuleState(component, ParserState.Error, exception);
}
catch (OperationCanceledException)
{
// no need to blow up
}
}
public void Resolve(CancellationToken token)
{
var options = new ParallelOptions { CancellationToken = token };
Parallel.ForEach(_state.ParseTrees, options, kvp =>
{
IRubberduckParser interface has seen breaking changes, and now looks like this:public interface IRubberduckParser
{
RubberduckParserState State { get; }
Task ParseAsync(VBComponent component, CancellationToken token);
void Resolve(CancellationToken token);
}This new interface does more than breaking all the existing unit tests and pretty much every single feature: it flips things around and centralizes parser state, which can now be shared amongst all features... and instead of being fired up by the many features, it's now activated by a keyhook that captures keypresses in the VBE and starts parser tasks asynchronously - features can be disabled while the parser is working in the background, and re-enabled when it's in a "ready" state... without ever annoying the user with a blocking call.
Here's the implementation:
```
public class RubberduckParser : IRubberduckParser
{
private readonly VBE _vbe;
private readonly Logger _logger;
public RubberduckParser(VBE vbe, RubberduckParserState state)
{
_vbe = vbe;
_state = state;
_logger = LogManager.GetCurrentClassLogger();
}
private readonly RubberduckParserState _state;
public RubberduckParserState State { get { return _state; } }
public async Task ParseAsync(VBComponent vbComponent, CancellationToken token)
{
var component = vbComponent;
var parseTask = Task.Run(() => ParseInternal(component, token), token);
try
{
await parseTask;
}
catch (SyntaxErrorException exception)
{
State.SetModuleState(component, ParserState.Error, exception);
}
catch (OperationCanceledException)
{
// no need to blow up
}
}
public void Resolve(CancellationToken token)
{
var options = new ParallelOptions { CancellationToken = token };
Parallel.ForEach(_state.ParseTrees, options, kvp =>
{
Solution
Nested ternary conditionals are ugly. Ternary conditionals nested three deep are ugly to the third power:
I would use
You can combine these
You can combine these conditions, and be sure to put braces on the body:
Because you usually use braces, I would guess those are from accepting R# suggestions. You can turn braces on in the settings by going to the Options and choosing Code Editing -> C# -> Formatting Style -> Braces Layout and changing the settings in the Force Braces group.
There is a bug in here in which the resolver is started before the parser finishes, causing messed up results. The parser must be finished before the resolver starts. This is accomplished by changing the following method:
First, we might as well check for a cancellation request. Add this line before both statements involving
Next, change the
The
Status = _moduleStates.Values.Any(value => value == ParserState.Error)
? ParserState.Error
: _moduleStates.Values.Any(value => value == ParserState.Parsing)
? ParserState.Parsing
: _moduleStates.Values.Any(value => value == ParserState.Resolving)
? ParserState.Resolving
: ParserState.Ready;I would use
ifs here. TopinFrassi has already addressed the issue that this may be unnecessarily expensive.You can combine these
ifs into one condition:if (procedureCall != null)
{
if (procedureCall.CALL() != null)
{
_contexts.Add(context);
return;
}
}You can combine these conditions, and be sure to put braces on the body:
if (memberCall == null) return;
if (memberCall.CALL() == null) return;Because you usually use braces, I would guess those are from accepting R# suggestions. You can turn braces on in the settings by going to the Options and choosing Code Editing -> C# -> Formatting Style -> Braces Layout and changing the settings in the Force Braces group.
There is a bug in here in which the resolver is started before the parser finishes, causing messed up results. The parser must be finished before the resolver starts. This is accomplished by changing the following method:
public async Task ParseAsync(VBComponent vbComponent, CancellationToken token)
{
var component = vbComponent;
var parseTask = Task.Run(() => ParseInternal(component, token), token);
try
{
await parseTask;
}
catch (SyntaxErrorException exception)
{
State.SetModuleState(component, ParserState.Error, exception);
}
catch (OperationCanceledException)
{
// no need to blow up
}
}First, we might as well check for a cancellation request. Add this line before both statements involving
parseTask:token.ThrowIfCancellationRequested();Next, change the
await parseTask; statement to:parseTask.Wait(token);The
async modifier is now unnecessary. Because this method is called asynchronously, the parser will still run asynchronously, but the resolver cannot start until the parse task completes.Code Snippets
Status = _moduleStates.Values.Any(value => value == ParserState.Error)
? ParserState.Error
: _moduleStates.Values.Any(value => value == ParserState.Parsing)
? ParserState.Parsing
: _moduleStates.Values.Any(value => value == ParserState.Resolving)
? ParserState.Resolving
: ParserState.Ready;if (procedureCall != null)
{
if (procedureCall.CALL() != null)
{
_contexts.Add(context);
return;
}
}if (memberCall == null) return;
if (memberCall.CALL() == null) return;public async Task ParseAsync(VBComponent vbComponent, CancellationToken token)
{
var component = vbComponent;
var parseTask = Task.Run(() => ParseInternal(component, token), token);
try
{
await parseTask;
}
catch (SyntaxErrorException exception)
{
State.SetModuleState(component, ParserState.Error, exception);
}
catch (OperationCanceledException)
{
// no need to blow up
}
}token.ThrowIfCancellationRequested();Context
StackExchange Code Review Q#111469, answer score: 7
Revisions (0)
No revisions yet.