Skip to content

Change list -> ConcurrentBag in BuildCheckContext #11461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 20, 2025
Merged

Change list -> ConcurrentBag in BuildCheckContext #11461

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d8b498
change list -> ConcurrentBag in buildcheckcontext
YuliiaKovalova Feb 19, 2025
f1894bd
handle the case with parallel removal and adding
YuliiaKovalova Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 54 additions & 19 deletions src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
Expand Down Expand Up @@ -65,14 +65,14 @@ internal sealed class BuildCheckManager : IBuildCheckManager, IBuildEngineDataRo
private readonly TracingReporter _tracingReporter = new TracingReporter();
private readonly IConfigurationProvider _configurationProvider = new ConfigurationProvider();
private readonly BuildCheckCentralContext _buildCheckCentralContext;
private readonly List<CheckFactoryContext> _checkRegistry;
private readonly ConcurrentBag<CheckFactoryContext> _checkRegistry;
private readonly bool[] _enabledDataSources = new bool[(int)BuildCheckDataSource.ValuesCount];
private readonly BuildEventsProcessor _buildEventsProcessor;
private readonly IBuildCheckAcquisitionModule _acquisitionModule;

internal BuildCheckManager()
{
_checkRegistry = new List<CheckFactoryContext>();
_checkRegistry = new ConcurrentBag<CheckFactoryContext>();
_acquisitionModule = new BuildCheckAcquisitionModule();
_buildCheckCentralContext = new(_configurationProvider, RemoveChecksAfterExecutedActions);
_buildEventsProcessor = new(_buildCheckCentralContext);
Expand Down Expand Up @@ -171,15 +171,23 @@ internal readonly record struct BuiltInCheckFactory(

private void RegisterBuiltInChecks(BuildCheckDataSource buildCheckDataSource)
{
_checkRegistry.AddRange(
s_builtInFactoriesPerDataSource[(int)buildCheckDataSource]
.Select(v => new CheckFactoryContext(v.Factory, v.RuleIds, v.DefaultEnablement)));
foreach (BuiltInCheckFactory item in s_builtInFactoriesPerDataSource[(int)buildCheckDataSource])
{
_checkRegistry.Add(new CheckFactoryContext(
item.Factory,
item.RuleIds,
item.DefaultEnablement));
}

if (s_testFactoriesPerDataSource is not null)
{
_checkRegistry.AddRange(
s_testFactoriesPerDataSource[(int)buildCheckDataSource]
.Select(v => new CheckFactoryContext(v.Factory, v.RuleIds, v.DefaultEnablement)));
foreach (BuiltInCheckFactory item in s_testFactoriesPerDataSource[(int)buildCheckDataSource])
{
_checkRegistry.Add(new CheckFactoryContext(
item.Factory,
item.RuleIds,
item.DefaultEnablement));
}
}
}

Expand Down Expand Up @@ -362,7 +370,8 @@ public void RemoveChecksAfterExecutedActions(List<CheckWrapper>? checksToRemove,
{
foreach (CheckWrapper check in checksToRemove)
{
var checkFactory = _checkRegistry.Find(c => c.MaterializedCheck == check);
var checkFactory = _checkRegistry.FirstOrDefault(c => c.MaterializedCheck == check);

if (checkFactory is not null)
{
checkContext.DispatchAsCommentFromText(MessageImportance.High, $"Dismounting check '{check.Check.FriendlyName}'. The check has thrown an unhandled exception while executing registered actions.");
Expand All @@ -371,7 +380,9 @@ public void RemoveChecksAfterExecutedActions(List<CheckWrapper>? checksToRemove,
}
}

foreach (var throttledCheck in _checkRegistry.FindAll(c => c.MaterializedCheck?.IsThrottled ?? false))
var throttledChecks = _checkRegistry.Where(c => c.MaterializedCheck?.IsThrottled ?? false).ToList();

foreach (var throttledCheck in throttledChecks)
{
checkContext.DispatchAsCommentFromText(MessageImportance.Normal, $"Dismounting check '{throttledCheck.FriendlyName}'. The check has exceeded the maximum number of results allowed. Any additional results will not be displayed.");
RemoveCheck(throttledCheck);
Expand All @@ -380,13 +391,33 @@ public void RemoveChecksAfterExecutedActions(List<CheckWrapper>? checksToRemove,

private void RemoveCheck(CheckFactoryContext checkToRemove)
{
_checkRegistry.Remove(checkToRemove);
var tempColl = new ConcurrentBag<CheckFactoryContext>();

// Take items one by one and only keep those we don't want to remove
while (_checkRegistry.TryTake(out var item))
{
if (item != checkToRemove)
{
tempColl.Add(item);
}
else if (item.MaterializedCheck is not null)
{
_buildCheckCentralContext.DeregisterCheck(item.MaterializedCheck);

var telemetryData = item.MaterializedCheck.GetRuleTelemetryData();
foreach (var data in telemetryData)
{
_ruleTelemetryData.Add(data);
}

item.MaterializedCheck.Check.Dispose();
}
}

if (checkToRemove.MaterializedCheck is not null)
// Add back all preserved items
foreach (var item in tempColl)
{
_buildCheckCentralContext.DeregisterCheck(checkToRemove.MaterializedCheck);
_ruleTelemetryData.AddRange(checkToRemove.MaterializedCheck.GetRuleTelemetryData());
checkToRemove.MaterializedCheck.Check.Dispose();
_checkRegistry.Add(item);
}
}

Expand Down Expand Up @@ -473,19 +504,23 @@ public void ProcessTaskParameterEventArgs(
=> _buildEventsProcessor
.ProcessTaskParameterEventArgs(checkContext, taskParameterEventArgs);

private readonly List<BuildCheckRuleTelemetryData> _ruleTelemetryData = [];
private readonly ConcurrentBag<BuildCheckRuleTelemetryData> _ruleTelemetryData = [];

public BuildCheckTracingData CreateCheckTracingStats()
{
foreach (CheckFactoryContext checkFactoryContext in _checkRegistry)
{
if (checkFactoryContext.MaterializedCheck != null)
{
_ruleTelemetryData.AddRange(checkFactoryContext.MaterializedCheck.GetRuleTelemetryData());
var telemetryData = checkFactoryContext.MaterializedCheck.GetRuleTelemetryData();
foreach (var data in telemetryData)
{
_ruleTelemetryData.Add(data);
}
}
}

return new BuildCheckTracingData(_ruleTelemetryData, _tracingReporter.GetInfrastructureTracingStats());
return new BuildCheckTracingData(_ruleTelemetryData.ToList(), _tracingReporter.GetInfrastructureTracingStats());
}

public void FinalizeProcessing(LoggingContext loggingContext)
Expand Down