Skip to content

Only subscribe Module registry events when hooks are overridden (#1446)#1487

Merged
tillig merged 2 commits into
developfrom
feature/issue-1446
Jun 15, 2026
Merged

Only subscribe Module registry events when hooks are overridden (#1446)#1487
tillig merged 2 commits into
developfrom
feature/issue-1446

Conversation

@tillig

@tillig tillig commented Jun 15, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #1446. Module.Configure unconditionally subscribed lambda event handlers to componentRegistry.Registered and componentRegistry.RegistrationSourceAdded, even when the concrete module overrode neither AttachToComponentRegistration nor AttachToRegistrationSource. Each subscription replays every existing registration, so with many registered modules this produced O(N²) delegate invocations and allocations at build time (the reporter measured ~1.56 GB allocated and ~22× slower build vs. ~55 MB without the subscriptions).

Fix

The two private subscription helpers now subscribe only when the derived module type actually overrides the corresponding virtual method, detected via reflection (GetMethod(...).DeclaringType != typeof(Module), using exact parameter-type arrays to select the right overload). This correctly handles overrides declared in an intermediate base class. Results are cached per-type in InternalReflectionCaches (registration-scoped, cleared after build) to avoid repeated reflection.

Modules that override the hooks behave identically to before; only modules that override neither stop subscribing — which is the entire point.

Tests

Tests reference #1446 and follow the repo conventions (no AAA comments; names aligned with the existing file style). Coverage:

  • A no-hook module subscribes to neither event.
  • A module overriding AttachToComponentRegistration (or AttachToRegistrationSource) still subscribes and still has its hook invoked (regression guards).
  • An override inherited from an intermediate abstract base class still subscribes and fires.
  • Independence guards: overriding only one hook must not subscribe to the other event (the two detection paths are independent).

Each "does not subscribe" test was confirmed to fail when the fix is disabled (forced always-subscribe), so they are genuine guards rather than tautologies. One pre-existing test (ModifiedScopesHaveTheirOwnDelegate) was updated to use TryGetValue for a registry property key that is now legitimately absent when no hook is overridden.

Benchmark

Adds ModuleRegistrationBenchmark (bench/Autofac.Benchmarks/, registered in BenchmarkSet.All) measuring container build cost vs. module count, with the MemoryDiagnoser from the existing config. Run it against a pre-fix package to see the difference:

dotnet run -c Release --project bench/Autofac.Benchmarks -- --baseline-version 9.1.0 --filter *ModuleRegistrationBenchmark*

Measured results (1000 no-hook modules):

Version Mean time Allocated
Package 9.1.0 (before) 11,776 µs 22,338 KB
Source (with fix) 1,825 µs 5,096 KB
Ratio 0.16 (≈6.4× faster) 0.23 (≈4.4× less)

Build time also scales roughly linearly with module count after the fix instead of quadratically.

No public API changes. Zero warnings/errors; full suites pass on net8.0 and net10.0.

…not overridden

In Module.Configure, AttachToRegistrations and AttachToSources previously
subscribed lambda event handlers to componentRegistry.Registered and
componentRegistry.RegistrationSourceAdded unconditionally — even when the
concrete module subclass overrode neither AttachToComponentRegistration nor
AttachToRegistrationSource. With many modules this caused O(N^2) delegate
invocations and excessive closure allocations at container build time.

The fix uses reflection to check whether the virtual method is overridden on
the concrete module type (DeclaringType != typeof(Module)), and only subscribes
the event handler when an override is present. The check handles overrides on
intermediate base classes correctly. Results are cached per-type in the
existing InternalReflectionCaches infrastructure (Usage = Registration) to
avoid repeated reflection on subsequent Configure calls for the same type.
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.45455% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.81%. Comparing base (f8d4095) to head (75ca161).
⚠️ Report is 6 commits behind head on develop.

Files with missing lines Patch % Lines
src/Autofac/Module.cs 94.44% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1487      +/-   ##
===========================================
+ Coverage    77.69%   77.81%   +0.11%     
===========================================
  Files          217      217              
  Lines         5829     5874      +45     
  Branches      1253     1258       +5     
===========================================
+ Hits          4529     4571      +42     
- Misses         764      765       +1     
- Partials       536      538       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance & Memory Issue on container setup

1 participant