From 7b4b039930b2b6f2afde60594e596c55206137d1 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Sun, 3 May 2026 10:53:02 -0500 Subject: [PATCH 1/3] Fix incremental generator equality for proper build caching GenerationInfo and NetworkRequest were plain classes without IEquatable, causing the incremental pipeline to always regenerate on every build since ReferenceEquals was used for cache comparison (always false for new objects). Changes: - Implement IEquatable on GenerationInfo and NetworkRequest - Replace HashSet with sorted ImmutableArray for stable value equality on FallbackTypes/FallbackTypesWithComparer - Replace NetworkRequest[] with ImmutableArray - Add RunGeneratorTwice helper to SourceGeneratorDriver for incremental tests - Add 4 tests verifying cache hits on same/unrelated changes and cache misses when attributes actually change Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SortingNetworkGenerator.cs | 72 +++++++++-- SortingNetworks.Tests/GeneratorTests.cs | 113 ++++++++++++++++++ .../SourceGeneratorDriver.cs | 23 ++++ 3 files changed, 199 insertions(+), 9 deletions(-) diff --git a/SortingNetworks.Generators/SortingNetworkGenerator.cs b/SortingNetworks.Generators/SortingNetworkGenerator.cs index 1c80c9c..83becac 100644 --- a/SortingNetworks.Generators/SortingNetworkGenerator.cs +++ b/SortingNetworks.Generators/SortingNetworkGenerator.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; @@ -158,9 +159,9 @@ comparerParam is INamedTypeSymbol comparerType && return new GenerationInfo( classSymbol.Name, namespaceName, - attributes.ToArray(), - fallbackTypes, - fallbackTypesWithComparer); + attributes.ToImmutableArray(), + fallbackTypes.OrderBy(s => s, StringComparer.Ordinal).ToImmutableArray(), + fallbackTypesWithComparer.OrderBy(s => s, StringComparer.Ordinal).ToImmutableArray()); } private static void Execute(SourceProductionContext context, ImmutableArray infos) @@ -966,15 +967,15 @@ private static void EmitComparerOverloads(StringBuilder sb, string typeName, Lis sb.AppendLine(); } - private sealed class GenerationInfo + private sealed class GenerationInfo : IEquatable { public string ClassName { get; } public string? Namespace { get; } - public NetworkRequest[] Requests { get; } - public HashSet FallbackTypes { get; } - public HashSet FallbackTypesWithComparer { get; } + public ImmutableArray Requests { get; } + public ImmutableArray FallbackTypes { get; } + public ImmutableArray FallbackTypesWithComparer { get; } - public GenerationInfo(string className, string? ns, NetworkRequest[] requests, HashSet fallbackTypes, HashSet fallbackTypesWithComparer) + public GenerationInfo(string className, string? ns, ImmutableArray requests, ImmutableArray fallbackTypes, ImmutableArray fallbackTypesWithComparer) { ClassName = className; Namespace = ns; @@ -982,6 +983,35 @@ public GenerationInfo(string className, string? ns, NetworkRequest[] requests, H FallbackTypes = fallbackTypes; FallbackTypesWithComparer = fallbackTypesWithComparer; } + + public bool Equals(GenerationInfo? other) + { + if (other is null) return false; + if (ReferenceEquals(this, other)) return true; + return ClassName == other.ClassName + && Namespace == other.Namespace + && Requests.SequenceEqual(other.Requests) + && FallbackTypes.SequenceEqual(other.FallbackTypes) + && FallbackTypesWithComparer.SequenceEqual(other.FallbackTypesWithComparer); + } + + public override bool Equals(object? obj) => Equals(obj as GenerationInfo); + + public override int GetHashCode() + { + unchecked + { + int hash = ClassName.GetHashCode(); + hash = hash * 397 ^ (Namespace?.GetHashCode() ?? 0); + foreach (var r in Requests) + hash = hash * 397 ^ r.GetHashCode(); + foreach (var f in FallbackTypes) + hash = hash * 397 ^ f.GetHashCode(); + foreach (var f in FallbackTypesWithComparer) + hash = hash * 397 ^ f.GetHashCode(); + return hash; + } + } } /// @@ -996,7 +1026,7 @@ private static (SpecialType Type32, SpecialType Type64)? GetNativeIntDelegateTyp return null; } - private sealed class NetworkRequest + private sealed class NetworkRequest : IEquatable { public int Size { get; } public string TypeName { get; } @@ -1012,6 +1042,30 @@ public NetworkRequest(int size, string typeName, SpecialType specialType, bool i IsCustomType = !SupportedSpecialTypes.Contains(specialType); IsComparable = isComparable; } + + public bool Equals(NetworkRequest? other) + { + if (other is null) return false; + if (ReferenceEquals(this, other)) return true; + return Size == other.Size + && TypeName == other.TypeName + && SpecialType == other.SpecialType + && IsComparable == other.IsComparable; + } + + public override bool Equals(object? obj) => Equals(obj as NetworkRequest); + + public override int GetHashCode() + { + unchecked + { + int hash = Size; + hash = hash * 397 ^ TypeName.GetHashCode(); + hash = hash * 397 ^ (int)SpecialType; + hash = hash * 397 ^ IsComparable.GetHashCode(); + return hash; + } + } } } } diff --git a/SortingNetworks.Tests/GeneratorTests.cs b/SortingNetworks.Tests/GeneratorTests.cs index 87877bf..46250b6 100644 --- a/SortingNetworks.Tests/GeneratorTests.cs +++ b/SortingNetworks.Tests/GeneratorTests.cs @@ -911,4 +911,117 @@ public partial class MySorter {{ }} Assert.Contains($"private static void Sort16(ref {type64} first)", generatedSource); Assert.Contains($"private static void Sort16(ref {type32} first)", generatedSource); } + + [Fact] + public void IncrementalCache_SameCompilation_OutputIsCached() + { + var source = @" +using SortingNetworks; + +[SortingNetwork(4, typeof(int))] +public partial class MySorter { } +"; + var compilation = SourceGeneratorDriver.CreateCompilation(source); + var result = SourceGeneratorDriver.RunGeneratorTwice(compilation, compilation); + + var generatorResult = result.Results.Single(); + + // The ForAttributeWithMetadataName step should report Cached on the second run + var outputStep = generatorResult.TrackedOutputSteps + .SelectMany(s => s.Value) + .SelectMany(s => s.Outputs); + Assert.All(outputStep, o => + Assert.Equal(IncrementalStepRunReason.Cached, o.Reason)); + } + + [Fact] + public void IncrementalCache_UnrelatedChange_OutputIsCached() + { + var source = @" +using SortingNetworks; + +[SortingNetwork(4, typeof(int))] +public partial class MySorter { } + +public class Unrelated { } +"; + var modified = @" +using SortingNetworks; + +[SortingNetwork(4, typeof(int))] +public partial class MySorter { } + +public class Unrelated { public void Foo() { } } +"; + var first = SourceGeneratorDriver.CreateCompilation(source); + var second = SourceGeneratorDriver.CreateCompilation(modified); + var result = SourceGeneratorDriver.RunGeneratorTwice(first, second); + + var generatorResult = result.Results.Single(); + + // Output should still be cached — the SortingNetwork attribute didn't change + var outputStep = generatorResult.TrackedOutputSteps + .SelectMany(s => s.Value) + .SelectMany(s => s.Outputs); + Assert.All(outputStep, o => + Assert.Equal(IncrementalStepRunReason.Cached, o.Reason)); + } + + [Fact] + public void IncrementalCache_AttributeChanged_OutputIsModified() + { + var source = @" +using SortingNetworks; + +[SortingNetwork(4, typeof(int))] +public partial class MySorter { } +"; + var modified = @" +using SortingNetworks; + +[SortingNetwork(8, typeof(int))] +public partial class MySorter { } +"; + var first = SourceGeneratorDriver.CreateCompilation(source); + var second = SourceGeneratorDriver.CreateCompilation(modified); + var result = SourceGeneratorDriver.RunGeneratorTwice(first, second); + + var generatorResult = result.Results.Single(); + + // Output should be Modified — the attribute size changed + var outputStep = generatorResult.TrackedOutputSteps + .SelectMany(s => s.Value) + .SelectMany(s => s.Outputs); + Assert.Contains(outputStep, o => + o.Reason == IncrementalStepRunReason.Modified); + } + + [Fact] + public void IncrementalCache_TypeChanged_OutputIsModified() + { + var source = @" +using SortingNetworks; + +[SortingNetwork(4, typeof(int))] +public partial class MySorter { } +"; + var modified = @" +using SortingNetworks; + +[SortingNetwork(4, typeof(long))] +public partial class MySorter { } +"; + var first = SourceGeneratorDriver.CreateCompilation(source); + var second = SourceGeneratorDriver.CreateCompilation(modified); + var result = SourceGeneratorDriver.RunGeneratorTwice(first, second); + + var generatorResult = result.Results.Single(); + + // Output should be Modified — the element type changed + var outputStep = generatorResult.TrackedOutputSteps + .SelectMany(s => s.Value) + .SelectMany(s => s.Outputs); + Assert.Contains(outputStep, o => + o.Reason == IncrementalStepRunReason.Modified); + } } diff --git a/SortingNetworks.Tests/SourceGeneratorDriver.cs b/SortingNetworks.Tests/SourceGeneratorDriver.cs index 2e2e873..1428651 100644 --- a/SortingNetworks.Tests/SourceGeneratorDriver.cs +++ b/SortingNetworks.Tests/SourceGeneratorDriver.cs @@ -125,6 +125,29 @@ public static (GeneratorDriverRunResult result, Compilation updatedCompilation) return (driver.GetRunResult(), updatedCompilation); } + /// + /// Runs the generator twice: first on the initial compilation, then on a second compilation. + /// Returns the run result from the second run so callers can inspect incremental step reasons. + /// + public static GeneratorDriverRunResult RunGeneratorTwice(CSharpCompilation first, CSharpCompilation second) + { + var generator = new SortingNetworkGenerator(); + + GeneratorDriver driver = CSharpGeneratorDriver.Create( + generators: new[] { generator.AsSourceGenerator() }, + driverOptions: new GeneratorDriverOptions( + disabledOutputs: IncrementalGeneratorOutputKind.None, + trackIncrementalGeneratorSteps: true)); + + // First run — primes the cache + driver = driver.RunGeneratorsAndUpdateCompilation(first, out _, out _); + + // Second run — should use cache if inputs are equal + driver = driver.RunGeneratorsAndUpdateCompilation(second, out _, out _); + + return driver.GetRunResult(); + } + /// /// Gets all compilation errors from a compilation (severity = Error). /// From a13f5beac474885ea25d64c6317033679ffbdebd Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Sun, 3 May 2026 10:57:07 -0500 Subject: [PATCH 2/3] Use xxHash32-based HashCode polyfill instead of manual XOR hashing Replace ad-hoc hash * 397 ^ value patterns with a proper HashCode polyfill based on the xxHash32 implementation from dotnet/runtime. This provides better hash distribution via HashCode.Combine and the Add/ToHashCode pattern. The polyfill is internal to the generator assembly, avoiding the need to redistribute a NuGet dependency (Microsoft.Bcl.HashCode) in the analyzer package. Uses a fixed seed since randomization is unnecessary for incremental generator caching. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- SortingNetworks.Generators/HashCode.cs | 168 ++++++++++++++++++ .../SortingNetworkGenerator.cs | 31 ++-- 2 files changed, 179 insertions(+), 20 deletions(-) create mode 100644 SortingNetworks.Generators/HashCode.cs diff --git a/SortingNetworks.Generators/HashCode.cs b/SortingNetworks.Generators/HashCode.cs new file mode 100644 index 0000000..6c62dee --- /dev/null +++ b/SortingNetworks.Generators/HashCode.cs @@ -0,0 +1,168 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Minimal polyfill of System.HashCode for netstandard2.0. +// Based on the xxHash32 implementation from dotnet/runtime: +// https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/HashCode.cs +// +// Original xxHash code by Yann Collet, BSD 2-Clause License. + +using System.Runtime.CompilerServices; + +namespace SortingNetworks.Generators +{ +#pragma warning disable CS0809 + internal struct HashCode + { + private const uint Prime1 = 2654435761U; + private const uint Prime2 = 2246822519U; + private const uint Prime3 = 3266489917U; + private const uint Prime4 = 668265263U; + private const uint Prime5 = 374761393U; + + // Fixed seed — randomization is unnecessary for incremental generator caching. + private const uint Seed = 0; + + private uint _v1, _v2, _v3, _v4; + private uint _queue1, _queue2, _queue3; + private uint _length; + + public static int Combine(T1 value1, T2 value2, T3 value3, T4 value4) + { + unchecked + { + uint hc1 = (uint)(value1?.GetHashCode() ?? 0); + uint hc2 = (uint)(value2?.GetHashCode() ?? 0); + uint hc3 = (uint)(value3?.GetHashCode() ?? 0); + uint hc4 = (uint)(value4?.GetHashCode() ?? 0); + + Initialize(out uint v1, out uint v2, out uint v3, out uint v4); + + v1 = Round(v1, hc1); + v2 = Round(v2, hc2); + v3 = Round(v3, hc3); + v4 = Round(v4, hc4); + + uint hash = MixState(v1, v2, v3, v4); + hash += 16; + + hash = MixFinal(hash); + return (int)hash; + } + } + + public void Add(T value) + { + Add(value?.GetHashCode() ?? 0); + } + + public int ToHashCode() + { + unchecked + { + uint length = _length; + uint position = length % 4; + + uint hash = length < 4 ? MixEmptyState() : MixState(_v1, _v2, _v3, _v4); + hash += length * 4; + + if (position > 0) + { + hash = QueueRound(hash, _queue1); + if (position > 1) + { + hash = QueueRound(hash, _queue2); + if (position > 2) + hash = QueueRound(hash, _queue3); + } + } + + hash = MixFinal(hash); + return (int)hash; + } + } + + private void Add(int value) + { + unchecked + { + uint val = (uint)value; + uint previousLength = _length++; + uint position = previousLength % 4; + + if (position == 0) + _queue1 = val; + else if (position == 1) + _queue2 = val; + else if (position == 2) + _queue3 = val; + else + { + if (previousLength == 3) + Initialize(out _v1, out _v2, out _v3, out _v4); + + _v1 = Round(_v1, _queue1); + _v2 = Round(_v2, _queue2); + _v3 = Round(_v3, _queue3); + _v4 = Round(_v4, val); + } + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void Initialize(out uint v1, out uint v2, out uint v3, out uint v4) + { + unchecked + { + v1 = Seed + Prime1 + Prime2; + v2 = Seed + Prime2; + v3 = Seed; + v4 = Seed - Prime1; + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static uint Round(uint hash, uint input) + { + unchecked { return RotateLeft(hash + input * Prime2, 13) * Prime1; } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static uint QueueRound(uint hash, uint queuedValue) + { + unchecked { return RotateLeft(hash + queuedValue * Prime3, 17) * Prime4; } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static uint MixState(uint v1, uint v2, uint v3, uint v4) + { + unchecked { return RotateLeft(v1, 1) + RotateLeft(v2, 7) + RotateLeft(v3, 12) + RotateLeft(v4, 18); } + } + + private static uint MixEmptyState() + { + unchecked { return Seed + Prime5; } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static uint MixFinal(uint hash) + { + unchecked + { + hash ^= hash >> 15; + hash *= Prime2; + hash ^= hash >> 13; + hash *= Prime3; + hash ^= hash >> 16; + return hash; + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static uint RotateLeft(uint value, int offset) + { + return (value << offset) | (value >> (32 - offset)); + } + } +#pragma warning restore CS0809 +} diff --git a/SortingNetworks.Generators/SortingNetworkGenerator.cs b/SortingNetworks.Generators/SortingNetworkGenerator.cs index 83becac..db3e7d7 100644 --- a/SortingNetworks.Generators/SortingNetworkGenerator.cs +++ b/SortingNetworks.Generators/SortingNetworkGenerator.cs @@ -999,18 +999,16 @@ public bool Equals(GenerationInfo? other) public override int GetHashCode() { - unchecked - { - int hash = ClassName.GetHashCode(); - hash = hash * 397 ^ (Namespace?.GetHashCode() ?? 0); - foreach (var r in Requests) - hash = hash * 397 ^ r.GetHashCode(); - foreach (var f in FallbackTypes) - hash = hash * 397 ^ f.GetHashCode(); - foreach (var f in FallbackTypesWithComparer) - hash = hash * 397 ^ f.GetHashCode(); - return hash; - } + var hc = new HashCode(); + hc.Add(ClassName); + hc.Add(Namespace); + foreach (var r in Requests) + hc.Add(r); + foreach (var f in FallbackTypes) + hc.Add(f); + foreach (var f in FallbackTypesWithComparer) + hc.Add(f); + return hc.ToHashCode(); } } @@ -1057,14 +1055,7 @@ public bool Equals(NetworkRequest? other) public override int GetHashCode() { - unchecked - { - int hash = Size; - hash = hash * 397 ^ TypeName.GetHashCode(); - hash = hash * 397 ^ (int)SpecialType; - hash = hash * 397 ^ IsComparable.GetHashCode(); - return hash; - } + return HashCode.Combine(Size, TypeName, SpecialType, IsComparable); } } } From f0fd1bf09a55a4d3ea0d7f21281f71c51a2ab05b Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Sun, 3 May 2026 11:17:35 -0500 Subject: [PATCH 3/3] Address review: target specific tracked step, assert non-empty, remove unnecessary pragma - Tests now query TrackedOutputSteps["SourceOutput"] specifically instead of flattening all steps, preventing false positives from vacuous Assert.All and brittleness if new tracked steps are added later. - Assert.NotEmpty added before Assert.All to guard against vacuous passes. - Removed unnecessary #pragma warning disable CS0809 from HashCode polyfill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- SortingNetworks.Generators/HashCode.cs | 2 - SortingNetworks.Tests/GeneratorTests.cs | 49 +++++++++++++++---------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/SortingNetworks.Generators/HashCode.cs b/SortingNetworks.Generators/HashCode.cs index 6c62dee..ea91614 100644 --- a/SortingNetworks.Generators/HashCode.cs +++ b/SortingNetworks.Generators/HashCode.cs @@ -11,7 +11,6 @@ namespace SortingNetworks.Generators { -#pragma warning disable CS0809 internal struct HashCode { private const uint Prime1 = 2654435761U; @@ -164,5 +163,4 @@ private static uint RotateLeft(uint value, int offset) return (value << offset) | (value >> (32 - offset)); } } -#pragma warning restore CS0809 } diff --git a/SortingNetworks.Tests/GeneratorTests.cs b/SortingNetworks.Tests/GeneratorTests.cs index 46250b6..c3612a4 100644 --- a/SortingNetworks.Tests/GeneratorTests.cs +++ b/SortingNetworks.Tests/GeneratorTests.cs @@ -926,11 +926,14 @@ public partial class MySorter { } var generatorResult = result.Results.Single(); - // The ForAttributeWithMetadataName step should report Cached on the second run - var outputStep = generatorResult.TrackedOutputSteps - .SelectMany(s => s.Value) - .SelectMany(s => s.Outputs); - Assert.All(outputStep, o => + // Target the specific SourceOutput step to avoid brittleness if more steps are added + Assert.True(generatorResult.TrackedOutputSteps.ContainsKey("SourceOutput"), + "Expected 'SourceOutput' tracked step"); + var outputs = generatorResult.TrackedOutputSteps["SourceOutput"] + .SelectMany(s => s.Outputs) + .ToArray(); + Assert.NotEmpty(outputs); + Assert.All(outputs, o => Assert.Equal(IncrementalStepRunReason.Cached, o.Reason)); } @@ -959,11 +962,13 @@ public class Unrelated { public void Foo() { } } var generatorResult = result.Results.Single(); - // Output should still be cached — the SortingNetwork attribute didn't change - var outputStep = generatorResult.TrackedOutputSteps - .SelectMany(s => s.Value) - .SelectMany(s => s.Outputs); - Assert.All(outputStep, o => + Assert.True(generatorResult.TrackedOutputSteps.ContainsKey("SourceOutput"), + "Expected 'SourceOutput' tracked step"); + var outputs = generatorResult.TrackedOutputSteps["SourceOutput"] + .SelectMany(s => s.Outputs) + .ToArray(); + Assert.NotEmpty(outputs); + Assert.All(outputs, o => Assert.Equal(IncrementalStepRunReason.Cached, o.Reason)); } @@ -988,11 +993,13 @@ public partial class MySorter { } var generatorResult = result.Results.Single(); - // Output should be Modified — the attribute size changed - var outputStep = generatorResult.TrackedOutputSteps - .SelectMany(s => s.Value) - .SelectMany(s => s.Outputs); - Assert.Contains(outputStep, o => + Assert.True(generatorResult.TrackedOutputSteps.ContainsKey("SourceOutput"), + "Expected 'SourceOutput' tracked step"); + var outputs = generatorResult.TrackedOutputSteps["SourceOutput"] + .SelectMany(s => s.Outputs) + .ToArray(); + Assert.NotEmpty(outputs); + Assert.Contains(outputs, o => o.Reason == IncrementalStepRunReason.Modified); } @@ -1017,11 +1024,13 @@ public partial class MySorter { } var generatorResult = result.Results.Single(); - // Output should be Modified — the element type changed - var outputStep = generatorResult.TrackedOutputSteps - .SelectMany(s => s.Value) - .SelectMany(s => s.Outputs); - Assert.Contains(outputStep, o => + Assert.True(generatorResult.TrackedOutputSteps.ContainsKey("SourceOutput"), + "Expected 'SourceOutput' tracked step"); + var outputs = generatorResult.TrackedOutputSteps["SourceOutput"] + .SelectMany(s => s.Outputs) + .ToArray(); + Assert.NotEmpty(outputs); + Assert.Contains(outputs, o => o.Reason == IncrementalStepRunReason.Modified); } }