Better avoidance of closure, fragmentation of cache reduction

Author: soenneker

src/Abstract/IHttpClientCache.cs

@@ -78,6 +78,22 @@ public interface IHttpClientCache : IAsyncDisposable, IDisposable
     [Pure]
     HttpClient GetSync(string id, Func<HttpClientOptions?> optionsFactory, CancellationToken cancellationToken = default);
 
+    /// <summary>
+    /// Retrieves a configured and ready-to-use synchronous instance of <see cref="HttpClient"/> associated with the
+    /// specified identifier.
+    /// </summary>
+    /// <remarks>This method blocks until the client is fully configured and available. If the options factory
+    /// performs asynchronous operations, they will be completed before the client is returned. The returned <see
+    /// cref="HttpClient"/> is intended for reuse and should not be disposed by the caller.</remarks>
+    /// <param name="id">The unique identifier for the <see cref="HttpClient"/> instance to retrieve. Cannot be null or empty.</param>
+    /// <param name="optionsFactory">A factory delegate that asynchronously provides <see cref="HttpClientOptions"/> for configuring the client. The
+    /// returned options may be null to use default settings.</param>
+    /// <param name="cancellationToken">A cancellation token that can be used to cancel the operation before completion. Optional.</param>
+    /// <returns>An <see cref="HttpClient"/> instance configured according to the provided options. The same instance may be
+    /// returned for repeated calls with the same identifier.</returns>
+    [Pure]
+    HttpClient GetSync(string id, Func<ValueTask<HttpClientOptions?>> optionsFactory, CancellationToken cancellationToken = default);
+
     /// <summary>
     /// Should be used if the component using <see cref="IHttpClientCache"/> is disposed (unless the entire app is being disposed). Includes disposal of the <see cref="HttpClient"/>.
     /// </summary>

src/HandlerKey.cs

@@ -1,18 +1,22 @@
-namespace Soenneker.Utils.HttpClientCache;
+using System.Net;
+using System.Net.Http;
+using System.Net.Security;
+
+namespace Soenneker.Utils.HttpClientCache;
 
 internal readonly record struct HandlerKey(
-    double LifetimeSeconds,
-    int MaxConnections,
+    long PooledConnectionLifetimeTicks,
+    int MaxConnectionsPerServer,
     bool UseCookies,
-    double ConnectTimeoutSeconds,
-    double? ResponseDrainTimeoutSeconds,
+    long ConnectTimeoutTicks,
+    long? ResponseDrainTimeoutTicks,
     bool? AllowAutoRedirect,
-    int? AutomaticDecompression,
-    double? KeepAlivePingDelaySeconds,
-    double? KeepAlivePingTimeoutSeconds,
-    int? KeepAlivePingPolicy,
+    DecompressionMethods? AutomaticDecompression,
+    long? KeepAlivePingDelayTicks,
+    long? KeepAlivePingTimeoutTicks,
+    HttpKeepAlivePingPolicy? KeepAlivePingPolicy,
     bool? UseProxy,
-    int? ProxyHashCode,
+    IWebProxy? Proxy,
     int? MaxResponseDrainSize,
     int? MaxResponseHeadersLength,
-    int? SslOptionsHashCode);
+    SslClientAuthenticationOptions? SslOptions);

src/HttpClientCache.cs

@@ -9,6 +9,7 @@
 using System.Collections.Generic;
 using System.Net;
 using System.Net.Http;
+using System.Net.Security;
 using System.Runtime.CompilerServices;
 using System.Threading;
 using System.Threading.Tasks;
@@ -18,61 +19,72 @@ namespace Soenneker.Utils.HttpClientCache;
 ///<inheritdoc cref="IHttpClientCache"/>
 public sealed class HttpClientCache : IHttpClientCache
 {
-    private readonly SingletonDictionary<HttpClient, Func<CancellationToken, ValueTask<HttpClientOptions?>>> _httpClients;
+    private readonly SingletonDictionary<HttpClient, OptionsFactory> _httpClients;
     private readonly ConcurrentDictionary<HandlerKey, SocketsHttpHandler> _handlers = new();
 
-    private static readonly Func<CancellationToken, ValueTask<HttpClientOptions?>> _nullOptionsFactory = static _ => default;
+    private static readonly bool _isBrowser = RuntimeUtil.IsBrowser();
+
+    private static readonly TimeSpan _defaultTimeout = TimeSpan.FromSeconds(100);
+    private static readonly TimeSpan _defaultConnectTimeout = TimeSpan.FromSeconds(100);
+    private static readonly TimeSpan _defaultPooledLifetime = TimeSpan.FromMinutes(10);
 
     public HttpClientCache()
     {
-        // We need the token-aware init path so we can call optionsFactory(token).
-        _httpClients =
-            new SingletonDictionary<HttpClient, Func<CancellationToken, ValueTask<HttpClientOptions?>>>(async (_, cancellationToken, optionsFactory) =>
-            {
-                HttpClientOptions? options = await optionsFactory(cancellationToken)
-                    .NoSync();
+        // Use method group to avoid a closure and still access instance state.
+        _httpClients = new SingletonDictionary<HttpClient, OptionsFactory>(InitializeHttpClient);
+    }
+
+    private async ValueTask<HttpClient> InitializeHttpClient(string _, CancellationToken cancellationToken, OptionsFactory factory)
+    {
+        HttpClientOptions? options = await factory.Invoke(cancellationToken)
+                                                  .NoSync();
 
-                HttpClient httpClient = CreateHttpClient(options);
+        HttpClient httpClient = CreateHttpClient(options);
 
-                await ConfigureHttpClient(httpClient, options)
-                    .NoSync();
+        await ConfigureHttpClient(httpClient, options)
+            .NoSync();
 
-                return httpClient;
-            });
+        return httpClient;
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public ValueTask<HttpClient> Get(string id, CancellationToken cancellationToken = default) =>
-        _httpClients.Get(id, () => _nullOptionsFactory, cancellationToken);
+        _httpClients.Get(id, OptionsFactory.Null, cancellationToken);
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public ValueTask<HttpClient> Get(string id, Func<CancellationToken, ValueTask<HttpClientOptions?>> optionsFactory,
-        CancellationToken cancellationToken = default) => _httpClients.Get(id, () => optionsFactory, cancellationToken);
+        CancellationToken cancellationToken = default) =>
+        _httpClients.Get(id, OptionsFactory.From(optionsFactory), cancellationToken);
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public ValueTask<HttpClient> Get(string id, Func<HttpClientOptions?> optionsFactory, CancellationToken cancellationToken = default) =>
-        _httpClients.Get(id, () => _ => new ValueTask<HttpClientOptions?>(optionsFactory()), cancellationToken);
+        _httpClients.Get(id, OptionsFactory.From(optionsFactory), cancellationToken);
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public ValueTask<HttpClient> Get(string id, Func<ValueTask<HttpClientOptions?>> optionsFactory, CancellationToken cancellationToken = default) =>
-        _httpClients.Get(id, () => _ => optionsFactory(), cancellationToken);
+        _httpClients.Get(id, OptionsFactory.From(optionsFactory), cancellationToken);
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public HttpClient GetSync(string id, CancellationToken cancellationToken = default) =>
-        _httpClients.GetSync(id, () => _nullOptionsFactory, cancellationToken);
+        _httpClients.GetSync(id, OptionsFactory.Null, cancellationToken);
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    public HttpClient GetSync(
-        string id, Func<CancellationToken, ValueTask<HttpClientOptions?>> optionsFactory, CancellationToken cancellationToken = default) =>
-        _httpClients.GetSync(id, () => optionsFactory, cancellationToken);
+    public HttpClient GetSync(string id, Func<CancellationToken, ValueTask<HttpClientOptions?>> optionsFactory,
+        CancellationToken cancellationToken = default) =>
+        _httpClients.GetSync(id, OptionsFactory.From(optionsFactory), cancellationToken);
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public HttpClient GetSync(string id, Func<HttpClientOptions?> optionsFactory, CancellationToken cancellationToken = default) =>
-        _httpClients.GetSync(id, () => _ => new ValueTask<HttpClientOptions?>(optionsFactory()), cancellationToken);
+        _httpClients.GetSync(id, OptionsFactory.From(optionsFactory), cancellationToken);
 
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public HttpClient GetSync(string id, Func<ValueTask<HttpClientOptions?>> optionsFactory, CancellationToken cancellationToken = default) =>
+        _httpClients.GetSync(id, OptionsFactory.From(optionsFactory), cancellationToken);
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     private HttpClient CreateHttpClient(HttpClientOptions? options)
     {
-        if (RuntimeUtil.IsBrowser())
+        if (_isBrowser)
         {
             return options?.HttpClientHandler != null ? new HttpClient(options.HttpClientHandler, disposeHandler: false) : new HttpClient();
         }
@@ -82,96 +94,92 @@ private HttpClient CreateHttpClient(HttpClientOptions? options)
             : new HttpClient(GetOrCreateHandler(options), disposeHandler: false);
     }
 
+    // Remove async state machine when ModifyClient is null.
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    private static async ValueTask ConfigureHttpClient(HttpClient httpClient, HttpClientOptions? options)
+    private static ValueTask ConfigureHttpClient(HttpClient httpClient, HttpClientOptions? options)
     {
-        httpClient.Timeout = options?.Timeout ?? TimeSpan.FromSeconds(100);
+        httpClient.Timeout = options?.Timeout ?? _defaultTimeout;
 
-        if (options?.BaseAddress != null)
-        {
-            Uri baseUri = new(options.BaseAddress);
+        // Prefer Uri to avoid parsing/allocation.
+        Uri? baseUri = options?.BaseAddressUri;
 
-            if (!Equals(httpClient.BaseAddress, baseUri))
-                httpClient.BaseAddress = baseUri;
-        }
+        if (baseUri is not null && !Equals(httpClient.BaseAddress, baseUri))
+            httpClient.BaseAddress = baseUri;
 
         AddDefaultRequestHeaders(httpClient, options?.DefaultRequestHeaders);
 
         Func<HttpClient, ValueTask>? modifyClient = options?.ModifyClient;
-
-        if (modifyClient is not null)
-            await modifyClient(httpClient)
-                .NoSync();
+        return modifyClient?.Invoke(httpClient) ?? default;
     }
 
     private SocketsHttpHandler GetOrCreateHandler(HttpClientOptions? options)
     {
-        double connectTimeoutSeconds = options?.Timeout?.TotalSeconds ?? 100;
-        
-        var key = new HandlerKey(
-            LifetimeSeconds: options?.PooledConnectionLifetime?.TotalSeconds ?? 600,
-            MaxConnections: options?.MaxConnectionsPerServer ?? 40,
-            UseCookies: options?.UseCookieContainer == true,
-            ConnectTimeoutSeconds: connectTimeoutSeconds,
-            ResponseDrainTimeoutSeconds: options?.ResponseDrainTimeout?.TotalSeconds,
-            AllowAutoRedirect: options?.AllowAutoRedirect,
-            AutomaticDecompression: (int?)options?.AutomaticDecompression,
-            KeepAlivePingDelaySeconds: options?.KeepAlivePingDelay?.TotalSeconds,
-            KeepAlivePingTimeoutSeconds: options?.KeepAlivePingTimeout?.TotalSeconds,
-            KeepAlivePingPolicy: (int?)options?.KeepAlivePingPolicy,
-            UseProxy: options?.UseProxy,
-            ProxyHashCode: options?.Proxy?.GetHashCode(),
-            MaxResponseDrainSize: options?.MaxResponseDrainSize,
-            MaxResponseHeadersLength: options?.MaxResponseHeadersLength,
-            SslOptionsHashCode: options?.SslOptions?.GetHashCode());
-
-        return _handlers.GetOrAdd(key, _ =>
+        // Do NOT tie connect timeout to request timeout.
+        TimeSpan connectTimeout = options?.ConnectTimeout ?? _defaultConnectTimeout;
+
+        // Extract refs so they become part of the key (no closure, no hash-key risk)
+        IWebProxy? proxy = options?.Proxy;
+        SslClientAuthenticationOptions? sslOptions = options?.SslOptions;
+
+        var key = new HandlerKey(PooledConnectionLifetimeTicks: (options?.PooledConnectionLifetime ?? _defaultPooledLifetime).Ticks,
+            MaxConnectionsPerServer: options?.MaxConnectionsPerServer ?? 40, UseCookies: options?.UseCookieContainer == true,
+            ConnectTimeoutTicks: connectTimeout.Ticks, ResponseDrainTimeoutTicks: options?.ResponseDrainTimeout?.Ticks,
+            AllowAutoRedirect: options?.AllowAutoRedirect, AutomaticDecompression: options?.AutomaticDecompression,
+            KeepAlivePingDelayTicks: options?.KeepAlivePingDelay?.Ticks, KeepAlivePingTimeoutTicks: options?.KeepAlivePingTimeout?.Ticks,
+            KeepAlivePingPolicy: options?.KeepAlivePingPolicy, UseProxy: options?.UseProxy, Proxy: proxy, MaxResponseDrainSize: options?.MaxResponseDrainSize,
+            MaxResponseHeadersLength: options?.MaxResponseHeadersLength, SslOptions: sslOptions);
+
+        // static factory => no closure allocation
+        return _handlers.GetOrAdd(key, static k => CreateHandlerFromKey(k));
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    private static SocketsHttpHandler CreateHandlerFromKey(in HandlerKey key)
+    {
+        var handler = new SocketsHttpHandler
         {
-            var handler = new SocketsHttpHandler
-            {
-                PooledConnectionLifetime = TimeSpan.FromSeconds(key.LifetimeSeconds),
-                MaxConnectionsPerServer = key.MaxConnections,
-                ConnectTimeout = TimeSpan.FromSeconds(key.ConnectTimeoutSeconds)
-            };
+            PooledConnectionLifetime = TimeSpan.FromTicks(key.PooledConnectionLifetimeTicks),
+            MaxConnectionsPerServer = key.MaxConnectionsPerServer,
+            ConnectTimeout = TimeSpan.FromTicks(key.ConnectTimeoutTicks)
+        };
 
-            if (key.UseCookies)
-                handler.CookieContainer = new CookieContainer();
+        if (key.UseCookies)
+            handler.CookieContainer = new CookieContainer();
 
-            if (key.ResponseDrainTimeoutSeconds.HasValue)
-                handler.ResponseDrainTimeout = TimeSpan.FromSeconds(key.ResponseDrainTimeoutSeconds.Value);
+        if (key.ResponseDrainTimeoutTicks.HasValue)
+            handler.ResponseDrainTimeout = TimeSpan.FromTicks(key.ResponseDrainTimeoutTicks.Value);
 
-            if (key.AllowAutoRedirect.HasValue)
-                handler.AllowAutoRedirect = key.AllowAutoRedirect.Value;
+        if (key.AllowAutoRedirect.HasValue)
+            handler.AllowAutoRedirect = key.AllowAutoRedirect.Value;
 
-            if (key.AutomaticDecompression.HasValue)
-                handler.AutomaticDecompression = (DecompressionMethods)key.AutomaticDecompression.Value;
+        if (key.AutomaticDecompression.HasValue)
+            handler.AutomaticDecompression = key.AutomaticDecompression.Value;
 
-            if (key.KeepAlivePingDelaySeconds.HasValue)
-                handler.KeepAlivePingDelay = TimeSpan.FromSeconds(key.KeepAlivePingDelaySeconds.Value);
+        if (key.KeepAlivePingDelayTicks.HasValue)
+            handler.KeepAlivePingDelay = TimeSpan.FromTicks(key.KeepAlivePingDelayTicks.Value);
 
-            if (key.KeepAlivePingTimeoutSeconds.HasValue)
-                handler.KeepAlivePingTimeout = TimeSpan.FromSeconds(key.KeepAlivePingTimeoutSeconds.Value);
+        if (key.KeepAlivePingTimeoutTicks.HasValue)
+            handler.KeepAlivePingTimeout = TimeSpan.FromTicks(key.KeepAlivePingTimeoutTicks.Value);
 
-            if (key.KeepAlivePingPolicy.HasValue)
-                handler.KeepAlivePingPolicy = (HttpKeepAlivePingPolicy)key.KeepAlivePingPolicy.Value;
+        if (key.KeepAlivePingPolicy.HasValue)
+            handler.KeepAlivePingPolicy = key.KeepAlivePingPolicy.Value;
 
-            if (key.UseProxy.HasValue)
-                handler.UseProxy = key.UseProxy.Value;
+        if (key.UseProxy.HasValue)
+            handler.UseProxy = key.UseProxy.Value;
 
-            if (options?.Proxy != null)
-                handler.Proxy = options.Proxy;
+        if (key.Proxy is not null)
+            handler.Proxy = key.Proxy;
 
-            if (key.MaxResponseDrainSize.HasValue)
-                handler.MaxResponseDrainSize = key.MaxResponseDrainSize.Value;
+        if (key.MaxResponseDrainSize.HasValue)
+            handler.MaxResponseDrainSize = key.MaxResponseDrainSize.Value;
 
-            if (key.MaxResponseHeadersLength.HasValue)
-                handler.MaxResponseHeadersLength = key.MaxResponseHeadersLength.Value;
+        if (key.MaxResponseHeadersLength.HasValue)
+            handler.MaxResponseHeadersLength = key.MaxResponseHeadersLength.Value;
 
-            if (options?.SslOptions != null)
-                handler.SslOptions = options.SslOptions;
+        if (key.SslOptions is not null)
+            handler.SslOptions = key.SslOptions;
 
-            return handler;
-        });
+        return handler;
     }
 
     private static void AddDefaultRequestHeaders(HttpClient httpClient, Dictionary<string, string>? headers)
@@ -183,13 +191,15 @@ private static void AddDefaultRequestHeaders(HttpClient httpClient, Dictionary<s
             httpClient.DefaultRequestHeaders.TryAddWithoutValidation(header.Key, header.Value);
     }
 
-    public ValueTask Remove(string id, CancellationToken cancellationToken = default) => _httpClients.Remove(id, cancellationToken);
+    public ValueTask Remove(string id, CancellationToken cancellationToken = default) =>
+        _httpClients.Remove(id, cancellationToken);
 
-    public void RemoveSync(string id, CancellationToken cancellationToken = default) => _httpClients.RemoveSync(id, cancellationToken);
+    public void RemoveSync(string id, CancellationToken cancellationToken = default) =>
+        _httpClients.RemoveSync(id, cancellationToken);
 
     private void DisposeHandlers()
     {
-        foreach (KeyValuePair<HandlerKey, SocketsHttpHandler> kvp in _handlers.ToArray())
+        foreach (KeyValuePair<HandlerKey, SocketsHttpHandler> kvp in _handlers)
         {
             if (_handlers.TryRemove(kvp.Key, out SocketsHttpHandler? handler))
                 handler.Dispose();

src/OptionsFactory.cs

@@ -0,0 +1,51 @@
+using Soenneker.Dtos.HttpClientOptions;
+using System;
+using System.Runtime.CompilerServices;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Soenneker.Utils.HttpClientCache;
+
+internal readonly struct OptionsFactory
+{
+    private readonly byte _kind; // 0=null, 1=token async, 2=sync, 3=async
+    private readonly Func<CancellationToken, ValueTask<HttpClientOptions?>>? _tokenAsync;
+    private readonly Func<HttpClientOptions?>? _sync;
+    private readonly Func<ValueTask<HttpClientOptions?>>? _async;
+
+    private OptionsFactory(byte kind, Func<CancellationToken, ValueTask<HttpClientOptions?>>? tokenAsync, Func<HttpClientOptions?>? sync,
+        Func<ValueTask<HttpClientOptions?>>? async)
+    {
+        _kind = kind;
+        _tokenAsync = tokenAsync;
+        _sync = sync;
+        _async = async;
+    }
+
+    public static OptionsFactory Null => default;
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static OptionsFactory From(Func<CancellationToken, ValueTask<HttpClientOptions?>> factory) =>
+        new(kind: 1, tokenAsync: factory, sync: null, async: null);
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static OptionsFactory From(Func<HttpClientOptions?> factory) =>
+        new(kind: 2, tokenAsync: null, sync: factory, async: null);
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static OptionsFactory From(Func<ValueTask<HttpClientOptions?>> factory) =>
+        new(kind: 3, tokenAsync: null, sync: null, async: factory);
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public ValueTask<HttpClientOptions?> Invoke(CancellationToken cancellationToken)
+    {
+        return _kind switch
+        {
+            0 => default,
+            1 => _tokenAsync!(cancellationToken),
+            2 => new ValueTask<HttpClientOptions?>(_sync!()),
+            3 => _async!(),
+            _ => default
+        };
+    }
+}