Skip to content

Commit

Permalink
Allow concurrent evaluation BuildCheck processing (#10959)
Browse files Browse the repository at this point in the history
Fixes #10956 by moving to a thread-safe dictionary.

These locks shouldn't ever really contend, because evaluation is single-
threaded and the events should generally come in order for a single
project, so I suspect locking is cheaper than switching the values to a
ConcurrentHashSet.
  • Loading branch information
rainersigwald authored Nov 8, 2024
1 parent e4f527b commit 77312bc
Showing 1 changed file with 16 additions and 11 deletions.
27 changes: 16 additions & 11 deletions src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ private void SetupSingleCheck(CheckFactoryContext checkFactoryContext, string pr
// For custom checks - it should run only on projects where referenced
// (otherwise error out - https://github.com/orgs/dotnet/projects/373/views/1?pane=issue&itemId=57849480)
// on others it should work similarly as disabling them.
// Disabled check should not only post-filter results - it shouldn't even see the data
// Disabled check should not only post-filter results - it shouldn't even see the data
CheckWrapper wrapper;
CheckConfigurationEffective[] configurations;
if (checkFactoryContext.MaterializedCheck == null)
Expand Down Expand Up @@ -376,9 +376,12 @@ public void ProcessEvaluationFinishedEventArgs(
{
if (importedProjects != null && TryGetProjectFullPath(checkContext.BuildEventContext, out string projectPath))
{
foreach (string importedProject in importedProjects)
lock (importedProjects)
{
_buildEventsProcessor.ProcessProjectImportedEventArgs(checkContext, projectPath, importedProject);
foreach (string importedProject in importedProjects)
{
_buildEventsProcessor.ProcessProjectImportedEventArgs(checkContext, projectPath, importedProject);
}
}
}
}
Expand Down Expand Up @@ -467,7 +470,7 @@ public void FinalizeProcessing(LoggingContext loggingContext)
private readonly ConcurrentDictionary<int, string> _projectsByInstanceId = new();
private readonly ConcurrentDictionary<int, string> _projectsByEvaluationId = new();

private readonly Dictionary<int, HashSet<string>> _deferredProjectEvalIdToImportedProjects = new();
private readonly ConcurrentDictionary<int, HashSet<string>> _deferredProjectEvalIdToImportedProjects = new();

/// <summary>
/// This method fetches the project full path from the context id.
Expand Down Expand Up @@ -539,10 +542,7 @@ public void ProcessProjectEvaluationStarted(
string projectFullPath)
{
_projectsByEvaluationId[checkContext.BuildEventContext.EvaluationId] = projectFullPath;
if (!_deferredProjectEvalIdToImportedProjects.ContainsKey(checkContext.BuildEventContext.EvaluationId))
{
_deferredProjectEvalIdToImportedProjects.Add(checkContext.BuildEventContext.EvaluationId, [projectFullPath]);
}
_deferredProjectEvalIdToImportedProjects.TryAdd(checkContext.BuildEventContext.EvaluationId, [projectFullPath]);
}

/*
Expand Down Expand Up @@ -586,10 +586,15 @@ public void StartProjectRequest(ICheckContext checkContext, string projectFullPa
/// <param name="newImportedProjectFile">The path of the newly imported project file.</param>
private void PropagateImport(int evaluationId, string originalProjectFile, string newImportedProjectFile)
{
if (_deferredProjectEvalIdToImportedProjects.TryGetValue(evaluationId, out HashSet<string>? importedProjects)
&& importedProjects.Contains(originalProjectFile))
if (_deferredProjectEvalIdToImportedProjects.TryGetValue(evaluationId, out HashSet<string>? importedProjects))
{
importedProjects.Add(newImportedProjectFile);
lock (importedProjects)
{
if (importedProjects.Contains(originalProjectFile))
{
importedProjects.Add(newImportedProjectFile);
}
}
}
}

Expand Down

0 comments on commit 77312bc

Please sign in to comment.