diff --git a/bench/Autofac.Benchmarks/BenchmarkSet.cs b/bench/Autofac.Benchmarks/BenchmarkSet.cs index 316b1a0e7..29ca87a78 100644 --- a/bench/Autofac.Benchmarks/BenchmarkSet.cs +++ b/bench/Autofac.Benchmarks/BenchmarkSet.cs @@ -10,6 +10,8 @@ public static class BenchmarkSet public static readonly Type[] All = { typeof(ChildScopeResolveBenchmark), + typeof(ContainerBuildBenchmark), + typeof(ContainerBuildInheritedMembersBenchmark), typeof(ConcurrencyBenchmark), typeof(ConcurrencyNestedScopeBenchmark), typeof(KeyedGenericBenchmark), diff --git a/bench/Autofac.Benchmarks/ContainerBuildBenchmark.cs b/bench/Autofac.Benchmarks/ContainerBuildBenchmark.cs new file mode 100644 index 000000000..c95a88b5d --- /dev/null +++ b/bench/Autofac.Benchmarks/ContainerBuildBenchmark.cs @@ -0,0 +1,44 @@ +// Copyright (c) Autofac Project. All rights reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. + +namespace Autofac.Benchmarks; + +/// +/// Measures container build (registration) time, isolated from any resolve activity. +/// +/// +/// Building a container constructs a +/// for each +/// reflection-based registration, which inspects the implementation type's members. +/// This benchmark guards that per-registration cost as the registration count grows. +/// +public class ContainerBuildBenchmark +{ + [Params(1000, 5000)] + public int RegistrationCount + { + get; set; + } + + [Benchmark] + public void Build() + { + var builder = new ContainerBuilder(); + + for (var i = 0; i < RegistrationCount; i++) + { + builder.RegisterType().As(); + } + + using var container = builder.Build(); + GC.KeepAlive(container); + } + + private interface IComponent + { + } + + private sealed class Component : IComponent + { + } +} diff --git a/bench/Autofac.Benchmarks/ContainerBuildInheritedMembersBenchmark.cs b/bench/Autofac.Benchmarks/ContainerBuildInheritedMembersBenchmark.cs new file mode 100644 index 000000000..368190942 --- /dev/null +++ b/bench/Autofac.Benchmarks/ContainerBuildInheritedMembersBenchmark.cs @@ -0,0 +1,259 @@ +// Copyright (c) Autofac Project. All rights reserved. +// Licensed under the MIT License. See LICENSE in the project root for license information. + +using Autofac.Core; + +namespace Autofac.Benchmarks; + +/// +/// Measures container build (registration) time for many distinct implementation +/// types that share a base class exposing many inherited writable properties. +/// +/// +/// +/// scans each +/// implementation type for [ServiceKey] on constructor parameters and properties. +/// Scanning inherited members from the derived type yields +/// objects whose ReflectedType is the +/// derived type, so a per-member attribute cache misses on every (derived type x inherited +/// member) pair - O(types x inherited members) of attribute reflection during registration. +/// Scanning declared-only members per type and recursing into the base type collapses that +/// to O(distinct declared members). +/// +/// +/// This benchmark exists because cannot detect that +/// cost: it registers a single sealed type with no properties and no base class, so there +/// are no inherited members to rescan, and the per-type memoization makes the scan a one-off +/// regardless of registration count. The two TypeCount values are the point of this +/// benchmark - a regression shows up as super-linear growth between them. +/// +/// +/// No [ServiceKey] attribute appears anywhere below, deliberately: the scan cost is +/// paid by every reflection-based registration whether or not the feature is used. +/// +/// +public class ContainerBuildInheritedMembersBenchmark +{ + private const int InheritedPropertyCount = 20; + + private List _implementationTypes = new(); + + [Params(250, 1000)] + public int TypeCount + { + get; set; + } + + [GlobalSetup] + public void Setup() + { + _implementationTypes = DistinctDerivedTypes(TypeCount); + + // Guard the fixture itself: the benchmark is only meaningful while the registered + // types really are distinct and really do inherit a wide set of writable properties. + if (_implementationTypes.Distinct().Count() != TypeCount) + { + throw new InvalidOperationException("Implementation types are not distinct."); + } + + var writableInheritedProperties = typeof(WideBase).GetProperties().Count(property => property.CanWrite); + if (writableInheritedProperties != InheritedPropertyCount) + { + throw new InvalidOperationException( + $"Expected {InheritedPropertyCount} writable properties on {nameof(WideBase)} but found {writableInheritedProperties}."); + } + } + + [Benchmark] + public void Build() + { + // The [ServiceKey] scan caches live in the process-wide ReflectionCacheSet.Shared and + // are not cleared on container build (their usage includes resolution), so without + // this reset only the first invocation would scan anything and BenchmarkDotNet's + // steady-state measurement would report a warm no-op. Clearing here - rather than in + // an [IterationSetup], which would require [InvocationCount(1)] and yields a + // multimodal, high-variance distribution - keeps the reset inside the measured region + // whatever invocation count BenchmarkDotNet picks. The clear itself is proportional to + // the entries this benchmark created, and is negligible next to the scan it forces. + ReflectionCacheSet.Shared.Clear(); + + var builder = new ContainerBuilder(); + + foreach (var implementationType in _implementationTypes) + { + builder.RegisterType(implementationType); + } + + using var container = builder.Build(); + GC.KeepAlive(container); + } + + /// + /// Produces distinct closed types, all + /// inheriting . The index is encoded as a fixed-depth nesting of + /// over two markers, which yields distinct type arguments + /// without a hand-written type declaration per registration. Nesting depth stays at + /// log2(count), so type names never get deep enough for name construction to skew the + /// measurement. + /// + /// The number of distinct types to produce. + /// The distinct closed implementation types. + private static List DistinctDerivedTypes(int count) + { + var bits = 1; + while (1 << bits < count) + { + bits++; + } + + var types = new List(count); + + for (var i = 0; i < count; i++) + { + var argument = typeof(Zero); + + for (var bit = 0; bit < bits; bit++) + { + var next = ((i >> bit) & 1) == 1 ? typeof(One) : typeof(Zero); + argument = typeof(Wrap<,>).MakeGenericType(argument, next); + } + + types.Add(typeof(Derived<>).MakeGenericType(argument)); + } + + return types; + } + + /// + /// Stands in for a framework or domain base class with many injectable properties; every + /// registered type inherits these writable members. + /// + private class WideBase + { + public string? Property01 + { + get; set; + } + + public string? Property02 + { + get; set; + } + + public string? Property03 + { + get; set; + } + + public string? Property04 + { + get; set; + } + + public string? Property05 + { + get; set; + } + + public string? Property06 + { + get; set; + } + + public string? Property07 + { + get; set; + } + + public string? Property08 + { + get; set; + } + + public string? Property09 + { + get; set; + } + + public string? Property10 + { + get; set; + } + + public string? Property11 + { + get; set; + } + + public string? Property12 + { + get; set; + } + + public string? Property13 + { + get; set; + } + + public string? Property14 + { + get; set; + } + + public string? Property15 + { + get; set; + } + + public string? Property16 + { + get; set; + } + + public string? Property17 + { + get; set; + } + + public string? Property18 + { + get; set; + } + + public string? Property19 + { + get; set; + } + + public string? Property20 + { + get; set; + } + } + + /// + /// The registered implementation type. Declares no members of its own, so all of + /// 's properties reach it by inheritance. + /// + /// Marker type argument that makes each closed type distinct. + private sealed class Derived : WideBase + { + } + + private sealed class Zero + { + } + + private sealed class One + { + } + + /// + /// Combines two marker types so an index can be encoded as a nesting of them. + /// + /// First marker. + /// Second marker. + private sealed class Wrap + { + } +} diff --git a/src/Autofac/Core/Activators/Reflection/ReflectionActivator.cs b/src/Autofac/Core/Activators/Reflection/ReflectionActivator.cs index 6814694f1..cf080cf02 100644 --- a/src/Autofac/Core/Activators/Reflection/ReflectionActivator.cs +++ b/src/Autofac/Core/Activators/Reflection/ReflectionActivator.cs @@ -52,14 +52,7 @@ public ReflectionActivator( } _implementationType = implementationType; - - // The cache key is the (already annotated) implementation type; the factory - // ignores the dictionary's unannotated key parameter and reads the annotated - // local instead, so the [DynamicallyAccessedMembers] contract flows correctly - // into UsesServiceKeyAttribute. - _requiresServiceKeyParameter = ReflectionCacheSet.Shared.Internal.ServiceKeyUsageByType.GetOrAdd( - _implementationType, - _ => UsesServiceKeyAttribute(implementationType)); + _requiresServiceKeyParameter = UsesServiceKeyAttributeCached(implementationType); ConstructorFinder = constructorFinder ?? throw new ArgumentNullException(nameof(constructorFinder)); ConstructorSelector = constructorSelector ?? throw new ArgumentNullException(nameof(constructorSelector)); _configuredProperties = configuredProperties.ToArray(); @@ -133,6 +126,10 @@ public void ConfigurePipeline(IComponentRegistryServices componentRegistryServic "Trimming", "IL2070:UnrecognizedReflectionPattern", Justification = "This is a best-effort scan for [ServiceKey] across all constructors and properties, including non-public ones. Autofac's trim/AOT contract preserves only public constructors and properties (see ActivatorMemberTypes); if a non-public member is trimmed away, it simply is not found here, which matches the documented behavior that non-public activation is not trim/AOT-safe. No public member that drives activation is missed.")] + [UnconditionalSuppressMessage( + "Trimming", + "IL2072:UnrecognizedReflectionPattern", + Justification = "Recurses into BaseType to inspect declared members level-by-level. BaseType members are preserved via the derived type's ActivatorMemberTypes annotation; if a base member is trimmed it simply is not found, matching the documented best-effort behavior.")] private static bool UsesServiceKeyAttribute([DynamicallyAccessedMembers(ActivatorMemberTypes.ActivatedType)] Type implementationType) { // Intentionally not picky about _which_ constructor or property has the @@ -141,7 +138,24 @@ private static bool UsesServiceKeyAttribute([DynamicallyAccessedMembers(Activato // where we "may or may not need it." If you mark a property with the // attribute but never inject properties, we'll still provide the // parameter "just in case" you change your mind at runtime. - foreach (var constructor in implementationType.GetConstructors(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)) + // + // We scan DeclaredOnly members and recurse into the base type (memoized per + // type via ServiceKeyUsageByType). This is critical for performance: scanning + // *inherited* members via GetProperties on a derived type yields PropertyInfo + // objects whose ReflectedType is the derived type, so the per-member attribute + // cache misses on every (derived type x inherited member) pair - O(types x + // inherited members) of attribute reflection. Scanning DeclaredOnly anchors each + // member to its declaring type so the cache is shared across all derived types. + // + // This also makes the scan more conservative than the one it replaces: it now + // additionally sees private base properties, members hidden by new or override, + // and base constructor parameters. That is safe - the flag only gates whether + // the key parameter is offered, and KeyedServiceKeyParameter.CanSupplyValue + // re-checks the attribute per parameter - so a false positive costs one unused + // Parameter, never a wrong injection. + const BindingFlags DeclaredMembers = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly; + + foreach (var constructor in implementationType.GetConstructors(DeclaredMembers)) { foreach (var parameter in constructor.GetParameters()) { @@ -152,7 +166,7 @@ private static bool UsesServiceKeyAttribute([DynamicallyAccessedMembers(Activato } } - foreach (var property in implementationType.GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)) + foreach (var property in implementationType.GetProperties(DeclaredMembers)) { if (property.CanWrite && ServiceKeyAttributeCache.PropertyHasServiceKey(property)) { @@ -160,9 +174,27 @@ private static bool UsesServiceKeyAttribute([DynamicallyAccessedMembers(Activato } } + var baseType = implementationType.BaseType; + if (baseType is not null && baseType != typeof(object)) + { + // IL2072 suppressed on this method: baseType is unannotated but its members + // are preserved transitively via the derived type's annotation. + return UsesServiceKeyAttributeCached(baseType); + } + return false; } + // Memoizes the per-type result so each base type in a hierarchy is scanned once, + // regardless of how many derived types share it. The factory captures the annotated + // 'type' local (the dictionary key parameter is ignored) so the DynamicallyAccessedMembers + // contract flows without an unannotated-to-annotated assignment. + [SuppressMessage("Major Code Smell", "S6612:The lambda parameter should be used instead of capturing arguments", Justification = "The factory deliberately reads the [DynamicallyAccessedMembers]-annotated 'type' local rather than the unannotated lambda parameter so the trimming contract flows into UsesServiceKeyAttribute.")] + private static bool UsesServiceKeyAttributeCached([DynamicallyAccessedMembers(ActivatorMemberTypes.ActivatedType)] Type type) + => ReflectionCacheSet.Shared.Internal.ServiceKeyUsageByType.GetOrAdd( + type, + _ => UsesServiceKeyAttribute(type)); + private void UseSingleConstructorActivation(IResolvePipelineBuilder pipelineBuilder, ConstructorBinder singleConstructor) { if (singleConstructor.ParameterCount == 0) diff --git a/test/Autofac.Specification.Test/Features/KeyedServiceTests.cs b/test/Autofac.Specification.Test/Features/KeyedServiceTests.cs index e6c8a1543..6dd389f54 100644 --- a/test/Autofac.Specification.Test/Features/KeyedServiceTests.cs +++ b/test/Autofac.Specification.Test/Features/KeyedServiceTests.cs @@ -1075,4 +1075,161 @@ public TVal Value get; } } + + [Fact] + public void ResolveKeyedServiceWithServiceKeyPropertyOnBaseClass() + { + // Issue #1480: A [ServiceKey] property inherited from a base class still receives the key. + var builder = new ContainerBuilder(); + builder.RegisterType().Keyed("inherited-property").PropertiesAutowired(); + var provider = builder.Build(); + + var svc = provider.ResolveKeyed("inherited-property"); + + Assert.Equal("inherited-property", svc.Key); + } + + [Fact] + public void ResolveKeyedServiceWithServiceKeyConstructorParameterOnDeclaringType() + { + // Issue #1480: Baseline for the inherited cases - [ServiceKey] on the type's own constructor parameter. + var builder = new ContainerBuilder(); + builder.RegisterType().Keyed("own-parameter"); + var provider = builder.Build(); + + var svc = provider.ResolveKeyed("own-parameter"); + + Assert.Equal("own-parameter", svc.Key); + } + + [Fact] + public void ResolveKeyedServiceWithServiceKeyConstructorParameterOnBaseClassOnly() + { + // Issue #1480: The base constructor parameter carries [ServiceKey], but the derived + // constructor passes its own literal, so the key must not reach it. + var builder = new ContainerBuilder(); + builder.RegisterType().Keyed("ignored-key"); + var provider = builder.Build(); + + var svc = provider.ResolveKeyed("ignored-key"); + + Assert.Equal("literal-from-derived", svc.Key); + } + + [Fact] + public void ResolveKeyedServiceWithPrivateServiceKeyPropertyOnBaseClass() + { + // Issue #1480: A private [ServiceKey] property on a base class is outside the injection + // surface (public setters only), so the resolve succeeds with the property left unset. + var builder = new ContainerBuilder(); + builder.RegisterType().Keyed("private-property").PropertiesAutowired(); + var provider = builder.Build(); + + var svc = provider.ResolveKeyed("private-property"); + + Assert.NotNull(svc); + Assert.Null(svc.ReadPrivateKey()); + } + + [Fact] + public void ResolveKeyedServiceWithRequiredServiceKeyPropertyOnBaseClass() + { + // Issue #1480: An inherited 'required' [ServiceKey] property receives the key even + // without PropertiesAutowired, because required members are always populated. + var builder = new ContainerBuilder(); + builder.RegisterType().Keyed("inherited-required"); + var provider = builder.Build(); + + var svc = provider.ResolveKeyed("inherited-required"); + + Assert.Equal("inherited-required", svc.Key); + } + + [Fact] + public void ResolveKeyedServiceWithDeepHierarchyAndNoServiceKey() + { + // Issue #1480: A multi-level hierarchy with no [ServiceKey] anywhere resolves normally - + // the base-type walk terminates without requiring the key parameter. + var builder = new ContainerBuilder(); + builder.RegisterType().Keyed("no-attribute").PropertiesAutowired(); + var provider = builder.Build(); + + var svc = provider.ResolveKeyed("no-attribute"); + + Assert.NotNull(svc); + } + + private class KeyAwareBase + { + [ServiceKey] + public string Key { get; set; } = default!; + } + + private class DerivedFromKeyAwareBase : KeyAwareBase + { + } + + private class OwnKeyConstructorService + { + public OwnKeyConstructorService([ServiceKey] string key) => Key = key; + + public string Key + { + get; + } + } + + private abstract class KeyAwareConstructorBase + { + protected KeyAwareConstructorBase([ServiceKey] string key) => Key = key; + + public string Key + { + get; + } + } + + private class DerivedPassingLiteralToKeyAwareBase : KeyAwareConstructorBase + { + public DerivedPassingLiteralToKeyAwareBase() + : base("literal-from-derived") + { + } + } + + private class PrivateKeyAwareBase + { + [ServiceKey] + private string PrivateKey { get; set; } = default!; + + public string ReadPrivateKey() => PrivateKey; + } + + private class DerivedFromPrivateKeyAwareBase : PrivateKeyAwareBase + { + } + + private class RequiredKeyAwareBase + { + [ServiceKey] + public required string Key { get; set; } = default!; + } + + private class DerivedFromRequiredKeyAwareBase : RequiredKeyAwareBase + { + } + + private class DeepLevel1 + { + public string FirstLevel { get; set; } = default!; + } + + private class DeepLevel2 : DeepLevel1 + { + public string SecondLevel { get; set; } = default!; + } + + private class DeepLevel3 : DeepLevel2 + { + } }