ClaimsPrincipal.SelectPrimaryIdentity allocates unnecessary enumerator

[idg10]

Description

The ClaimsPrincipal.SelectPrimaryIdentity method (which is used by the public Identity property) always allocates an enumerator, when this should never be necessary.

The method is declared as taking an IEnumerable<ClaimsIdentity>. In normal usage, this argument is always the List<ClaimsIdentity> from the _identities field. (There's an obscure situation in which it might not be: user code might retrieve the static PrimaryIdentitySelector property and invoke it passing in their own IEnumerable<ClaimsIdentity>.)

This means that the foreach inside SelectPrimaryIdentity can't take advantage of the zero-allocation enumeration normally possible with a List<T>, because the CLR has to generate code that can cope with any IEnumerable<T>. So the overwhelming majority of use cases pay an allocation penalty that is unnecessary.

This could be modified to detect an IList as a special case to avoid the allocation.

Configuration

.NET 8.0.8, SDK 8.0.400. All architectures, any OS.

Regression?

No.

Analysis

We saw this allocation in a performance analysis. We were able to prevent it by writing this helper:

public static IIdentity? GetPrimaryIdentity(this ClaimsPrincipal claimsPrincipal)
{
    if (claimsPrincipal.Identities is IList<ClaimsIdentity> identities)
    {
        for (int i = 0; i < identities.Count; i++)
        {
            if (identities[i] is not null)
            {
                return identities[i];
            }
        }
    }

    return claimsPrincipal.Identity;
}

and using it instead of user.Identity. Running our performance analysis again with this in place showed that we no longer saw an enumerator being allocated.

[dotnet-policy-service]

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

[PranavSenthilnathan]

I benchmarked the code with these changes and they show about 75% improvement for ClaimsPrinciple.Identity calls and ~3% worse perf for ClaimsPrincipal.PrimaryIdentitySelector on non-Lists. I think this change would be worth taking assuming that ClaimsPrinciple.Identity is common enough and ClaimsPrincipal.PrimaryIdentitySelector isn't used much, which seems to be the case for public repos (https://grep.app/search?q=PrimaryIdentitySelector).

/cc @bartonjs or @JamesNK to confirm

Full perf numbers and code changes:

Perf Results

BenchmarkDotNet v0.14.0, Windows 11 (10.0.22631.4460/23H2/2023Update/SunValley3)
AMD Ryzen 9 9950X, 1 CPU, 32 logical and 16 physical cores
.NET SDK 9.0.100-rc.2.24474.11
  [Host]     : .NET 9.0.0 (9.0.24.47305), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
  Job-NAUUTY : .NET 10.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
  Job-GUADOS : .NET 10.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI

Method	Toolchain	NumberOfIdentities	Mean	Error	Ratio	Allocated	Alloc Ratio
GetIdentity_WithNulls	main	1	4.722 ns	0.0303 ns	1.00	40 B	1.00
GetIdentity_WithNulls	pr	1	1.297 ns	0.0100 ns	0.27	-	0.00
GetIdentity_WithoutNulls	main	1	4.736 ns	0.0321 ns	1.00	40 B	1.00
GetIdentity_WithoutNulls	pr	1	1.299 ns	0.0063 ns	0.27	-	0.00
IdentitySelector_WithNulls	main	1	3.244 ns	0.0648 ns	1.00	32 B	1.00
IdentitySelector_WithNulls	pr	1	3.357 ns	0.0406 ns	1.03	32 B	1.00
IdentitySelector_WithoutNulls	main	1	16.279 ns	0.0867 ns	1.00	168 B	1.00
IdentitySelector_WithoutNulls	pr	1	16.672 ns	0.0695 ns	1.02	168 B	1.00
GetIdentity_WithNulls	main	2	5.999 ns	0.0185 ns	1.00	40 B	1.00
GetIdentity_WithNulls	pr	2	1.513 ns	0.0262 ns	0.25	-	0.00
GetIdentity_WithoutNulls	main	2	4.965 ns	0.0216 ns	1.00	40 B	1.00
GetIdentity_WithoutNulls	pr	2	1.325 ns	0.0288 ns	0.27	-	0.00
IdentitySelector_WithNulls	main	2	25.184 ns	0.0905 ns	1.00	136 B	1.00
IdentitySelector_WithNulls	pr	2	25.962 ns	0.1776 ns	1.03	136 B	1.00
IdentitySelector_WithoutNulls	main	2	16.251 ns	0.0932 ns	1.00	168 B	1.00
IdentitySelector_WithoutNulls	pr	2	16.550 ns	0.0793 ns	1.02	168 B	1.00
GetIdentity_WithNulls	main	3	7.039 ns	0.0417 ns	1.00	40 B	1.00
GetIdentity_WithNulls	pr	3	1.702 ns	0.0114 ns	0.24	-	0.00
GetIdentity_WithoutNulls	main	3	4.728 ns	0.0482 ns	1.00	40 B	1.00
GetIdentity_WithoutNulls	pr	3	1.318 ns	0.0255 ns	0.28	-	0.00
IdentitySelector_WithNulls	main	3	27.389 ns	0.1267 ns	1.00	136 B	1.00
IdentitySelector_WithNulls	pr	3	27.777 ns	0.1078 ns	1.01	136 B	1.00
IdentitySelector_WithoutNulls	main	3	16.105 ns	0.0777 ns	1.00	168 B	1.00
IdentitySelector_WithoutNulls	pr	3	16.572 ns	0.0443 ns	1.03	168 B	1.00
GetIdentity_WithNulls	main	5	9.311 ns	0.0342 ns	1.00	40 B	1.00
GetIdentity_WithNulls	pr	5	2.300 ns	0.0485 ns	0.25	-	0.00
GetIdentity_WithoutNulls	main	5	4.709 ns	0.0153 ns	1.00	40 B	1.00
GetIdentity_WithoutNulls	pr	5	1.294 ns	0.0093 ns	0.27	-	0.00
IdentitySelector_WithNulls	main	5	35.628 ns	0.1622 ns	1.00	136 B	1.00
IdentitySelector_WithNulls	pr	5	35.521 ns	0.2105 ns	1.00	136 B	1.00
IdentitySelector_WithoutNulls	main	5	16.791 ns	0.1132 ns	1.00	168 B	1.00
IdentitySelector_WithoutNulls	pr	5	16.509 ns	0.0730 ns	0.98	168 B	1.00
GetIdentity_WithNulls	main	10	14.685 ns	0.0685 ns	1.00	40 B	1.00
GetIdentity_WithNulls	pr	10	3.731 ns	0.0109 ns	0.25	-	0.00
GetIdentity_WithoutNulls	main	10	4.685 ns	0.0129 ns	1.00	40 B	1.00
GetIdentity_WithoutNulls	pr	10	1.299 ns	0.0070 ns	0.28	-	0.00
IdentitySelector_WithNulls	main	10	55.697 ns	0.2269 ns	1.00	136 B	1.00
IdentitySelector_WithNulls	pr	10	55.141 ns	0.2982 ns	0.99	136 B	1.00
IdentitySelector_WithoutNulls	main	10	16.223 ns	0.0723 ns	1.00	168 B	1.00
IdentitySelector_WithoutNulls	pr	10	16.745 ns	0.1201 ns	1.03	168 B	1.00

Test code

using System.Security.Claims;
using System.Security.Principal;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

new BenchmarkSwitcher(typeof(Program).Assembly).Run(args);

[MemoryDiagnoser(false)]
[HideColumns("Job", "StdDev", "Median", "RatioSD")]
public class Benchmarks
{
    private IEnumerable<ClaimsIdentity>? _identitiesWithNulls;
    private IEnumerable<ClaimsIdentity>? _identitiesWithoutNulls;
    
    private ClaimsPrincipal? _principalWithNulls;
    private ClaimsPrincipal? _principalWithoutNulls;

    [Params(1, 2, 3, 5, 10)]
    public int NumberOfIdentities;

    [GlobalSetup]
    public void Setup()
    {
        _identitiesWithNulls = Enumerable.Range(0, NumberOfIdentities - 1).Select(_ => default(ClaimsIdentity)).Concat([new ClaimsIdentity()]);
        _identitiesWithoutNulls = Enumerable.Range(0, NumberOfIdentities).Select(_ => new ClaimsIdentity());
        
        _principalWithNulls = new ClaimsPrincipal(_identitiesWithNulls);
        _principalWithoutNulls = new ClaimsPrincipal(_identitiesWithoutNulls);
    }

    [Benchmark]
    public IIdentity? GetIdentity_WithNulls() => _principalWithNulls!.Identity;

    [Benchmark]
    public IIdentity? GetIdentity_WithoutNulls() => _principalWithoutNulls!.Identity;

    [Benchmark]
    public IIdentity? IdentitySelector_WithNulls() => ClaimsPrincipal.PrimaryIdentitySelector(_identitiesWithNulls!);

    [Benchmark]
    public IIdentity? IdentitySelector_WithoutNulls() => ClaimsPrincipal.PrimaryIdentitySelector(_identitiesWithoutNulls!);
}

Code change

private static ClaimsIdentity? SelectPrimaryIdentity(IEnumerable<ClaimsIdentity> identities)
{
    ArgumentNullException.ThrowIfNull(identities);

+   if (identities is List<ClaimsIdentity> list)
+   {
+       foreach (ClaimsIdentity identity in list)
+       {
+           if (identity != null)
+           {
+               return identity;
+           }
+       }
+
+       return null;
+   }
+ 
   foreach (ClaimsIdentity identity in identities)
    {
        if (identity != null)
        {
            return identity;
        }
    }

    return null;
}