dotnet / dotnet/msbuild

MSBuildNameIgnoreCaseComparer might be optimizable

Open
#10,926 0 comments 0 reactions 0 assignees View on GitHub
Area: Performance Priority:2 triaged
Dominant language
C#
Stars
5.5k
Forks
1.5k
Avg merge
1d 8h
Merged PRs (30d)
141

Description

tl;dr: I don't think we should change anything now but I did some perf investigation on `MSBuildNameIgnoreCaseComparer`.

https://github.com/dotnet/msbuild/blob/0cce533e766000b2f6218332447c38ab84985070/src/Shared/MSBuildNameIgnoreCaseComparer.cs#L13-L20

. . . but does that stuff hold true any more? There's been a bit of change since (checks notes) 2010, when it was introduced.

I pulled a copy into a tiny BDN application, seeded with ~30k actual comparisons sourced from a real-world .NET 9 build.

Instrumentation to collect that:

```diff
diff --git a/src/Shared/MSBuildNameIgnoreCaseComparer.cs b/src/Shared/MSBuildNameIgnoreCaseComparer.cs
index 9517b5f2646..23b11f824c5 100644
--- a/src/Shared/MSBuildNameIgnoreCaseComparer.cs
+++ b/src/Shared/MSBuildNameIgnoreCaseComparer.cs
@@ -3,6 +3,8 @@

using System;
using System.Collections.Generic;
+using System.IO;
+
using Microsoft.Build.Shared;

#nullable disable
@@ -19,6 +21,9 @@ namespace Microsoft.Build.Collections
[Serializable]
internal class MSBuildNameIgnoreCaseComparer : IConstrainedEqualityComparer, IEqualityComparer
{
+ private StreamWriter EqualsLogger = new($@"S:\temp\MSBuildNameIgnoreCaseComparerEquals{Guid.NewGuid()}.txt");
+ private StreamWriter HashLogger = new($@"S:\temp\MSBuildNameIgnoreCaseComparerHash{Guid.NewGuid()}.txt");
+
///
/// The processor architecture on which we are running, but default it will be x86
///
@@ -44,6 +49,8 @@ namespace Microsoft.Build.Collections
///
public bool Equals(string compareToString, string constrainedString, int start, int lengthToCompare)
{
+ EqualsLogger.Write($"{compareToString}\u001F{constrainedString}\u001F{start}\u001F{lengthToCompare}\u001E");
+
if (lengthToCompare < 0)
{
ErrorUtilities.ThrowInternalError("Invalid lengthToCompare '{0}' {1} {2}", constrainedString, start, lengthToCompare);
@@ -116,6 +123,8 @@ namespace Microsoft.Build.Collections
return 0; // per BCL convention
}

+ EqualsLogger.Write($"{obj}\u001F{start}\u001F{length}\u001E");
+
if ((s_runningProcessorArchitecture != NativeMethodsShared.ProcessorArchitectures.IA64)
&& (s_runningProcessorArchitecture != NativeMethodsShared.ProcessorArchitectures.ARM))
{
```

Benchmark program:
[MSBuildNameIgnoreCaseComparer_Bench.zip](https://github.com/user-attachments/files/17603376/MSBuildNameIgnoreCaseComparer_Bench.zip)

## Benchmark results

### 13900K desktop

```
BenchmarkDotNet v0.14.0, Windows 11 (10.0.26120.2200)
Unknown processor
.NET SDK 9.0.100-rc.2.24474.11
[Host] : .NET 9.0.0 (9.0.24.47305), X64 RyuJIT AVX2
.NET 9.0 : .NET 9.0.0 (9.0.24.47305), X64 RyuJIT AVX2
.NET Framework 4.7.2 : .NET Framework 4.8.1 (4.8.9032.0), X64 RyuJIT VectorSize=256
```

| Method | Job | Runtime | Mean | Error | StdDev | Ratio | RatioSD |
|---------------------- |--------------------- |--------------------- |---------:|----------:|----------:|------:|--------:|
| MSBuildNameIgnoreCase | .NET 9.0 | .NET 9.0 | 9.845 ns | 0.1375 ns | 0.1286 ns | 1.00 | 0.02 |
| BuiltIn | .NET 9.0 | .NET 9.0 | 9.147 ns | 0.1347 ns | 0.1194 ns | 0.93 | 0.02 |
| | | | | | | | |
| MSBuildNameIgnoreCase | .NET Framework 4.7.2 | .NET Framework 4.7.2 | 9.166 ns | 0.1201 ns | 0.1064 ns | 1.00 | 0.02 |
| BuiltIn | .NET Framework 4.7.2 | .NET Framework 4.7.2 | 9.506 ns | 0.1478 ns | 0.1382 ns | 1.04 | 0.02 |

### Dev box

```
BenchmarkDotNet v0.14.0, Windows 11 (10.0.26100.2033) (Hyper-V)
AMD EPYC 7763, 1 CPU, 16 logical and 8 physical cores
.NET SDK 9.0.100-rc.2.24474.11
[Host] : .NET 9.0.0 (9.0.24.47305), X64 RyuJIT AVX2
.NET 9.0 : .NET 9.0.0 (9.0.24.47305), X64 RyuJIT AVX2
.NET Framework 4.7.2 : .NET Framework 4.8.1 (4.8.9277.0), X64 RyuJIT VectorSize=256
```

| Method | Job | Runtime | Mean | Error | StdDev | Ratio | RatioSD |
|---------------------- |--------------------- |--------------------- |---------:|---------:|---------:|------:|--------:|
| MSBuildNameIgnoreCase | .NET 9.0 | .NET 9.0 | 17.50 ns | 0.342 ns | 0.512 ns | 1.00 | 0.04 |
| BuiltIn | .NET 9.0 | .NET 9.0 | 16.61 ns | 0.238 ns | 0.199 ns | 0.95 | 0.03 |
| | | | | | | | |
| MSBuildNameIgnoreCase | .NET Framework 4.7.2 | .NET Framework 4.7.2 | 17.70 ns | 0.114 ns | 0.101 ns | 1.00 | 0.01 |
| BuiltIn | .NET Framework 4.7.2 | .NET Framework 4.7.2 | 19.12 ns | 0.146 ns | 0.122 ns | 1.08 | 0.01 |

### macOS ARM64

```
BenchmarkDotNet v0.14.0, macOS Sequoia 15.1 (24B83) [Darwin 24.1.0]
Apple M3 Pro, 1 CPU, 12 logical and 12 physical cores
.NET SDK 9.0.100-rc.2.24474.11
[Host] : .NET 9.0.0 (9.0.24.47305), Arm64 RyuJIT AdvSIMD
.NET 9.0 : .NET 9.0.0 (9.0.24.47305), Arm64 RyuJIT AdvSIMD
```

| Method | Mean | Error | StdDev | Ratio | RatioSD |
|---------------------- |----------:|----------:|----------:|------:|--------:|
| MSBuildNameIgnoreCase | 8.007 ns | 0.0594 ns | 0.0527 ns | 1.00 | 0.01 |
| BuiltIn | 10.654 ns | 0.2028 ns | 0.1897 ns | 1.33 | 0.02 |

### Codespace

⚠️ `net8.0` because I'm lazy!

```
BenchmarkDotNet v0.14.0, Ubuntu 20.04.6 LTS (Focal Fossa) (container)
AMD EPYC 7763, 1 CPU, 2 logical cores and 1 physical core
.NET SDK 8.0.402
[Host] : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2
.NET 8.0 : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2

Job=.NET 8.0 Runtime=.NET 8.0
```

| Method | Mean | Error | StdDev | Ratio | RatioSD |
|---------------------- |---------:|---------:|---------:|------:|--------:|
| MSBuildNameIgnoreCase | 17.85 ns | 0.424 ns | 1.182 ns | 1.00 | 0.09 |
| BuiltIn | 19.07 ns | 0.481 ns | 1.373 ns | 1.07 | 0.10 |

## Conclusions

So that's not very convincing; maaybe we should consider using `OrdinalIgnoreCase` on Windows/.NET, but it's a big loss in other cases.

I also poked around the same thing with `GetHashCode` with surprising results--I was hoping for a clean win on the case where we allocate a substring (since `string.GetHashCode(ReadOnlySpan)` now exists) but we seem to do that quite rarely, averaging 5 bytes per call in the sample:

[MSBuildNameIgnoreCaseComparer_Bench_with_hashes.zip](https://github.com/user-attachments/files/17603927/MSBuildNameIgnoreCaseComparer_Bench_with_hashes.zip)

| Method | Runtime | Mean | Error | StdDev | Median | Ratio | RatioSD | Gen0 | Allocated | Alloc Ratio |
|--------------------------------- |--------------------- |-----------:|----------:|----------:|-----------:|------:|--------:|-------:|----------:|------------:|
| MSBuildNameIgnoreCase_Hash | .NET 9.0 | 5.6479 ns | 0.0579 ns | 0.0514 ns | 5.6519 ns | 0.696 | 0.01 | - | - | NA |
| OrdinalIgnoreCase_Hash_Substring | .NET 9.0 | 12.1174 ns | 0.1419 ns | 0.1328 ns | 12.1209 ns | 1.492 | 0.02 | 0.0002 | 5 B | NA |
| OrdinalIgnoreCase_Hash_Span | .NET 9.0 | 11.3895 ns | 0.0930 ns | 0.0776 ns | 11.3921 ns | 1.403 | 0.02 | - | - | NA |
| MSBuildNameIgnoreCase_Hash | .NET Framework 4.7.2 | 8.1208 ns | 0.0942 ns | 0.0881 ns | 8.1265 ns | 1.000 | 0.01 | - | - | NA |
| OrdinalIgnoreCase_Hash_Substring | .NET Framework 4.7.2 | 19.9626 ns | 0.1757 ns | 0.1643 ns | 19.9121 ns | 2.458 | 0.03 | 0.0007 | 5 B | NA |

We can get rid of those allocations but at the cost of almost doubling the compute time, so I guess we can keep allocating sometimes.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.