Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve perf of CredentialCache.GetCredential #103714

Merged
merged 2 commits into from
Jun 25, 2024
Merged

Conversation

rzikm
Copy link
Member

@rzikm rzikm commented Jun 19, 2024

Fixes #101991.
Fixes #102281.

Not sure what introduced the original regression, but we can save some time by first checking if candidate credential can have longer prefix match before we start comparing auth schemes and uri, and we can stop early if we find exact match.

Copy link
Contributor

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

@rzikm rzikm marked this pull request as ready for review June 19, 2024 17:18
@rzikm rzikm requested a review from a team June 19, 2024 17:39
@rzikm
Copy link
Member Author

rzikm commented Jun 19, 2024

/benchmark micro aspnet-perf-lin libs --variable filter=*GetCredential_Uri

@MihaZupan
Copy link
Member

@EgorBot

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;
using System.Collections.Generic;
using System.Net;

public class CredentialCacheTests
{
    private const string UriPrefix = "http://name";
    private const string HostPrefix = "name";
    private const int Port = 80;
    private const string AuthenticationType = "authType";

    private static readonly NetworkCredential s_credential = new NetworkCredential();

    private readonly Dictionary<(int uriCount, int hostPortCount), CredentialCache> _caches = new()
    {
        { (0, 0), CreateCredentialCache(0, 0) },
        { (0, 10), CreateCredentialCache(0, 10) },
        { (10, 0), CreateCredentialCache(10, 0) },
        { (10, 10), CreateCredentialCache(10, 10) }
    };

    [Benchmark]
    [Arguments("http://notfound", 0)]
    [Arguments("http://notfound", 10)]
    [Arguments("http://name5", 10)]
    public NetworkCredential GetCredential_Uri(string uriString, int uriCount)
    {
        CredentialCache cc = _caches[(uriCount: uriCount, hostPortCount: 0)];

        return cc.GetCredential(new Uri(uriString), AuthenticationType);
    }

    private static CredentialCache CreateCredentialCache(int uriCount, int hostPortCount)
    {
        var cc = new CredentialCache();

        for (int i = 0; i < uriCount; i++)
        {
            Uri uri = new Uri(UriPrefix + i.ToString());
            cc.Add(uri, AuthenticationType, s_credential);
        }

        for (int i = 0; i < hostPortCount; i++)
        {
            string host = HostPrefix + i.ToString();
            cc.Add(host, Port, AuthenticationType, s_credential);
        }

        return cc;
    }
}

@EgorBot
Copy link

EgorBot commented Jun 19, 2024

Benchmark results on Intel
BenchmarkDotNet v0.13.12, Ubuntu 22.04.4 LTS (Jammy Jellyfish)
Intel Xeon Platinum 8370C CPU 2.80GHz, 1 CPU, 16 logical and 8 physical cores
  Job-OMNTDN : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
  Job-SHAJJN : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
Method Toolchain uriString uriCount Mean Error Ratio
GetCredential_Uri Main http://name5 10 395.20 ns 0.447 ns 1.00
GetCredential_Uri PR http://name5 10 330.18 ns 0.224 ns 0.84
GetCredential_Uri Main http://notfound 0 75.30 ns 0.073 ns 1.00
GetCredential_Uri PR http://notfound 0 75.17 ns 0.091 ns 1.00
GetCredential_Uri Main http://notfound 10 240.26 ns 0.968 ns 1.00
GetCredential_Uri PR http://notfound 10 304.42 ns 0.310 ns 1.27

BDN_Artifacts.zip

@rzikm
Copy link
Member Author

rzikm commented Jun 24, 2024

I took a look at why we regressed the one scenario, and it seems that that particular benchmark is measuring a corner case. I locally editted the benchmarks so that the creds in the cache on the same host, but different prefix (http://name/prefix{i}/) and added couple more test cases and the changes actually improve common scenario. Numbers below are this PR compared to main before introducing the regression.

BenchmarkDotNet v0.13.13-nightly.20240311.145, Windows 11 (10.0.22631.3737/23H2/2023Update/SunValley3)
Intel Core i9-10900K CPU 3.70GHz, 1 CPU, 20 logical and 10 physical cores
.NET SDK 9.0.100-preview.5.24307.3
  [Host]     : .NET 9.0.0 (9.0.24.30607), X64 RyuJIT AVX2
  Job-WTZYSK : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
  Job-CDIHKU : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2

PowerPlanMode=00000000-0000-0000-0000-000000000000  IterationTime=250ms  MaxIterationCount=20
MinIterationCount=15  WarmupCount=1

| Method            | Job        | Toolchain          | uriString                        | uriCount | Mean      | Error    | StdDev   | Median    | Min       | Max       | Ratio | RatioSD | Gen0   | Allocated | Alloc Ratio |
|------------------ |----------- |------------------- |--------------------------------- |--------- |----------:|---------:|---------:|----------:|----------:|----------:|------:|--------:|-------:|----------:|------------:|
| GetCredential_Uri | Job-WTZYSK | \9.0.0\corerun.exe | http://differentHost             | 10       | 273.23 ns | 4.014 ns | 3.755 ns | 273.96 ns | 265.50 ns | 276.82 ns |  1.21 |    0.02 | 0.0248 |     264 B |        1.43 |
| GetCredential_Uri | Job-CDIHKU | \main\corerun.exe  | http://differentHost             | 10       | 225.99 ns | 3.684 ns | 3.266 ns | 225.76 ns | 221.59 ns | 232.48 ns |  1.00 |    0.02 | 0.0171 |     184 B |        1.00 |
|                   |            |                    |                                  |          |           |          |          |           |           |           |       |         |        |           |             |
| GetCredential_Uri | Job-WTZYSK | \9.0.0\corerun.exe | http://name/diff/path            | 10       | 317.86 ns | 5.780 ns | 5.124 ns | 317.80 ns | 308.61 ns | 326.92 ns |  0.90 |    0.02 | 0.0259 |     272 B |        1.00 |
| GetCredential_Uri | Job-CDIHKU | \main\corerun.exe  | http://name/diff/path            | 10       | 352.02 ns | 6.018 ns | 5.629 ns | 349.61 ns | 346.69 ns | 365.42 ns |  1.00 |    0.02 | 0.0258 |     272 B |        1.00 |
|                   |            |                    |                                  |          |           |          |          |           |           |           |       |         |        |           |             |
| GetCredential_Uri | Job-WTZYSK | \9.0.0\corerun.exe | http://name/differentPrefix/path | 10       | 456.69 ns | 8.940 ns | 8.363 ns | 454.95 ns | 445.83 ns | 470.55 ns |  0.99 |    0.02 | 0.0270 |     288 B |        1.00 |
| GetCredential_Uri | Job-CDIHKU | \main\corerun.exe  | http://name/differentPrefix/path | 10       | 459.11 ns | 7.361 ns | 6.885 ns | 457.80 ns | 448.95 ns | 474.82 ns |  1.00 |    0.02 | 0.0263 |     288 B |        1.00 |
|                   |            |                    |                                  |          |           |          |          |           |           |           |       |         |        |           |             |
| GetCredential_Uri | Job-WTZYSK | \9.0.0\corerun.exe | http://name/prefix5/path         | 10       | 381.24 ns | 7.080 ns | 6.623 ns | 380.00 ns | 372.30 ns | 392.75 ns |  0.81 |    0.02 | 0.0258 |     272 B |        1.00 |
| GetCredential_Uri | Job-CDIHKU | \main\corerun.exe  | http://name/prefix5/path         | 10       | 469.81 ns | 5.792 ns | 5.418 ns | 469.72 ns | 460.92 ns | 478.05 ns |  1.00 |    0.02 | 0.0246 |     272 B |        1.00 |
|                   |            |                    |                                  |          |           |          |          |           |           |           |       |         |        |           |             |
| GetCredential_Uri | Job-WTZYSK | \9.0.0\corerun.exe | http://notfound                  | 0        |  59.25 ns | 1.229 ns | 1.150 ns |  58.77 ns |  57.81 ns |  61.34 ns |  0.98 |    0.02 | 0.0051 |      56 B |        1.00 |
| GetCredential_Uri | Job-CDIHKU | \main\corerun.exe  | http://notfound                  | 0        |  60.65 ns | 0.773 ns | 0.723 ns |  60.47 ns |  59.68 ns |  62.08 ns |  1.00 |    0.02 | 0.0052 |      56 B |        1.00 |

Since the regressed scenario is when we are querying a completely different host, I think the regression is not a big problem.

@dotnet dotnet deleted a comment from EgorBot Jun 24, 2024
@rzikm
Copy link
Member Author

rzikm commented Jun 24, 2024

@EgorBot

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;
using System.Collections.Generic;
using System.Net;

    public class CredentialCacheTests
    {
        private const string UriPrefix = "http://name/prefix";
        private const string HostPrefix = "name";
        private const int Port = 80;
        private const string AuthenticationType = "authType";

        private static readonly NetworkCredential s_credential = new NetworkCredential();

        private readonly Dictionary<(int uriCount, int hostPortCount), CredentialCache> _caches =
            new Dictionary<(int uriCount, int hostPortCount), CredentialCache>
            {
                { (0, 0), CreateCredentialCache(0, 0) },
                { (0, 10), CreateCredentialCache(0, 10) },
                { (10, 0), CreateCredentialCache(10, 0) },
                { (10, 10), CreateCredentialCache(10, 10) }
        };

        [Benchmark]
        [Arguments("http://notfound", 0)]
        [Arguments("http://differentHost", 10)]
        [Arguments("http://name/prefix5/path", 10)]
        [Arguments("http://name/differentPrefix/path", 10)]
        [Arguments("http://name/diff/path", 10)]
        public NetworkCredential GetCredential_Uri(string uriString, int uriCount)
        {
            CredentialCache cc = _caches[(uriCount: uriCount, hostPortCount: 0)];

            return cc.GetCredential(new Uri(uriString), AuthenticationType);
        }

        private static CredentialCache CreateCredentialCache(int uriCount, int hostPortCount)
        {
            var cc = new CredentialCache();

            for (int i = 0; i < uriCount; i++)
            {
                Uri uri = new Uri(UriPrefix + i.ToString() + "/");
                cc.Add(uri, AuthenticationType, s_credential);
            }

            for (int i = 0; i < hostPortCount; i++)
            {
                string host = HostPrefix + i.ToString();
                cc.Add(host, Port, AuthenticationType, s_credential);
            }

            return cc;
        }
    }

@EgorBot
Copy link

EgorBot commented Jun 24, 2024

Benchmark results on Intel
BenchmarkDotNet v0.13.12, Ubuntu 22.04.4 LTS (Jammy Jellyfish)
Intel Xeon Platinum 8370C CPU 2.80GHz, 1 CPU, 8 logical and 4 physical cores
  Job-PJPJZA : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
  Job-YATEEO : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX-512F+CD+BW+DQ+VL+VBMI
Method Toolchain uriString uriCount Mean Error Ratio
GetCredential_Uri Main http://differentHost 10 270.15 ns 0.622 ns 1.00
GetCredential_Uri PR http://differentHost 10 342.75 ns 1.019 ns 1.27
GetCredential_Uri Main http:(...)/path [21] 10 500.69 ns 0.553 ns 1.00
GetCredential_Uri PR http:(...)/path [21] 10 375.33 ns 0.780 ns 0.75
GetCredential_Uri Main http:(...)/path [32] 10 667.06 ns 0.513 ns 1.00
GetCredential_Uri PR http:(...)/path [32] 10 589.57 ns 2.944 ns 0.88
GetCredential_Uri Main http:(...)/path [24] 10 659.35 ns 0.849 ns 1.00
GetCredential_Uri PR http:(...)/path [24] 10 456.99 ns 0.295 ns 0.69
GetCredential_Uri Main http://notfound 0 77.91 ns 0.176 ns 1.00
GetCredential_Uri PR http://notfound 0 76.56 ns 0.173 ns 0.98

BDN_Artifacts.zip

@rzikm
Copy link
Member Author

rzikm commented Jun 24, 2024

@dotnet/ncl Can I get a review?

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This can still be noise compared to actual TLS.

@rzikm rzikm merged commit afb75a2 into dotnet:main Jun 25, 2024
78 of 83 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 26, 2024
@karelz karelz added this to the 9.0.0 milestone Sep 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants