From 3be67d64685b96026c8a01c0caae01a0be50b8f3 Mon Sep 17 00:00:00 2001 From: Alex Crome Date: Sat, 14 Sep 2024 12:32:56 +0100 Subject: [PATCH] Parallelise Report Generator I've had a go at trying to parallelise the - aimed particularly at helping GitHub Actions / Azure DevOps where the report generator can be quite slow, presumably due to disk IO. I've read some issues about previous attempts and realised it's not as simple as processing classes in parallel as not all the `IReportBuilder` implements support concurrency. With this in mind, I believe two parts of the report generation process (`ReportGenerator`) can be parallelised 1. The initial File Analysis can be done concurrently with the report generation 2. Introduced `IParallelisableReportBuilder` - `IReportBuilder`s that also implement `IParallelisableReportBuilder` can then be processed in parallel this PR has a draft implementation of this, and with the change, I have been able to get report generation (measuring time spent in `Generator.GenerateReport`) down from 21 secs to down as low as 4 seconds with extreme concurrency, and 6secs with more reasonable concurrency levels. This PR is still a bit of a work in progress - in particular I need to plumb the concurrency level through as a config option, and I also want to do some testing with GitHub Actions / Azure Devops. However I wanted to raise it in it's current form for some initial thoughts on the approach. Note, this parallelisation change highly benefits form #TODO --- .../Builders/HtmlBlueRedReportBuilder.cs | 3 +- .../HtmlBlueRedSummaryReportBuilder.cs | 3 +- .../Builders/HtmlDarkReportBuilder.cs | 3 +- ...mlInlineAzurePipelinesDarkReportBuilder.cs | 3 +- ...lInlineAzurePipelinesLightReportBuilder.cs | 3 +- .../HtmlInlineAzurePipelinesReportBuilder.cs | 3 +- ...HtmlInlineCssAndJavaScriptReportBuilder.cs | 3 +- .../Builders/HtmlLightReportBuilder.cs | 3 +- .../Reporting/Builders/HtmlReportBuilder.cs | 3 +- .../Builders/HtmlReportBuilderBase.cs | 2 +- .../Builders/HtmlSummaryReportBuilder.cs | 3 +- .../Builders/Rendering/HtmlRenderer.cs | 9 +- .../Reporting/IParallelisableReportBuilder.cs | 7 + .../Reporting/ReportGenerator.cs | 134 +++++++++++++----- 14 files changed, 129 insertions(+), 53 deletions(-) create mode 100644 src/ReportGenerator.Core/Reporting/IParallelisableReportBuilder.cs diff --git a/src/ReportGenerator.Core/Reporting/Builders/HtmlBlueRedReportBuilder.cs b/src/ReportGenerator.Core/Reporting/Builders/HtmlBlueRedReportBuilder.cs index 8e2dfc31..b58b65dd 100644 --- a/src/ReportGenerator.Core/Reporting/Builders/HtmlBlueRedReportBuilder.cs +++ b/src/ReportGenerator.Core/Reporting/Builders/HtmlBlueRedReportBuilder.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using Palmmedia.ReportGenerator.Core.Parser.Analysis; @@ -18,7 +19,7 @@ public class HtmlBlueRedReportBuilder : HtmlReportBuilderBase /// /// Dictionary containing the filenames of the class reports by class. /// - private readonly IDictionary fileNameByClass = new Dictionary(); + private readonly ConcurrentDictionary fileNameByClass = new ConcurrentDictionary(); /// /// Initializes a new instance of the class. diff --git a/src/ReportGenerator.Core/Reporting/Builders/HtmlBlueRedSummaryReportBuilder.cs b/src/ReportGenerator.Core/Reporting/Builders/HtmlBlueRedSummaryReportBuilder.cs index 967a69f4..7725d340 100644 --- a/src/ReportGenerator.Core/Reporting/Builders/HtmlBlueRedSummaryReportBuilder.cs +++ b/src/ReportGenerator.Core/Reporting/Builders/HtmlBlueRedSummaryReportBuilder.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using Palmmedia.ReportGenerator.Core.Parser.Analysis; @@ -33,7 +34,7 @@ public override void CreateClassReport(Class @class, IEnumerable f /// The summary result. public override void CreateSummaryReport(SummaryResult summaryResult) { - using (var renderer = new HtmlRenderer(new Dictionary(), true, HtmlMode.InlineCssAndJavaScript, new string[] { "custom_adaptive.css", "custom_bluered.css" }, "custom.css")) + using (var renderer = new HtmlRenderer(new ConcurrentDictionary(), true, HtmlMode.InlineCssAndJavaScript, new string[] { "custom_adaptive.css", "custom_bluered.css" }, "custom.css")) { this.CreateSummaryReport(renderer, summaryResult); } diff --git a/src/ReportGenerator.Core/Reporting/Builders/HtmlDarkReportBuilder.cs b/src/ReportGenerator.Core/Reporting/Builders/HtmlDarkReportBuilder.cs index acd7f627..cb6f23a6 100644 --- a/src/ReportGenerator.Core/Reporting/Builders/HtmlDarkReportBuilder.cs +++ b/src/ReportGenerator.Core/Reporting/Builders/HtmlDarkReportBuilder.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using Palmmedia.ReportGenerator.Core.Parser.Analysis; @@ -18,7 +19,7 @@ public class HtmlDarkReportBuilder : HtmlReportBuilderBase /// /// Dictionary containing the filenames of the class reports by class. /// - private readonly IDictionary fileNameByClass = new Dictionary(); + private readonly ConcurrentDictionary fileNameByClass = new ConcurrentDictionary(); /// /// Initializes a new instance of the class. diff --git a/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineAzurePipelinesDarkReportBuilder.cs b/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineAzurePipelinesDarkReportBuilder.cs index d8f80eac..e7e2d734 100644 --- a/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineAzurePipelinesDarkReportBuilder.cs +++ b/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineAzurePipelinesDarkReportBuilder.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using Palmmedia.ReportGenerator.Core.Parser.Analysis; @@ -13,7 +14,7 @@ public class HtmlInlineAzurePipelinesDarkReportBuilder : HtmlReportBuilderBase /// /// Dictionary containing the filenames of the class reports by class. /// - private readonly IDictionary fileNameByClass = new Dictionary(); + private readonly ConcurrentDictionary fileNameByClass = new ConcurrentDictionary(); /// /// Gets the report type. diff --git a/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineAzurePipelinesLightReportBuilder.cs b/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineAzurePipelinesLightReportBuilder.cs index 2293b950..3c670a5a 100644 --- a/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineAzurePipelinesLightReportBuilder.cs +++ b/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineAzurePipelinesLightReportBuilder.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using Palmmedia.ReportGenerator.Core.Parser.Analysis; @@ -13,7 +14,7 @@ public class HtmlInlineAzurePipelinesLightReportBuilder : HtmlReportBuilderBase /// /// Dictionary containing the filenames of the class reports by class. /// - private readonly IDictionary fileNameByClass = new Dictionary(); + private readonly ConcurrentDictionary fileNameByClass = new ConcurrentDictionary(); /// /// Gets the report type. diff --git a/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineAzurePipelinesReportBuilder.cs b/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineAzurePipelinesReportBuilder.cs index 4f0dee4a..28f63239 100644 --- a/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineAzurePipelinesReportBuilder.cs +++ b/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineAzurePipelinesReportBuilder.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using Palmmedia.ReportGenerator.Core.Parser.Analysis; @@ -13,7 +14,7 @@ public class HtmlInlineAzurePipelinesReportBuilder : HtmlReportBuilderBase /// /// Dictionary containing the filenames of the class reports by class. /// - private readonly IDictionary fileNameByClass = new Dictionary(); + private readonly ConcurrentDictionary fileNameByClass = new ConcurrentDictionary(); /// /// Gets the report type. diff --git a/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineCssAndJavaScriptReportBuilder.cs b/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineCssAndJavaScriptReportBuilder.cs index fb994ee5..00964b09 100644 --- a/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineCssAndJavaScriptReportBuilder.cs +++ b/src/ReportGenerator.Core/Reporting/Builders/HtmlInlineCssAndJavaScriptReportBuilder.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using Palmmedia.ReportGenerator.Core.Parser.Analysis; @@ -13,7 +14,7 @@ public class HtmlInlineCssAndJavaScriptReportBuilder : HtmlReportBuilderBase /// /// Dictionary containing the filenames of the class reports by class. /// - private readonly IDictionary fileNameByClass = new Dictionary(); + private readonly ConcurrentDictionary fileNameByClass = new ConcurrentDictionary(); /// /// Gets the report type. diff --git a/src/ReportGenerator.Core/Reporting/Builders/HtmlLightReportBuilder.cs b/src/ReportGenerator.Core/Reporting/Builders/HtmlLightReportBuilder.cs index 15ae58d5..19c72a3d 100644 --- a/src/ReportGenerator.Core/Reporting/Builders/HtmlLightReportBuilder.cs +++ b/src/ReportGenerator.Core/Reporting/Builders/HtmlLightReportBuilder.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using Palmmedia.ReportGenerator.Core.Parser.Analysis; @@ -18,7 +19,7 @@ public class HtmlLightReportBuilder : HtmlReportBuilderBase /// /// Dictionary containing the filenames of the class reports by class. /// - private readonly IDictionary fileNameByClass = new Dictionary(); + private readonly ConcurrentDictionary fileNameByClass = new ConcurrentDictionary(); /// /// Initializes a new instance of the class. diff --git a/src/ReportGenerator.Core/Reporting/Builders/HtmlReportBuilder.cs b/src/ReportGenerator.Core/Reporting/Builders/HtmlReportBuilder.cs index 5d124c99..bfa38091 100644 --- a/src/ReportGenerator.Core/Reporting/Builders/HtmlReportBuilder.cs +++ b/src/ReportGenerator.Core/Reporting/Builders/HtmlReportBuilder.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using Palmmedia.ReportGenerator.Core.Parser.Analysis; @@ -18,7 +19,7 @@ public class HtmlReportBuilder : HtmlReportBuilderBase /// /// Dictionary containing the filenames of the class reports by class. /// - private readonly IDictionary fileNameByClass = new Dictionary(); + private readonly ConcurrentDictionary fileNameByClass = new ConcurrentDictionary(); /// /// Initializes a new instance of the class. diff --git a/src/ReportGenerator.Core/Reporting/Builders/HtmlReportBuilderBase.cs b/src/ReportGenerator.Core/Reporting/Builders/HtmlReportBuilderBase.cs index 08548072..4e12d9df 100644 --- a/src/ReportGenerator.Core/Reporting/Builders/HtmlReportBuilderBase.cs +++ b/src/ReportGenerator.Core/Reporting/Builders/HtmlReportBuilderBase.cs @@ -15,7 +15,7 @@ namespace Palmmedia.ReportGenerator.Core.Reporting.Builders /// /// Implementation of that uses to create reports. /// - public abstract class HtmlReportBuilderBase : IReportBuilder + public abstract class HtmlReportBuilderBase : IParallelisableReportBuilder { /// /// The Logger. diff --git a/src/ReportGenerator.Core/Reporting/Builders/HtmlSummaryReportBuilder.cs b/src/ReportGenerator.Core/Reporting/Builders/HtmlSummaryReportBuilder.cs index ba76f58c..8e3639cd 100644 --- a/src/ReportGenerator.Core/Reporting/Builders/HtmlSummaryReportBuilder.cs +++ b/src/ReportGenerator.Core/Reporting/Builders/HtmlSummaryReportBuilder.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using Palmmedia.ReportGenerator.Core.Parser.Analysis; @@ -33,7 +34,7 @@ public override void CreateClassReport(Class @class, IEnumerable f /// The summary result. public override void CreateSummaryReport(SummaryResult summaryResult) { - using (var renderer = new HtmlRenderer(new Dictionary(), true, HtmlMode.InlineCssAndJavaScript)) + using (var renderer = new HtmlRenderer(new ConcurrentDictionary(), true, HtmlMode.InlineCssAndJavaScript)) { this.CreateSummaryReport(renderer, summaryResult); } diff --git a/src/ReportGenerator.Core/Reporting/Builders/Rendering/HtmlRenderer.cs b/src/ReportGenerator.Core/Reporting/Builders/Rendering/HtmlRenderer.cs index 7d1309d3..736dce6d 100644 --- a/src/ReportGenerator.Core/Reporting/Builders/Rendering/HtmlRenderer.cs +++ b/src/ReportGenerator.Core/Reporting/Builders/Rendering/HtmlRenderer.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Globalization; using System.IO; @@ -58,7 +59,7 @@ internal class HtmlRenderer : IHtmlRenderer, IDisposable /// /// Dictionary containing the filenames of the class reports by class. /// - private readonly IDictionary fileNameByClass; + private readonly ConcurrentDictionary fileNameByClass; /// /// Indicates that only a summary report is created (no class reports). @@ -109,7 +110,7 @@ internal class HtmlRenderer : IHtmlRenderer, IDisposable /// Optional CSS file resource. /// Optional additional CSS file resource. internal HtmlRenderer( - IDictionary fileNameByClass, + ConcurrentDictionary fileNameByClass, bool onlySummary, HtmlMode htmlMode, string cssFileResource = "custom.css", @@ -132,7 +133,7 @@ internal HtmlRenderer( /// Optional additional CSS file resources. /// Optional CSS file resource. internal HtmlRenderer( - IDictionary fileNameByClass, + ConcurrentDictionary fileNameByClass, bool onlySummary, HtmlMode htmlMode, string[] additionalCssFileResources, @@ -1557,7 +1558,7 @@ private string GetClassReportFilename(Assembly assembly, string className) while (this.fileNameByClass.Values.Any(v => v.Equals(fileName, StringComparison.OrdinalIgnoreCase))); } - this.fileNameByClass.Add(key, fileName); + this.fileNameByClass[key] = fileName; } return fileName; diff --git a/src/ReportGenerator.Core/Reporting/IParallelisableReportBuilder.cs b/src/ReportGenerator.Core/Reporting/IParallelisableReportBuilder.cs new file mode 100644 index 00000000..f67f2805 --- /dev/null +++ b/src/ReportGenerator.Core/Reporting/IParallelisableReportBuilder.cs @@ -0,0 +1,7 @@ +namespace Palmmedia.ReportGenerator.Core.Reporting +{ + /// + /// Interface indicating that an can build multiple reports concurrently. + /// + public interface IParallelisableReportBuilder : IReportBuilder { } +} diff --git a/src/ReportGenerator.Core/Reporting/ReportGenerator.cs b/src/ReportGenerator.Core/Reporting/ReportGenerator.cs index 72999067..865e4648 100644 --- a/src/ReportGenerator.Core/Reporting/ReportGenerator.cs +++ b/src/ReportGenerator.Core/Reporting/ReportGenerator.cs @@ -1,6 +1,8 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Palmmedia.ReportGenerator.Core.Common; using Palmmedia.ReportGenerator.Core.Logging; @@ -16,6 +18,9 @@ namespace Palmmedia.ReportGenerator.Core.Reporting /// internal class ReportGenerator { + // TODO: Make this configurable + private const int MaxConcurrency = 8; + /// /// The Logger. /// @@ -34,7 +39,7 @@ internal class ReportGenerator /// /// The renderers. /// - private readonly IEnumerable renderers; + private readonly List renderers; /// /// Initializes a new instance of the class. @@ -46,7 +51,7 @@ internal ReportGenerator(IFileReader fileReader, ParserResult parserResult, IEnu { this.fileReader = fileReader ?? throw new ArgumentNullException(nameof(fileReader)); this.parserResult = parserResult ?? throw new ArgumentNullException(nameof(parserResult)); - this.renderers = renderers ?? throw new ArgumentNullException(nameof(renderers)); + this.renderers = renderers?.ToList() ?? throw new ArgumentNullException(nameof(renderers)); } /// @@ -58,54 +63,73 @@ internal ReportGenerator(IFileReader fileReader, ParserResult parserResult, IEnu /// The custom tag (e.g. build number). internal void CreateReport(bool addHistoricCoverage, List overallHistoricCoverages, DateTime executionTime, string tag) { + // TODO: Can we change overallHistoricCoverages to a ConcurrentBag to avoid this? + object overallHistoricCoveragesLock = new object(); + + // TODO: This probably belongs in the ReportGenerator Console and/or Global Tool + // AND can probably be a bit smarter. + // + // If there aren't enough threads in the thread pool, the Parallelism here can deadlock until the pool grows large enough + // With all avaialble threads being used in class analysis, but with ConcurrentReportBuilder threads having + // to wait for the thread pool to grow. By default, .Net core adds 2 threads every 0.5 secs + // App will become responsive within a few seconds, but given that report generator can complete in tens of seconds, + // these stalls become significant + ThreadPool.SetMinThreads(200, 200); + + var allClasses = this.parserResult.Assemblies.SelectMany(a => a.Classes); + var classAnalysis = Partitioner.Create(allClasses, EnumerablePartitionerOptions.NoBuffering) + .AsParallel() + .AsOrdered() + .WithDegreeOfParallelism(MaxConcurrency) + .Select(AnalyseClass) + .AsSequential(); + int numberOfClasses = this.parserResult.Assemblies.SafeSum(a => a.Classes.Count()); Logger.DebugFormat(Resources.AnalyzingClasses, numberOfClasses); int counter = 0; + var concurrentRenderers = this.renderers.OfType().ToList(); + var sequentialRenderers = this.renderers.Except(concurrentRenderers).ToList(); - foreach (var assembly in this.parserResult.Assemblies) + var concurrentRenderQueue = new BlockingCollection<(IReportBuilder renderer, Class @class, List analysis)>(MaxConcurrency); + Task concurrentRendererTask = Task.CompletedTask; + + if (concurrentRenderers.Any()) { - foreach (var @class in assembly.Classes) + concurrentRendererTask = Task.Factory.StartNew(() => { - counter++; - - Logger.DebugFormat( - Resources.CreatingReport, - counter, - numberOfClasses, - @class.Assembly.ShortName, - @class.Name); - - var fileAnalyses = @class.Files.Select(f => f.AnalyzeFile(this.fileReader)).ToArray(); - - if (addHistoricCoverage) - { - var historicCoverage = new HistoricCoverage(@class, executionTime, tag); - @class.AddHistoricCoverage(historicCoverage); - overallHistoricCoverages.Add(historicCoverage); - } + Partitioner.Create(concurrentRenderQueue.GetConsumingEnumerable(), EnumerablePartitionerOptions.NoBuffering) + .AsParallel() + .WithDegreeOfParallelism(MaxConcurrency) + .ForAll(x => RenderReport(x.renderer, x.@class, x.analysis)); + }); + } - Parallel.ForEach( - this.renderers, - renderer => - { - try - { - renderer.CreateClassReport(@class, fileAnalyses); - } - catch (Exception ex) - { - Logger.ErrorFormat( - Resources.ErrorDuringRenderingClassReport, - @class.Name, - renderer.ReportType, - ex.GetExceptionMessageForDisplay()); - } - }); + foreach (var (@class, analysis) in classAnalysis) + { + counter++; + Logger.DebugFormat( + Resources.CreatingReport, + counter, + numberOfClasses, + @class.Assembly.ShortName, + @class.Name); + + foreach (var renderer in concurrentRenderers) + { + concurrentRenderQueue.Add((renderer, @class, analysis)); } + + sequentialRenderers + .AsParallel() + .WithMergeOptions(ParallelMergeOptions.NotBuffered) + .ForAll(x => RenderReport(x, @class, analysis)); } + concurrentRenderQueue.CompleteAdding(); + concurrentRendererTask.Wait(); + Logger.Debug(Resources.CreatingSummary); SummaryResult summaryResult = new SummaryResult(this.parserResult); @@ -128,6 +152,40 @@ internal void CreateReport(bool addHistoricCoverage, List over } } } + + (Class @class, List analysis) AnalyseClass(Class @class) + { + var fileAnalyses = @class.Files.Select(f => f.AnalyzeFile(this.fileReader)).ToList(); + + if (addHistoricCoverage) + { + var historicCoverage = new HistoricCoverage(@class, executionTime, tag); + @class.AddHistoricCoverage(historicCoverage); + + lock (overallHistoricCoveragesLock) + { + overallHistoricCoverages.Add(historicCoverage); + } + } + + return (@class, fileAnalyses); + } + } + + private static void RenderReport(IReportBuilder renderer, Class @class, List analysis) + { + try + { + renderer.CreateClassReport(@class, analysis); + } + catch (Exception ex) + { + Logger.ErrorFormat( + Resources.ErrorDuringRenderingClassReport, + @class.Name, + renderer.ReportType, + ex.GetExceptionMessageForDisplay()); + } } } } \ No newline at end of file