Refine #1446 tests and add module registration benchmark

- Remove AAA comments from module tests; reference #1446 in each
- Align test names with the file's existing convention
- Add independence guards: overriding one hook must not subscribe to the other event
- Add ModuleRegistrationBenchmark measuring container build cost vs module count

Author: tillig

bench/Autofac.Benchmarks/BenchmarkSet.cs

@@ -33,5 +33,6 @@ public static class BenchmarkSet
         typeof(MultiConstructorBenchmark),
         typeof(LambdaResolveBenchmark),
         typeof(RequiredPropertyBenchmark),
+        typeof(ModuleRegistrationBenchmark),
     };
 }

bench/Autofac.Benchmarks/ModuleRegistrationBenchmark.cs

@@ -0,0 +1,58 @@
+// Copyright (c) Autofac Project. All rights reserved.
+// Licensed under the MIT License. See LICENSE in the project root for license information.
+
+namespace Autofac.Benchmarks;
+
+/// <summary>
+/// Measures the cost of building a container that registers many modules which do
+/// not override <see cref="Module.AttachToComponentRegistration"/> or
+/// <see cref="Module.AttachToRegistrationSource"/>.
+/// </summary>
+/// <remarks>
+/// See https://github.com/autofac/Autofac/issues/1446. Prior to the fix, every module
+/// unconditionally subscribed to the registry's <c>Registered</c> and
+/// <c>RegistrationSourceAdded</c> events. Each subscription replays all existing
+/// registrations, so build time and allocations grew quadratically with the number of
+/// modules. Modules that do not override the hooks no longer subscribe, removing the
+/// quadratic behavior. Compare against a baseline package version (for example
+/// <c>--baseline-version 9.1.0</c>) to see the difference.
+/// </remarks>
+public class ModuleRegistrationBenchmark
+{
+    [Params(100, 1000)]
+    public int ModuleCount
+    {
+        get; set;
+    }
+
+    [Benchmark]
+    public void BuildContainerWithModules()
+    {
+        var builder = new ContainerBuilder();
+
+        for (var i = 0; i < ModuleCount; i++)
+        {
+            builder.RegisterModule<NoHookModule>();
+        }
+
+        using var container = builder.Build();
+        GC.KeepAlive(container);
+    }
+
+    // A module that overrides neither hook - the common case from issue #1446.
+    private sealed class NoHookModule : Module
+    {
+        protected override void Load(ContainerBuilder builder)
+        {
+            builder.RegisterType<Service>().As<IService>();
+        }
+
+        private interface IService
+        {
+        }
+
+        private sealed class Service : IService
+        {
+        }
+    }
+}

test/Autofac.Test/ModuleTests.cs

@@ -206,7 +206,8 @@ public void CanUseBuilderPropertyBag()
         Assert.Equal("value", builder.Properties["prop"]);
     }
 
-    // A module that overrides neither hook (the common case that caused the performance issue).
+    // #1446: A module that overrides neither hook (the common case that caused the
+    // O(N^2) performance issue) must not subscribe to the registry events at all.
     internal class NoHookModule : Module
     {
         protected override void Load(ContainerBuilder builder)
@@ -216,38 +217,35 @@ protected override void Load(ContainerBuilder builder)
     }
 
     [Fact]
-    public void NoHookModule_DoesNotSubscribeToRegisteredEvent()
+    public void ModuleWithoutHookDoesNotSubscribeToRegisteredEvent()
     {
-        // Arrange
+        // #1446: Subscribing replays every existing registration, so a module that
+        // does not override AttachToComponentRegistration must not subscribe.
         using var registryBuilder = Factory.CreateEmptyComponentRegistryBuilder();
 
-        // Act
         new NoHookModule().Configure(registryBuilder);
 
-        // Assert: the Registered event delegate stored in Properties should be null
-        // (no module subscription was added), meaning the key is either absent or null.
         var hasRegisteredHandler =
             registryBuilder.Properties.TryGetValue(MetadataKeys.RegisteredPropertyKey, out var registeredValue)
             && registeredValue is not null;
 
-        Assert.False(hasRegisteredHandler, "A module that does not override AttachToComponentRegistration should not subscribe to the Registered event.");
+        Assert.False(hasRegisteredHandler);
     }
 
     [Fact]
-    public void NoHookModule_DoesNotSubscribeToRegistrationSourceAddedEvent()
+    public void ModuleWithoutHookDoesNotSubscribeToRegistrationSourceAddedEvent()
     {
-        // Arrange
+        // #1446: A module that does not override AttachToRegistrationSource must not
+        // subscribe to the RegistrationSourceAdded event.
         using var registryBuilder = Factory.CreateEmptyComponentRegistryBuilder();
 
-        // Act
         new NoHookModule().Configure(registryBuilder);
 
-        // Assert: the RegistrationSourceAdded event delegate stored in Properties should be null.
         var hasSourceHandler =
             registryBuilder.Properties.TryGetValue(MetadataKeys.RegistrationSourceAddedPropertyKey, out var sourceValue)
             && sourceValue is not null;
 
-        Assert.False(hasSourceHandler, "A module that does not override AttachToRegistrationSource should not subscribe to the RegistrationSourceAdded event.");
+        Assert.False(hasSourceHandler);
     }
 
     // A module that overrides only AttachToComponentRegistration.
@@ -264,26 +262,24 @@ protected override void AttachToComponentRegistration(
     }
 
     [Fact]
-    public void ComponentRegistrationHookModule_SubscribesToRegisteredEvent()
+    public void ModuleOverridingComponentRegistrationHookSubscribesToRegisteredEvent()
     {
-        // Arrange
+        // #1446: A module that overrides AttachToComponentRegistration must still subscribe.
         using var registryBuilder = Factory.CreateEmptyComponentRegistryBuilder();
 
-        // Act
         new ComponentRegistrationHookModule().Configure(registryBuilder);
 
-        // Assert: the Registered event delegate stored in Properties should be non-null.
         var hasRegisteredHandler =
             registryBuilder.Properties.TryGetValue(MetadataKeys.RegisteredPropertyKey, out var registeredValue)
             && registeredValue is not null;
 
-        Assert.True(hasRegisteredHandler, "A module that overrides AttachToComponentRegistration must subscribe to the Registered event.");
+        Assert.True(hasRegisteredHandler);
     }
 
     [Fact]
-    public void ComponentRegistrationHookModule_HookIsCalledForSubsequentRegistrations()
+    public void ModuleOverridingComponentRegistrationHookStillReceivesRegistrations()
     {
-        // Verify the hook is still called when the override is present (regression guard).
+        // #1446: regression guard - the overridden hook must still fire for registrations.
         var module = new ComponentRegistrationHookModule();
         var builder = new ContainerBuilder();
         builder.RegisterModule(module);
@@ -294,6 +290,22 @@ public void ComponentRegistrationHookModule_HookIsCalledForSubsequentRegistratio
         Assert.NotEmpty(module.Seen);
     }
 
+    [Fact]
+    public void ModuleOverridingOnlyComponentRegistrationHookDoesNotSubscribeToRegistrationSourceAddedEvent()
+    {
+        // #1446: the two detection paths are independent - overriding only the
+        // component-registration hook must not subscribe to the source-added event.
+        using var registryBuilder = Factory.CreateEmptyComponentRegistryBuilder();
+
+        new ComponentRegistrationHookModule().Configure(registryBuilder);
+
+        var hasSourceHandler =
+            registryBuilder.Properties.TryGetValue(MetadataKeys.RegistrationSourceAddedPropertyKey, out var sourceValue)
+            && sourceValue is not null;
+
+        Assert.False(hasSourceHandler);
+    }
+
     // A module that overrides only AttachToRegistrationSource.
     internal class RegistrationSourceHookModule : Module
     {
@@ -308,20 +320,34 @@ protected override void AttachToRegistrationSource(
     }
 
     [Fact]
-    public void RegistrationSourceHookModule_SubscribesToRegistrationSourceAddedEvent()
+    public void ModuleOverridingRegistrationSourceHookSubscribesToRegistrationSourceAddedEvent()
     {
-        // Arrange
+        // #1446: A module that overrides AttachToRegistrationSource must still subscribe.
         using var registryBuilder = Factory.CreateEmptyComponentRegistryBuilder();
 
-        // Act
         new RegistrationSourceHookModule().Configure(registryBuilder);
 
-        // Assert: the RegistrationSourceAdded event delegate stored in Properties should be non-null.
         var hasSourceHandler =
             registryBuilder.Properties.TryGetValue(MetadataKeys.RegistrationSourceAddedPropertyKey, out var sourceValue)
             && sourceValue is not null;
 
-        Assert.True(hasSourceHandler, "A module that overrides AttachToRegistrationSource must subscribe to the RegistrationSourceAdded event.");
+        Assert.True(hasSourceHandler);
+    }
+
+    [Fact]
+    public void ModuleOverridingOnlyRegistrationSourceHookDoesNotSubscribeToRegisteredEvent()
+    {
+        // #1446: the two detection paths are independent - overriding only the
+        // source-added hook must not subscribe to the component-registered event.
+        using var registryBuilder = Factory.CreateEmptyComponentRegistryBuilder();
+
+        new RegistrationSourceHookModule().Configure(registryBuilder);
+
+        var hasRegisteredHandler =
+            registryBuilder.Properties.TryGetValue(MetadataKeys.RegisteredPropertyKey, out var registeredValue)
+            && registeredValue is not null;
+
+        Assert.False(hasRegisteredHandler);
     }
 
     internal class NullRegistrationSource : IRegistrationSource
@@ -335,9 +361,9 @@ public IEnumerable<IComponentRegistration> RegistrationsFor(Service service, Fun
     }
 
     [Fact]
-    public void RegistrationSourceHookModule_HookIsCalledWhenSourceIsAdded()
+    public void ModuleOverridingRegistrationSourceHookStillReceivesSources()
     {
-        // Verify the hook is still called when the override is present (regression guard).
+        // #1446: regression guard - the overridden hook must still fire for sources.
         var module = new RegistrationSourceHookModule();
         var builder = new ContainerBuilder();
         builder.RegisterModule(module);
@@ -371,10 +397,10 @@ protected override void Load(ContainerBuilder builder)
     }
 
     [Fact]
-    public void IntermediateBaseOverride_HookIsCalledForConcreteSubclass()
+    public void ModuleInheritingHookFromIntermediateBaseStillReceivesRegistrations()
     {
-        // A module that inherits the override from an intermediate class must still
-        // have its hook invoked (DeclaringType != typeof(Module) covers this case).
+        // #1446: the override-detection uses DeclaringType != typeof(Module), so an
+        // override inherited from an intermediate base class must still fire.
         var module = new ConcreteIntermediateModule();
         var builder = new ContainerBuilder();
         builder.RegisterModule(module);
@@ -386,8 +412,9 @@ public void IntermediateBaseOverride_HookIsCalledForConcreteSubclass()
     }
 
     [Fact]
-    public void IntermediateBaseOverride_SubscribesToRegisteredEvent()
+    public void ModuleInheritingHookFromIntermediateBaseSubscribesToRegisteredEvent()
     {
+        // #1446: a module whose override is declared on an intermediate base class must subscribe.
         using var registryBuilder = Factory.CreateEmptyComponentRegistryBuilder();
 
         new ConcreteIntermediateModule().Configure(registryBuilder);
@@ -396,7 +423,7 @@ public void IntermediateBaseOverride_SubscribesToRegisteredEvent()
             registryBuilder.Properties.TryGetValue(MetadataKeys.RegisteredPropertyKey, out var registeredValue)
             && registeredValue is not null;
 
-        Assert.True(hasRegisteredHandler, "A module with an override declared on an intermediate base class must still subscribe to the Registered event.");
+        Assert.True(hasRegisteredHandler);
     }
 
     private class Service1