Skip to content
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

Make TaskRegistry tasks ordering deterministic (FIFO) #8974

Merged
merged 3 commits into from
Jul 14, 2023
Merged

Make TaskRegistry tasks ordering deterministic (FIFO) #8974

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c141b2e
Make TaskRegistry tasks ordering deterministic (FIFO)
JanKrivanek Jun 28, 2023
213d0c0
Make the tasks sequence counter an instance of TaskRegistry
JanKrivanek Jun 29, 2023
79152b8
No-op change to kick cla bot
JanKrivanek Jun 30, 2023
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
1 change: 1 addition & 0 deletions src/Build.UnitTests/TestComparers/TaskRegistryComparers.cs
View file
Open in desktop
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
106 changes: 56 additions & 50 deletions src/Build/Instance/TaskRegistry.cs
View file
Open in desktop
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,6 +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