RuntimePropertyInfo performance regression when used in a hash table

[jairbubbles]

Description

When migrating some code from net472 to net8.0 I noticed a huge performance regression with some code dealing with caching MemberInfo attributes in a ConcurrentDictionary.

I investigated the issue and narrowed it down to a simple sample:

HashSet<PropertyInfo> propertyInfos = new();
foreach (PropertyInfo property in GetManyProperties())
{
     propertyInfos.Add(property);
}

In my benchmark I'm testing with the Graph API assembly. It has a lot of types (120k properties in total!), which is perfect to make the issue more visible, in this condition I get:

| Method                      | Job                  | Runtime              | Mean       | Error      | StdDev     | Ratio | RatioSD |
|---------------------------- |--------------------- |--------------------- |-----------:|-----------:|-----------:|------:|--------:|
| AddToHashSet_WithBaseHash   | .NET 8.0             | .NET 8.0             | 637.151 ms | 12.3232 ms | 15.5849 ms |  1.00 |    0.03 |
| AddToHashSet_WithBaseHash   | .NET Framework 4.7.2 | .NET Framework 4.7.2 |   7.236 ms |  0.1813 ms |  0.5287 ms |  1.01 |    0.10 |

When profiling I noticed that we were spending a lot of time in Equals as many elements were put in the same bucket. Those elements were all the properties which are shared by inheritance. They shared the same hash code but Equals would always return false.

I believe the issue was introduced by #78249 where ReflectedType was added as a criteria for equality but not the hash code computation.

Workaround

As a workaround I'm using a custom EqualityComparer which also uses the actual type (not only the declaring type)

public int GetHashCode(PropertyInfo obj)
{
  // Take in account the ReflectedType as it's also used in RuntimePropertyInfo.Equals
  return HashCode.Combine(obj.MetadataToken, obj.DeclaringType!.GetHashCode(), obj.ReflectedType!.GetHashCode());
}

In those conditions .NET 8 becomes faster:

Method	Job	Runtime	Mean	Error	StdDev	Ratio	RatioSD
AddToHashSet_WithCustomHash	.NET 8.0	.NET 8.0	9.529 ms	0.1877 ms	0.3876 ms	0.01	0.00
AddToHashSet_WithCustomHash	.NET Framework 4.7.2	.NET Framework 4.7.2	12.815 ms	0.2563 ms	0.4281 ms	1.78	0.14

Sample

Here is the full benchmark project: Benchmark.zip

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

[steveharter]

Verified regression.

Note the repro has ~120K properties in the same hashset since it combines all properties from all types. This is not a common scenario.

If we fix this, we should also add benchmarks.

[jairbubbles]

Yes indeed, the scenario was a bit exagerated but I found the issue in a real world application. Clearly we can challenge the code but I feel like we should at least be on par with .NET Framework performances.

If we fix this, we should also add benchmarks.

I can take a look but I will need some guidance for those.

[steveharter]

Benchmark docs are at https://github.com/dotnet/performance/blob/main/docs/benchmarking-workflow-dotnet-runtime.md.

It's nice to get the benchmarks in before the code changes so our CI system has a chance to record the initial status.

Also we should apply the GetHashCode change to all info types: PropertyInfo, FieldInfo, MethodInfo, EventInfo.