Skip to content

Commit

Permalink
Merge pull request #8974 from JanKrivanek/proto/TaskRegistryOrdering
Browse files Browse the repository at this point in the history 
Make TaskRegistry tasks ordering deterministic (FIFO)
  • Loading branch information
@JanKrivanek
JanKrivanek authored Jul 14, 2023
2 parents 4989625 + 79152b8 commit 29a2f07
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 51 deletions.
1 change: 1 addition & 0 deletions src/Build.UnitTests/TestComparers/TaskRegistryComparers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ internal sealed class TaskRegistryComparer : IEqualityComparer<TaskRegistry>
public bool Equals(TaskRegistry x, TaskRegistry y)
{
Assert.Equal(x.Toolset, y.Toolset, new ToolsetComparer());
Assert.Equal(x.NextRegistrationOrderId, y.NextRegistrationOrderId);

Helpers.AssertDictionariesEqual(
x.TaskRegistrations,
Expand Down
107 changes: 56 additions & 51 deletions src/Build/Instance/TaskRegistry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
using System.Collections.ObjectModel;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Threading;
using Microsoft.Build.BackEnd;
using Microsoft.Build.Collections;
using Microsoft.Build.Construction;
Expand Down Expand Up @@ -122,6 +124,11 @@ internal sealed class TaskRegistry : ITranslatable
/// </summary>
private static string s_potentialTasksCoreLocation = Path.Combine(BuildEnvironmentHelper.Instance.CurrentMSBuildToolsDirectory, s_tasksCoreFilename);

/// <summary>
/// Monotonically increasing counter for registered tasks.
/// </summary>
private int _nextRegistrationOrderId = 0;

/// <summary>
/// Cache of tasks already found using exact matching,
/// keyed by the task identity requested.
Expand Down Expand Up @@ -192,6 +199,12 @@ internal Toolset Toolset
{ return _toolset; }
}

/// <summary>
/// Access the next registration sequence id.
/// FOR UNIT TESTING ONLY.
/// </summary>
internal int NextRegistrationOrderId => _nextRegistrationOrderId;

/// <summary>
/// Access list of task registrations.
/// FOR UNIT TESTING ONLY.
Expand Down Expand Up @@ -525,23 +538,11 @@ internal RegisteredTaskRecord GetTaskRegistrationRecord(
}
}

Dictionary<RegisteredTaskIdentity, List<RegisteredTaskRecord>> registrations = GetRelevantRegistrations(taskIdentity, exactMatchRequired);
IEnumerable<RegisteredTaskRecord> registrations = GetRelevantOrderedRegistrations(taskIdentity, exactMatchRequired);

// look for the given task name in the registry; if not found, gather all registered task names that partially
// match the given name
foreach (KeyValuePair<RegisteredTaskIdentity, List<RegisteredTaskRecord>> registration in registrations)
{
// if the given task name is longer than the registered task name
// we will use the longer name to help disambiguate between multiple matches
string mostSpecificTaskName = (taskName.Length > registration.Key.Name.Length) ? taskName : registration.Key.Name;

taskRecord = GetMatchingRegistration(mostSpecificTaskName, registration.Value, taskProjectFile, taskIdentityParameters, targetLoggingContext, elementLocation);

if (taskRecord != null)
{
break;
}
}
taskRecord = GetMatchingRegistration(taskName, registrations, taskProjectFile, taskIdentityParameters, targetLoggingContext, elementLocation);
}

// If we didn't find the task but we have a fallback registry in the toolset state, try that one.
Expand Down Expand Up @@ -603,36 +604,24 @@ private static bool IsTaskFactoryClass(Type type, object unused)
/// If no exact match is found, looks for partial matches.
/// A task name that is not fully qualified may produce several partial matches.
/// </summary>
private Dictionary<RegisteredTaskIdentity, List<RegisteredTaskRecord>> GetRelevantRegistrations(RegisteredTaskIdentity taskIdentity, bool exactMatchRequired)
private IEnumerable<RegisteredTaskRecord> GetRelevantOrderedRegistrations(RegisteredTaskIdentity taskIdentity, bool exactMatchRequired)
{
Dictionary<RegisteredTaskIdentity, List<RegisteredTaskRecord>> relevantTaskRegistrations =
new Dictionary<RegisteredTaskIdentity, List<RegisteredTaskRecord>>(RegisteredTaskIdentity.RegisteredTaskIdentityComparer.Exact);

List<RegisteredTaskRecord> taskAssemblies;

// if we find an exact match
if (_taskRegistrations.TryGetValue(taskIdentity, out taskAssemblies))
if (_taskRegistrations.TryGetValue(taskIdentity, out List<RegisteredTaskRecord> taskAssemblies))
{
// we're done
relevantTaskRegistrations[taskIdentity] = taskAssemblies;
return relevantTaskRegistrations;
// (records for single key should be ordered by order of registrations - as they are inserted into the list)
return taskAssemblies;
}

if (exactMatchRequired)
{
return relevantTaskRegistrations;
return Enumerable.Empty<RegisteredTaskRecord>();
}

// look through all task declarations for partial matches
foreach (KeyValuePair<RegisteredTaskIdentity, List<RegisteredTaskRecord>> taskRegistration in _taskRegistrations)
{
if (RegisteredTaskIdentity.RegisteredTaskIdentityComparer.IsPartialMatch(taskIdentity, taskRegistration.Key))
{
relevantTaskRegistrations[taskRegistration.Key] = taskRegistration.Value;
}
}

return relevantTaskRegistrations;
return _taskRegistrations
.Where(tp => RegisteredTaskIdentity.RegisteredTaskIdentityComparer.IsPartialMatch(taskIdentity, tp.Key))
.SelectMany(tp => tp.Value)
.OrderBy(r => r.RegistrationOrderId);
}

// Create another set containing architecture-specific task entries.
Expand Down Expand Up @@ -673,7 +662,13 @@ private void RegisterTask(
_taskRegistrations[taskIdentity] = registeredTaskEntries;
}

RegisteredTaskRecord newRecord = new RegisteredTaskRecord(taskName, assemblyLoadInfo, taskFactory, taskFactoryParameters, inlineTaskRecord);
RegisteredTaskRecord newRecord = new RegisteredTaskRecord(
taskName,
assemblyLoadInfo,
taskFactory,
taskFactoryParameters,
inlineTaskRecord,
Interlocked.Increment(ref _nextRegistrationOrderId));

if (overrideTask)
{
Expand Down Expand Up @@ -721,23 +716,21 @@ private static Dictionary<RegisteredTaskIdentity, List<RegisteredTaskRecord>> Cr
/// </summary>
private RegisteredTaskRecord GetMatchingRegistration(
string taskName,
List<RegisteredTaskRecord> taskRecords,
IEnumerable<RegisteredTaskRecord> taskRecords,
string taskProjectFile,
IDictionary<string, string> taskIdentityParameters,
TargetLoggingContext targetLoggingContext,
ElementLocation elementLocation)
{
foreach (RegisteredTaskRecord record in taskRecords)
{
if (record.CanTaskBeCreatedByFactory(taskName, taskProjectFile, taskIdentityParameters, targetLoggingContext, elementLocation))
{
return record;
}
}

// Cannot find the task in any of the records
return null;
}
=>
taskRecords.FirstOrDefault(r =>
r.CanTaskBeCreatedByFactory(
// if the given task name is longer than the registered task name
// we will use the longer name to help disambiguate between multiple matches
(taskName.Length > r.TaskIdentity.Name.Length) ? taskName : r.TaskIdentity.Name,
taskProjectFile,
taskIdentityParameters,
targetLoggingContext,
elementLocation));

/// <summary>
/// An object representing the identity of a task -- not just task name, but also
Expand Down Expand Up @@ -1115,17 +1108,23 @@ internal class RegisteredTaskRecord : ITranslatable
/// </summary>
private ParameterGroupAndTaskElementRecord _parameterGroupAndTaskBody;

/// <summary>
/// The registration order id for this task. This is used to determine the order in which tasks are registered.
/// </summary>
private int _registrationOrderId;

/// <summary>
/// Constructor
/// </summary>
internal RegisteredTaskRecord(string registeredName, AssemblyLoadInfo assemblyLoadInfo, string taskFactory, Dictionary<string, string> taskFactoryParameters, ParameterGroupAndTaskElementRecord inlineTask)
internal RegisteredTaskRecord(string registeredName, AssemblyLoadInfo assemblyLoadInfo, string taskFactory, Dictionary<string, string> taskFactoryParameters, ParameterGroupAndTaskElementRecord inlineTask, int registrationOrderId)
{
ErrorUtilities.VerifyThrowArgumentNull(assemblyLoadInfo, "AssemblyLoadInfo");
_registeredName = registeredName;
_taskFactoryAssemblyLoadInfo = assemblyLoadInfo;
_taskFactoryParameters = taskFactoryParameters;
_taskIdentity = new RegisteredTaskIdentity(registeredName, taskFactoryParameters);
_parameterGroupAndTaskBody = inlineTask;
_registrationOrderId = registrationOrderId;

if (String.IsNullOrEmpty(taskFactory))
{
Expand Down Expand Up @@ -1206,6 +1205,11 @@ internal ParameterGroupAndTaskElementRecord ParameterGroupAndTaskBody
/// </summary>
internal RegisteredTaskIdentity TaskIdentity => _taskIdentity;

/// <summary>
/// The registration order id for this task. This is used to determine the order in which tasks are registered.
/// </summary>
internal int RegistrationOrderId => _registrationOrderId;

/// <summary>
/// Ask the question, whether or not the task name can be created by the task factory.
/// To answer this question we need to instantiate and initialize the task factory and ask it if it can create the given task name.
Expand Down Expand Up @@ -1765,6 +1769,7 @@ public void Translate(ITranslator translator)
translator.Translate(ref _taskFactoryAssemblyLoadInfo, AssemblyLoadInfo.FactoryForTranslation);
translator.Translate(ref _taskFactory);
translator.Translate(ref _parameterGroupAndTaskBody);
translator.Translate(ref _registrationOrderId);

IDictionary<string, string> localParameters = _taskFactoryParameters;
translator.TranslateDictionary(ref localParameters, count => CreateTaskFactoryParametersDictionary(count));
Expand All @@ -1787,7 +1792,7 @@ internal static RegisteredTaskRecord FactoryForDeserialization(ITranslator trans
public void Translate(ITranslator translator)
{
translator.Translate(ref _toolset, Toolset.FactoryForDeserialization);

translator.Translate(ref _nextRegistrationOrderId);
IDictionary<RegisteredTaskIdentity, List<RegisteredTaskRecord>> copy = _taskRegistrations;
translator.TranslateDictionary(ref copy, TranslateTaskRegistrationKey, TranslateTaskRegistrationValue, count => CreateRegisteredTaskDictionary(count));

Expand Down

0 comments on commit 29a2f07

Please sign in to comment.