Collect parse results into a ConcurrentBag and add to parser state later

This removes the need for the lock on the parser state, which we used previously.

Author: MDoerner

Rubberduck.Parsing/VBA/Parsing/IModuleParser.cs

@@ -3,7 +3,6 @@
 using Antlr4.Runtime;
 using Antlr4.Runtime.Tree;
 using Rubberduck.Parsing.Annotations;
-using Rubberduck.Parsing.Rewriter;
 using Rubberduck.Parsing.Symbols;
 using Rubberduck.VBEditor;
 

Rubberduck.Parsing/VBA/Parsing/ParseRunner.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
@@ -21,26 +22,30 @@ public ParseRunner(
             parser)
         { }
 
-        public override void ParseModules(IReadOnlyCollection<QualifiedModuleName> modules, CancellationToken token)
+        protected override IReadOnlyCollection<(QualifiedModuleName module, ModuleParseResults results)> ModulePareResults(IReadOnlyCollection<QualifiedModuleName> modules, CancellationToken token)
         {
             if (!modules.Any())
             {
-                return;
+                return new List<(QualifiedModuleName module, ModuleParseResults results)>();
             }
 
             token.ThrowIfCancellationRequested();
 
             var parsingStageTimer = ParsingStageTimer.StartNew();
 
-            var options = new ParallelOptions();
-            options.CancellationToken = token;
-            options.MaxDegreeOfParallelism = _maxDegreeOfParserParallelism;
+            var results = new ConcurrentBag<(QualifiedModuleName module, ModuleParseResults results)>();
+
+            var options = new ParallelOptions
+            {
+                CancellationToken = token,
+                MaxDegreeOfParallelism = _maxDegreeOfParserParallelism
+            };
 
             try
             {
                 Parallel.ForEach(modules,
                     options,
-                    module => ParseModule(module, token)
+                    module => results.Add((module, ModuleParseResults(module, token)))
                 );
             }
             catch (AggregateException exception)
@@ -55,6 +60,8 @@ public override void ParseModules(IReadOnlyCollection<QualifiedModuleName> modul
 
             parsingStageTimer.Stop();
             parsingStageTimer.Log("Parsed user modules in {0}ms.");
+
+            return results;
         }
     }
 }

Rubberduck.Parsing/VBA/Parsing/ParseRunnerBase.cs

@@ -1,5 +1,7 @@
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading;
 using Rubberduck.Parsing.VBA.Parsing.ParsingExceptions;
 using Rubberduck.VBEditor;
@@ -36,23 +38,27 @@ protected ParseRunnerBase(
             _parser = parser;
         }
 
+        public void ParseModules(IReadOnlyCollection<QualifiedModuleName> modules, CancellationToken token)
+        {
+            var parseResults = ModulePareResults(modules, token);
+            SaveModuleParseResultsOnState(parseResults, token);
+        }
 
-        public abstract void ParseModules(IReadOnlyCollection<QualifiedModuleName> modules, CancellationToken token);
-
+        protected abstract IReadOnlyCollection<(QualifiedModuleName module, ModuleParseResults results)> ModulePareResults(IReadOnlyCollection<QualifiedModuleName> modules, CancellationToken token);
 
-        protected void ParseModule(QualifiedModuleName module, CancellationToken token)
+        protected ModuleParseResults ModuleParseResults(QualifiedModuleName module, CancellationToken token)
         {
             _state.ClearStateCache(module);
             try
             {
-                var parseResults = _parser.Parse(module, token);
-                SaveModuleParseResultsOnState(module, parseResults, token);
+                return _parser.Parse(module, token);
             }
             catch (SyntaxErrorException syntaxErrorException)
             {
                 //In contrast to the situation in the success scenario, the overall parser state is reevaluated immediately.
-                //This sets the state directly on the state because it is the sole instance where we have to pass the SyntaxErorException.
+                //This sets the state directly on the state because it is the sole instance where we have to pass the SyntaxErrorException.
                 _state.SetModuleState(module, ParserState.Error, token, syntaxErrorException);
+                return default;
             }
             catch (Exception exception)
             {
@@ -61,30 +67,40 @@ protected void ParseModule(QualifiedModuleName module, CancellationToken token)
             }
         }
 
-        private void SaveModuleParseResultsOnState(QualifiedModuleName module, ModuleParseResults results, CancellationToken token)
+        private void SaveModuleParseResultsOnState(IReadOnlyCollection<(QualifiedModuleName module, ModuleParseResults results)> parseResults, CancellationToken token)
         {
-            lock (_state)
+            foreach (var (module, result) in parseResults)
             {
-                token.ThrowIfCancellationRequested();
+                if (result.CodePaneParseTree == null)
+                {
+                    continue;
+                }
 
-                //This has to come first because it creates the module state if not present.
-                _state.AddModuleStateIfNotPresent(module);
+                SaveModuleParseResultsOnState(module, result, token);
+            }
+        }
 
-                _state.SaveContentHash(module);
-                _state.AddParseTree(module, results.CodePaneParseTree);
-                _state.AddParseTree(module, results.AttributesParseTree, CodeKind.AttributesCode);
-                _state.SetModuleComments(module, results.Comments);
-                _state.SetModuleAnnotations(module, results.Annotations);
-                _state.SetModuleAttributes(module, results.Attributes);
-                _state.SetMembersAllowingAttributes(module, results.MembersAllowingAttributes);
-                _state.SetCodePaneTokenStream(module, results.CodePaneTokenStream);
-                _state.SetAttributesTokenStream(module, results.AttributesTokenStream);
+        private void SaveModuleParseResultsOnState(QualifiedModuleName module, ModuleParseResults results, CancellationToken token)
+        {
+            token.ThrowIfCancellationRequested();
 
-                // This really needs to go last
-                //It does not reevaluate the overall parer state to avoid concurrent evaluation of all module states and for performance reasons.
-                //The evaluation has to be triggered manually in the calling procedure.
-                StateManager.SetModuleState(module, ParserState.Parsed, token, false); //Note that this is ok because locks allow re-entrancy.
-            }
+            //This has to come first because it creates the module state if not present.
+            _state.AddModuleStateIfNotPresent(module);
+
+            _state.SaveContentHash(module);
+            _state.AddParseTree(module, results.CodePaneParseTree);
+            _state.AddParseTree(module, results.AttributesParseTree, CodeKind.AttributesCode);
+            _state.SetModuleComments(module, results.Comments);
+            _state.SetModuleAnnotations(module, results.Annotations);
+            _state.SetModuleAttributes(module, results.Attributes);
+            _state.SetMembersAllowingAttributes(module, results.MembersAllowingAttributes);
+            _state.SetCodePaneTokenStream(module, results.CodePaneTokenStream);
+            _state.SetAttributesTokenStream(module, results.AttributesTokenStream);
+
+            // This really needs to go last
+            //It does not reevaluate the overall parer state to avoid concurrent evaluation of all module states and for performance reasons.
+            //The evaluation has to be triggered manually in the calling procedure.
+            StateManager.SetModuleState(module, ParserState.Parsed, token,false); //Note that this is ok because locks allow re-entrancy.
         }
     }
 }

Rubberduck.Parsing/VBA/Parsing/SynchronousParseRunner.cs

@@ -18,20 +18,22 @@ public SynchronousParseRunner(
         { }
 
 
-        public override void ParseModules(IReadOnlyCollection<QualifiedModuleName> modules, CancellationToken token)
+        protected override IReadOnlyCollection<(QualifiedModuleName module, ModuleParseResults results)> ModulePareResults(IReadOnlyCollection<QualifiedModuleName> modules, CancellationToken token)
         {
             if (!modules.Any())
             {
-                return;
+                return new List<(QualifiedModuleName module, ModuleParseResults results)>();
             }
 
             token.ThrowIfCancellationRequested();
 
+            var results = new List<(QualifiedModuleName module, ModuleParseResults results)>();
+
             try
             {
                 foreach (var module in modules)
                 {
-                    ParseModule(module, token);
+                    results.Add((module, ModuleParseResults(module, token)));
                 }
             }
             catch (OperationCanceledException)
@@ -43,6 +45,8 @@ public override void ParseModules(IReadOnlyCollection<QualifiedModuleName> modul
                 StateManager.SetStatusAndFireStateChanged(this, ParserState.Error, token);
                 throw;
             }
+
+            return results;
         }
     }
 }