Skip to content

Commit f4196fc

Browse files
Fix comparison in TaskHostParameters (#12733)
This change introduces a new read only struct that is used in Dictionary for caching in TaskRegistry #12620 Without a proper equality check it spawns a new process due to mismatch. Experimental insertion without extra allocation reported: https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/686770
1 parent a0e8539 commit f4196fc

File tree

2 files changed

+28
-6
lines changed

2 files changed

+28
-6
lines changed

src/Build/Instance/TaskRegistry.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -950,18 +950,16 @@ private static bool IdentityParametersMatch(in TaskHostParameters x, in TaskHost
950950
return false;
951951
}
952952

953-
// For exact match, all properties must be equal (case-insensitive)
953+
// For exact match, only identity properties must be equal
954+
// Paths are NOT part of identity - they're just resolved locations
954955
return string.Equals(x.Runtime, y.Runtime, StringComparison.OrdinalIgnoreCase) &&
955956
string.Equals(x.Architecture, y.Architecture, StringComparison.OrdinalIgnoreCase) &&
956-
string.Equals(x.DotnetHostPath, y.DotnetHostPath, StringComparison.OrdinalIgnoreCase) &&
957-
string.Equals(x.MSBuildAssemblyPath, y.MSBuildAssemblyPath, StringComparison.OrdinalIgnoreCase) &&
958957
x.TaskHostFactoryExplicitlyRequested == y.TaskHostFactoryExplicitlyRequested;
959958
}
960959
else
961960
{
962961
// Fuzzy match: null is treated as "don't care"
963962
// Only check runtime and architecture for fuzzy matching
964-
965963
string runtimeX = x.Runtime;
966964
string runtimeY = y.Runtime;
967965
string architectureX = x.Architecture;

src/Framework/TaskHostParameters.cs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System;
45
using System.Collections.Generic;
56

67
namespace Microsoft.Build.Framework
78
{
89
/// <summary>
910
/// A readonly struct that represents task host parameters used to determine which host process to launch.
1011
/// </summary>
11-
public readonly struct TaskHostParameters
12+
public readonly struct TaskHostParameters : IEquatable<TaskHostParameters>
1213
{
1314
/// <summary>
1415
/// A static empty instance to avoid allocations when default parameters are needed.
@@ -72,6 +73,29 @@ internal TaskHostParameters(
7273
/// </summary>
7374
public bool? TaskHostFactoryExplicitlyRequested => _taskHostFactoryExplicitlyRequested;
7475

76+
public override bool Equals(object? obj) => obj is TaskHostParameters other && Equals(other);
77+
78+
public bool Equals(TaskHostParameters other) =>
79+
StringComparer.OrdinalIgnoreCase.Equals(Runtime ?? string.Empty, other.Runtime ?? string.Empty)
80+
&& StringComparer.OrdinalIgnoreCase.Equals(Architecture ?? string.Empty, other.Architecture ?? string.Empty)
81+
&& TaskHostFactoryExplicitlyRequested == other.TaskHostFactoryExplicitlyRequested;
82+
83+
public override int GetHashCode()
84+
{
85+
// Manual hash code implementation for compatibility with .NET Framework 4.7.2
86+
var comparer = StringComparer.OrdinalIgnoreCase;
87+
88+
unchecked
89+
{
90+
int hash = 17;
91+
hash = hash * 31 + comparer.GetHashCode(Runtime ?? string.Empty);
92+
hash = hash * 31 + comparer.GetHashCode(Architecture ?? string.Empty);
93+
hash = hash * 31 + (TaskHostFactoryExplicitlyRequested?.GetHashCode() ?? 0);
94+
95+
return hash;
96+
}
97+
}
98+
7599
/// <summary>
76100
/// Gets a value indicating whether returns true if parameters were unset.
77101
/// </summary>
@@ -134,7 +158,7 @@ internal TaskHostParameters WithTaskHostFactoryExplicitlyRequested(bool taskHost
134158
/// <summary>
135159
/// The method was added to sustain compatibility with ITaskFactory2 factoryIdentityParameters parameters dictionary.
136160
/// </summary>
137-
internal Dictionary<string, string> ToDictionary() => new(3)
161+
internal Dictionary<string, string> ToDictionary() => new(3, StringComparer.OrdinalIgnoreCase)
138162
{
139163
{ nameof(Runtime), Runtime ?? string.Empty },
140164
{ nameof(Architecture), Architecture ?? string.Empty },

0 commit comments

Comments
 (0)