diff --git a/CHANGELOG.md b/CHANGELOG.md index 060bb1c..b670754 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,11 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [1.0.0-rc9230] - 2026-08-12 + +Everything since `1.0.0-rc9210`. Still a release candidate: convention registration and the NUnit +integration are both new, and `DecoratorRegistration` changed shape, so the surface is not committed +to yet. ### Added @@ -46,6 +50,52 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Module loading costs about a third less to start.** Registering 200 services takes 6µs; the + first `AddModules` call took 4.16ms and the second 0.03ms, so nearly all of it was one-time JIT + and type loading rather than work that scales with the number of services. An empty module cost + 2.9ms against a 0.62ms floor for a bare `ServiceCollection`. Measured against that: + + `ProcessModuleEnvironment` built a `ConcurrentDictionary` on every `AddModules` call to serve a + cache most applications never read; it is allocated on first process read instead. Module + discovery used `List.Contains`, which routes through `EqualityComparer.Default` + — constructing that for an interface was the single most expensive step in the load path, to + compare a list that usually holds one item. The interface defaults returned + `ArraySegment.Empty` and reached the empty case by building an enumerator; they return + `Array.Empty()` and the empty case is a `Count` test. The environment lookup and its guard + walked the collection twice and now share one scan. The lists in `DependencyRegistry` allocate + on first use, and the `System.Linq` tokens in `GetModules` moved behind a non-inlined method so + that assembly is not loaded for applications that never call `AddModule`. + + Empty module 2.92ms → 1.81ms; 200 services 4.44ms → 3.17ms; 42 → 34 methods JIT-ed. Native AOT + startup was already 0.02ms and is unchanged — there is no JIT there, so for AOT this is a size + change, 33KB off a published binary. + +- **`ApplicationModule` defers to a declared module instead of repeating it.** A project with a + `Program.cs` gets an `ApplicationModule` whether or not it declares a module of its own, and both + are modules with no realm restriction, so both registered every service in the compilation — the + registrations, decorations *and* interceptions were each emitted twice, byte for byte. In a 200 + service project the duplicate was 5,413 bytes of IL, 44% of the assembly and 21% of the + ReadyToRun image, dead in every application that never names `ApplicationModule`. + + The auto module now returns the declared one from `InternalGetModules`, so + `AddModule()` registers exactly what it always did from one copy. It defers + only to a module with no realm restriction and no constructor parameters, since an `OnlyRealm` + module takes just the registrations aimed at it and deferring to one would drop the rest. + Assembly IL for that project falls from 17,763 to 12,337 bytes. + + **Behaviour change:** loading `ApplicationModule` alongside the module it defers to now registers + each service once. It previously registered everything twice, because the two carried independent + copies of the same registrations. + +- **`DecoratorRegistration` is a sealed class rather than a readonly struct.** As a struct it forced + its own instantiation of `List` and of the LINQ ordering machinery — 44 methods JIT-ed to sort + three decorators, 13% of every method compiled in the process. Ordering is a stable insertion sort + now, so no LINQ is instantiated for it at all. + + **Breaking:** this changes the signature encoding of `IEnumerable`, so an + assembly compiled against an earlier runtime throws `MissingMethodException` from + `IDependencyModule.InternalGetDecorators` until it is rebuilt. Source is unaffected. + - **A generator declaring its own module attribute gets the module written for it.** `BaseSourceGenerator.SetupRootGenerator` was `virtual` and empty, so a framework naming its own attribute through `ModuleAttributeTypes()` and not overriding it compiled cleanly, emitted no @@ -98,6 +148,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **The single-argument apply overloads no longer refuse the environment they were given.** + `FindOrCreateEnvironment` ran its guard before its lookup, so `DependencyRegistry.ApplyServices` + and `ApplyDecorators` taking only a service collection — and the generated + `IDependencyModule.InternalApplyServices(IServiceCollection)` that calls into them — threw for any + collection holding an `IModuleEnvironment`, including one registered as the singleton instance + they document as the way to supply it. The message said it was not registered as a singleton + instance while it was. The lookup runs first now, matching the two-argument path, which always had + the order right. + + The guard still fires for an environment registered by type or by factory, which cannot decide + registrations because there is no provider to build it from yet. It now tests the descriptor the + container would actually resolve rather than any match, so an unusable registration shadowed by a + usable one is no longer reported. + - **The narrowest `IServiceProviderBuilderAttribute` now wins.** Both integrations took the *first* match out of an attribute list ordered widest scope first, so an assembly-level container builder silently beat one on the class or the method — the reverse of the interface's own documentation, @@ -458,4 +522,5 @@ The entries below were written for a 1.0.0 that was not cut. They describe the s Enable it with ``. - A tag-driven release workflow publishing to nuget.org and GitHub Packages. +[1.0.0-rc9230]: https://github.com/ipjohnson/DependencyModules/releases/tag/v1.0.0-rc9230 [1.0.0-rc9210]: https://github.com/ipjohnson/DependencyModules/releases/tag/v1.0.0-rc9210 diff --git a/Directory.Build.props b/Directory.Build.props index 4743727..2f99ec5 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -7,6 +7,11 @@ --> 1.0.0 + + rc9230 1.0.0.0 1.0.0.0 diff --git a/src/DependencyModules.Runtime/Helpers/DecoratorRegistration.cs b/src/DependencyModules.Runtime/Helpers/DecoratorRegistration.cs index 41d7503..c6fc7a9 100644 --- a/src/DependencyModules.Runtime/Helpers/DecoratorRegistration.cs +++ b/src/DependencyModules.Runtime/Helpers/DecoratorRegistration.cs @@ -19,7 +19,7 @@ namespace DependencyModules.Runtime.Helpers; /// The function that rewrites registrations in the collection, receiving the environment any /// condition on the decorator is evaluated against. /// -public readonly struct DecoratorRegistration(int order, EnvironmentRegistryFunc registryFunc) { +public sealed class DecoratorRegistration(int order, EnvironmentRegistryFunc registryFunc) { /// /// A decorator with no environment condition. /// diff --git a/src/DependencyModules.Runtime/Helpers/DependencyRegistry.cs b/src/DependencyModules.Runtime/Helpers/DependencyRegistry.cs index e31eaf3..46e3811 100644 --- a/src/DependencyModules.Runtime/Helpers/DependencyRegistry.cs +++ b/src/DependencyModules.Runtime/Helpers/DependencyRegistry.cs @@ -1,4 +1,5 @@ using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; using DependencyModules.Runtime.Features; using DependencyModules.Runtime.Interfaces; using Microsoft.Extensions.DependencyInjection; @@ -36,9 +37,9 @@ public delegate void EnvironmentRegistryFunc( public class DependencyRegistry { // ReSharper disable StaticMemberInGenericType private static readonly object SyncLock = new(); - private static readonly List RegistryFuncs = []; - private static readonly List Decorators = []; - private static readonly List Modules = []; + private static List? RegistryFuncs; + private static List? Decorators; + private static List? Modules; /// /// Add registration func @@ -47,7 +48,7 @@ public class DependencyRegistry { /// public static int Add(RegistryFunc registryFunc) { lock (SyncLock) { - RegistryFuncs.Add((serviceCollection, _) => registryFunc(serviceCollection)); + (RegistryFuncs ??= []).Add((serviceCollection, _) => registryFunc(serviceCollection)); } return 1; @@ -60,7 +61,7 @@ public static int Add(RegistryFunc registryFunc) { /// public static int Add(EnvironmentRegistryFunc registryFunc) { lock (SyncLock) { - RegistryFuncs.Add(registryFunc); + (RegistryFuncs ??= []).Add(registryFunc); } return 1; @@ -77,7 +78,7 @@ public static int Add( Func provider, ServiceLifetime lifetime = ServiceLifetime.Transient) where TInstance : class { lock (SyncLock) { - RegistryFuncs.Add( + (RegistryFuncs ??= []).Add( (registry, _) => registry.Add( new ServiceDescriptor( typeof(TInstance), @@ -107,7 +108,7 @@ public static int Add( ServiceLifetime lifetime = ServiceLifetime.Transient, object? serviceKey = null) where TInstance : class { lock (SyncLock) { - RegistryFuncs.Add( + (RegistryFuncs ??= []).Add( (registry, _) => registry.Add( new ServiceDescriptor( typeof(TInstance), @@ -134,7 +135,7 @@ public static int Add( /// public static int AddDecorator(RegistryFunc registryFunc, int order = 0) { lock (SyncLock) { - Decorators.Add(new DecoratorRegistration(order, registryFunc)); + (Decorators ??= []).Add(new DecoratorRegistration(order, registryFunc)); } return 1; @@ -153,7 +154,7 @@ public static int AddDecorator(RegistryFunc registryFunc, int order = 0) { /// public static int AddDecorator(EnvironmentRegistryFunc registryFunc, int order = 0) { lock (SyncLock) { - Decorators.Add(new DecoratorRegistration(order, registryFunc)); + (Decorators ??= []).Add(new DecoratorRegistration(order, registryFunc)); } return 1; @@ -166,7 +167,7 @@ public static int AddDecorator(EnvironmentRegistryFunc registryFunc, int order = /// public static int AddModule(params IDependencyModule[] modules) { lock (SyncLock) { - Modules.AddRange(modules); + (Modules ??= []).AddRange(modules); } return 1; @@ -204,6 +205,9 @@ public static void ApplyServices(IServiceCollection serviceCollection) { public static void ApplyServices(IServiceCollection serviceCollection, IModuleEnvironment environment) { EnvironmentRegistryFunc[] snapshot; lock (SyncLock) { + if (RegistryFuncs == null) { + return; + } snapshot = RegistryFuncs.ToArray(); } @@ -231,9 +235,27 @@ public static void ApplyDecorators(IServiceCollection serviceCollection) { /// variables and give the same answers. /// private static IModuleEnvironment FindOrCreateEnvironment(IServiceCollection serviceCollection) { - RefuseUnusableEnvironment(serviceCollection); + var environment = FindModuleEnvironment(serviceCollection); - return FindModuleEnvironment(serviceCollection) ?? ModuleEnvironment.CreateDefault(); + return environment ?? ModuleEnvironment.CreateDefault(); + } + + /// + /// Stable insertion sort by Order. Replaces OrderBy so that no LINQ ordering machinery is + /// instantiated for DecoratorRegistration at startup. + /// + private static void SortByOrder(List list) { + for (var i = 1; i < list.Count; i++) { + var item = list[i]; + var j = i - 1; + + while (j >= 0 && list[j].Order > item.Order) { + list[j + 1] = list[j]; + j--; + } + + list[j + 1] = item; + } } /// @@ -253,10 +275,12 @@ private static IModuleEnvironment ResolveEnvironment(IServiceCollection serviceC return environment; } - RefuseUnusableEnvironment(serviceCollection); - environment = ModuleEnvironment.CreateDefault(); - serviceCollection.AddSingleton(environment); + + // The descriptor is built directly rather than through AddSingleton. The extension + // method is generic, and instantiating it here was one more thing to compile on a path + // every application walks exactly once. + serviceCollection.Add(new ServiceDescriptor(typeof(IModuleEnvironment), environment)); return environment; } @@ -267,10 +291,10 @@ private static IModuleEnvironment ResolveEnvironment(IServiceCollection serviceC /// /// public static void ApplyDecorators(IServiceCollection serviceCollection, IModuleEnvironment environment) { - // OrderBy is a stable sort, so decorators sharing an order keep their registration order - // rather than nesting arbitrarily. - foreach (var decorator in GetDecorators().OrderBy(decorator => decorator.Order)) { - decorator.RegistryFunc(serviceCollection, environment); + var list = new List(GetDecorators()); + SortByOrder(list); + for (var i = 0; i < list.Count; i++) { + list[i].RegistryFunc(serviceCollection, environment); } } @@ -284,7 +308,7 @@ public static void ApplyDecorators(IServiceCollection serviceCollection, IModule /// public static IReadOnlyList GetDecorators() { lock (SyncLock) { - return Decorators.ToArray(); + return Decorators == null ? Array.Empty() : Decorators.ToArray(); } } @@ -294,20 +318,31 @@ public static IReadOnlyList GetDecorators() { /// /// public static IEnumerable GetModules(params object[] modules) { - List snapshot; lock (SyncLock) { - snapshot = Modules.ToList(); - } + if (Modules == null || Modules.Count == 0) { + return modules; + } - if (modules.Length == 0) { - return snapshot; + return CombineWithAddedModules(Modules, modules); } + } - if (snapshot.Count == 0) { - return modules; - } + /// + /// The AddModule case, split out so that the common path does not carry it. + /// + /// + /// Kept in its own non-inlined method because JIT-ing a method resolves every token in it, and + /// the tokens here pull in System.Linq. Inline, that assembly was loaded during startup for + /// every application, including the overwhelming majority that never call AddModule and + /// return on the line above. + /// + [MethodImpl(MethodImplOptions.NoInlining)] + private static IEnumerable CombineWithAddedModules( + List registered, object[] modules) { - return snapshot.Concat(modules); + var snapshot = registered.ToList(); + + return modules.Length == 0 ? snapshot : snapshot.Concat(modules); } private static void ApplyDecorators(IServiceCollection serviceCollection, IReadOnlyList modules) { @@ -315,13 +350,23 @@ private static void ApplyDecorators(IServiceCollection serviceCollection, IReadO // sorts feature applicators. Applying each module's decorators in turn would let module // discovery order outrank the declared order, which breaks a pipeline assembled from more // than one package. - var decorators = new List(); + List? decorators = null; for (var i = 0; i < modules.Count; i++) { - decorators.AddRange(modules[i].InternalGetDecorators()); + var registrations = modules[i].InternalGetDecorators(); + + // Tested before enumerating. A module with no decorators is the common case, and asking + // it for an enumerator to immediately find it empty built one per module per startup. + if (registrations is ICollection { Count: 0 }) { + continue; + } + + foreach (var registration in registrations) { + (decorators ??= []).Add(registration); + } } - if (decorators.Count > 0) { + if (decorators != null) { // The same environment the registrations were decided against. ApplyServices runs first // and registers one when nothing supplied it, so this finds that instance rather than // building a second answer to "what environment is this" — a decorator gated on @@ -330,8 +375,10 @@ private static void ApplyDecorators(IServiceCollection serviceCollection, IReadO // than to the registered one would be exactly that divergence. var environment = ResolveEnvironment(serviceCollection); - foreach (var decorator in decorators.OrderBy(decorator => decorator.Order)) { - decorator.RegistryFunc(serviceCollection, environment); + SortByOrder(decorators); + + for (var i = 0; i < decorators.Count; i++) { + decorators[i].RegistryFunc(serviceCollection, environment); } } @@ -397,12 +444,8 @@ private static void ApplyServices(IServiceCollection serviceCollection, IReadOnl /// the registration that was ignored got shadowed by the one added in its place — a service /// gated on "Development" quietly took its production branch. /// - private static void RefuseUnusableEnvironment(IServiceCollection serviceCollection) { - for (var i = serviceCollection.Count - 1; i >= 0; i--) { - if (serviceCollection[i].ServiceType != typeof(IModuleEnvironment)) { - continue; - } - + private static void RefuseUnusableEnvironment(ServiceDescriptor descriptor) { + if (descriptor.ServiceType == typeof(IModuleEnvironment)) { throw new InvalidOperationException( "An IModuleEnvironment is registered, but not as a singleton instance, so it cannot " + "be used. The environment decides which services are registered, which happens " + @@ -413,35 +456,52 @@ private static void RefuseUnusableEnvironment(IServiceCollection serviceCollecti } } + /// + /// The usable environment in the collection, or null, refusing an unusable one on the way past. + /// + /// + /// One scan rather than two. The two questions - "is there an environment" and "is it in a form + /// that can be used" - are answered by the same descriptor, and asking them separately walked + /// the collection twice on every AddModules call. + /// + /// The last matching descriptor decides, because that is the one the container would resolve. + /// Anything earlier is shadowed and cannot be what the application meant. + /// private static IModuleEnvironment? FindModuleEnvironment(IServiceCollection serviceCollection) { for (var i = serviceCollection.Count - 1; i >= 0; i--) { var descriptor = serviceCollection[i]; - if (descriptor.ServiceType == typeof(IModuleEnvironment) && - descriptor is { - Lifetime: ServiceLifetime.Singleton, + + if (descriptor.ServiceType != typeof(IModuleEnvironment)) { + continue; + } + + if (descriptor is { + Lifetime: ServiceLifetime.Singleton, ImplementationInstance: IModuleEnvironment environment }) { return environment; } + + RefuseUnusableEnvironment(descriptor); } return null; } private static void ApplyFeatures(IServiceCollection serviceCollection, IReadOnlyList modules) { - var features = new List(); + List? features = null; for (var i = 0; i < modules.Count; i++) { var module = modules[i]; if (module is IDependencyModuleApplicatorProvider provider) { foreach (var featureApplicator in provider.FeatureApplicators()) { - features.Add(featureApplicator); + (features ??= []).Add(featureApplicator); } } } - if (features.Count > 0) { + if (features != null) { features.Sort((x, y) => x.Order.CompareTo(y.Order)); for (var i = 0; i < features.Count; i++) { @@ -464,25 +524,59 @@ private static IReadOnlyList GetAllModules(IDependencyModule[ private static void InternalGetModules(IDependencyModule dependencyModule, List allDependencyModules) { - if (!dependencyModule.LoadModule || - allDependencyModules.Contains(dependencyModule)) { + if (!dependencyModule.LoadModule || + AlreadySeen(allDependencyModules, dependencyModule)) { return; } allDependencyModules.Insert(0, dependencyModule); - foreach (var dependencyObject in dependencyModule.InternalGetModules()) { - if (dependencyObject is IDependencyModuleProvider moduleProvider) { - var dep = moduleProvider.GetModule(); - InternalGetModules(dep, allDependencyModules); - } - else if (dependencyObject is IDependencyModule module) { - InternalGetModules(module, allDependencyModules); + var declared = dependencyModule.InternalGetModules(); + + if (declared is not ICollection { Count: 0 }) { + foreach (var dependencyObject in declared) { + if (dependencyObject is IDependencyModuleProvider moduleProvider) { + var dep = moduleProvider.GetModule(); + InternalGetModules(dep, allDependencyModules); + } + else if (dependencyObject is IDependencyModule module) { + InternalGetModules(module, allDependencyModules); + } } } - - foreach (var module in dependencyModule.GetModules()) { + + var overridden = dependencyModule.GetModules(); + + // Both lists are empty for the overwhelming majority of modules, and both are reached + // through an interface. Testing for an empty collection first avoids building an enumerator + // for each of them on every module of every startup. + if (overridden is ICollection { Count: 0 }) { + return; + } + + foreach (var module in overridden) { InternalGetModules(module, allDependencyModules); } } + + /// + /// Whether this module is already in the list, by the module's own equality. + /// + /// + /// Written out rather than List.Contains. That routes through + /// EqualityComparer<IDependencyModule>.Default, and building the default comparer + /// for an interface is a runtime type-construction step - it showed up as the single most + /// expensive thing in module discovery, to compare a list that usually holds one item. + /// + private static bool AlreadySeen(List modules, IDependencyModule candidate) { + for (var i = 0; i < modules.Count; i++) { + // Argument order matches EqualityComparer.Default, which asks the element rather + // than the candidate, so a hand-written asymmetric Equals behaves as it always did. + if (modules[i].Equals(candidate)) { + return true; + } + } + + return false; + } } \ No newline at end of file diff --git a/src/DependencyModules.Runtime/Interfaces/IDependencyModule.cs b/src/DependencyModules.Runtime/Interfaces/IDependencyModule.cs index e294a89..1cdbff2 100644 --- a/src/DependencyModules.Runtime/Interfaces/IDependencyModule.cs +++ b/src/DependencyModules.Runtime/Interfaces/IDependencyModule.cs @@ -25,7 +25,7 @@ public interface IDependencyModule { /// /// IEnumerable GetModules() { - return ArraySegment.Empty; + return Array.Empty(); } /// @@ -34,7 +34,9 @@ IEnumerable GetModules() { /// [Browsable(false)] IEnumerable InternalGetModules() { - return ArraySegment.Empty; + // Array.Empty() rather than an array of the interface, so the runtime's empty check + // is a plain ICollection test rather than one relying on array covariance. + return Array.Empty(); } /// @@ -77,6 +79,6 @@ void InternalApplyDecorators(IServiceCollection serviceCollection) { } /// [Browsable(false)] IEnumerable InternalGetDecorators() { - return ArraySegment.Empty; + return Array.Empty(); } } \ No newline at end of file diff --git a/src/DependencyModules.Runtime/ModuleEnvironment.cs b/src/DependencyModules.Runtime/ModuleEnvironment.cs index 4a38f48..234dd6e 100644 --- a/src/DependencyModules.Runtime/ModuleEnvironment.cs +++ b/src/DependencyModules.Runtime/ModuleEnvironment.cs @@ -1,5 +1,4 @@ using System.Collections; -using System.Collections.Concurrent; using DependencyModules.Runtime.Interfaces; namespace DependencyModules.Runtime; @@ -29,7 +28,11 @@ public class ModuleEnvironment : IModuleEnvironment, IEnumerable _processValues = new(); + // Allocated on first process read rather than in the constructor. An environment is built on + // every AddModules call and most applications never read a value through it, so constructing a + // ConcurrentDictionary here put its type load and its lock and bucket arrays on every startup + // to serve a cache that stayed empty. + private Dictionary? _processValues; /// /// Creates an environment with a fixed name and an optional set of values, falling back to @@ -104,7 +107,7 @@ public ModuleEnvironment( // Separate from _values rather than written back into it. That dictionary is what the caller // supplied, and GetEnumerator says so — folding process reads into it would have this // environment report values nobody gave it. - return _processValues.GetOrAdd(name, static key => Environment.GetEnvironmentVariable(key)); + return ProcessValueCache.Read(ref _processValues, name); } /// @@ -172,8 +175,45 @@ public ModuleEnvironment( /// public static IModuleEnvironment None { get; } = new EmptyModuleEnvironment(); + /// + /// The process-variable cache both environments read through, allocated on first use. + /// + /// + /// A plain dictionary behind a lock rather than a ConcurrentDictionary. Reads are rare and + /// almost always hits, while the constructor ran on every startup - loading the concurrent + /// collections and allocating its lock and bucket arrays to serve a cache most applications + /// never touch. + /// + private static class ProcessValueCache { + public static string? Read(ref Dictionary? cache, string name) { + var map = Volatile.Read(ref cache); + + if (map != null) { + lock (map) { + if (map.TryGetValue(name, out var cached)) { + return cached; + } + } + } + else { + // Whoever gets there first owns the cache; a loser simply fills the winner's. + map = Interlocked.CompareExchange( + ref cache, new Dictionary(StringComparer.Ordinal), null) + ?? Volatile.Read(ref cache)!; + } + + var value = Environment.GetEnvironmentVariable(name); + + lock (map) { + map[name] = value; + } + + return value; + } + } + private sealed class ProcessModuleEnvironment : IModuleEnvironment { - private readonly ConcurrentDictionary _values = new(); + private Dictionary? _values; // Not cached. It is read once per AddModules call rather than per service, and a fresh // instance is what CreateDefault hands out anyway. @@ -182,8 +222,7 @@ private sealed class ProcessModuleEnvironment : IModuleEnvironment { Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT") ?? "Production"; - public string? Value(string name) => - _values.GetOrAdd(name, static key => Environment.GetEnvironmentVariable(key)); + public string? Value(string name) => ProcessValueCache.Read(ref _values, name); } private sealed class EmptyModuleEnvironment : IModuleEnvironment { diff --git a/src/DependencyModules.SourceGenerator.Impl/DependencyModuleWriter.cs b/src/DependencyModules.SourceGenerator.Impl/DependencyModuleWriter.cs index d17995d..7e36124 100644 --- a/src/DependencyModules.SourceGenerator.Impl/DependencyModuleWriter.cs +++ b/src/DependencyModules.SourceGenerator.Impl/DependencyModuleWriter.cs @@ -28,11 +28,41 @@ public void GenerateSource(SourceProductionContext context, foreach (var entryPointModel in entryPointList) { context.CancellationToken.ThrowIfCancellationRequested(); - - ProcessEntryPoint(context, entryPointModel, configurationModel); + + ProcessEntryPoint( + context, + WithDelegateTarget(entryPointModel, entryPointList), + configurationModel); } } + /// + /// Points an auto-generated module at the declared one carrying the registrations it would + /// otherwise have emitted for itself. + /// + /// + /// The target joins AdditionalModules, so it comes out of InternalGetModules and + /// the runtime loads it exactly as it loads a module named by an attribute. Nothing else about + /// the emitted module changes, and a module with nothing to defer to is returned untouched. + /// + private static ModuleEntryPointModel WithDelegateTarget( + ModuleEntryPointModel entryPointModel, IList entryPointList) { + + var target = EntryModelUtil.DelegateTargetFor(entryPointModel, entryPointList); + + if (target == null) { + return entryPointModel; + } + + var modules = new List(entryPointModel.AdditionalModules); + + if (!modules.Contains(target)) { + modules.Add(target); + } + + return entryPointModel with { AdditionalModules = modules }; + } + private void ProcessEntryPoint( SourceProductionContext context, ModuleEntryPointModel entryPointModel, diff --git a/src/DependencyModules.SourceGenerator.Impl/Utilities/EntryModelUtil.cs b/src/DependencyModules.SourceGenerator.Impl/Utilities/EntryModelUtil.cs index 0355a27..09edd11 100644 --- a/src/DependencyModules.SourceGenerator.Impl/Utilities/EntryModelUtil.cs +++ b/src/DependencyModules.SourceGenerator.Impl/Utilities/EntryModelUtil.cs @@ -6,6 +6,88 @@ namespace DependencyModules.SourceGenerator.Impl.Utilities; public class EntryModelUtil { + /// + /// The declared module an auto-generated ApplicationModule should defer to, or null when + /// it has to carry its own registrations. + /// + /// + /// + /// A module with no realm restriction registers every service in the compilation that is not + /// aimed at some other realm. The auto-generated module is one of those, so when the project + /// also declares one the two produce byte-identical registration bodies — measured at 5,413 + /// bytes of IL each in a 200 service project, where the duplicate was 44% of the assembly and + /// 21% of the ReadyToRun image. It is dead code in every application that does not name + /// ApplicationModule, and code the AOT compiler still has to compile in the ones that do. + /// + /// + /// Deferring rather than dropping keeps AddModule<ApplicationModule>() registering + /// what it always did: the auto module returns the declared one from + /// InternalGetModules, and the runtime loads it. The two register the same set, so what + /// reaches the collection is unchanged. + /// + /// + /// Only a module with no realm restriction is a valid target. An OnlyRealm module takes + /// just the registrations aimed at it, so deferring to one would silently drop everything else. + /// Where several qualify, any of them registers the same set; the name orders them so the + /// choice does not move between builds. + /// + /// + public static ITypeDefinition? DelegateTargetFor( + ModuleEntryPointModel entryPointModel, IEnumerable allEntryPoints) { + + if (!entryPointModel.ModuleFeatures.HasFlag(ModuleEntryPointFeatures.AutoGenerateModule)) { + return null; + } + + ModuleEntryPointModel? target = null; + + foreach (var candidate in allEntryPoints) { + if (candidate.ModuleFeatures.HasFlag(ModuleEntryPointFeatures.AutoGenerateModule) || + candidate.ModuleFeatures.HasFlag(ModuleEntryPointFeatures.OnlyRealm) || + candidate.ModuleFeatures.HasFlag(ModuleEntryPointFeatures.NotPartial)) { + continue; + } + + // A module the caller has to supply arguments for cannot be constructed by the auto + // module, which has nothing to pass. + if (candidate.Parameters.Count > 0) { + continue; + } + + if (target == null || string.Compare(FullName(candidate), FullName(target), StringComparison.Ordinal) < 0) { + target = candidate; + } + } + + return target?.EntryPointType; + } + + /// + /// The entry points that should carry the registrations for a compilation. + /// + /// + /// Every writer that emits registrations, decorations or interceptions into a module filters + /// through this, so an auto-generated module that defers to a declared one is skipped by all of + /// them rather than by whichever ones remembered to. + /// + public static IList RegistrationTargets(IList entryPoints) { + List? filtered = null; + + for (var i = 0; i < entryPoints.Count; i++) { + if (DelegateTargetFor(entryPoints[i], entryPoints) == null) { + filtered?.Add(entryPoints[i]); + continue; + } + + filtered ??= new List(entryPoints.Take(i)); + } + + return filtered ?? entryPoints; + } + + private static string FullName(ModuleEntryPointModel model) => + model.EntryPointType.Namespace + "." + model.EntryPointType.Name; + /// /// Rewrites a generated partial declaration when the module is a record. /// diff --git a/src/DependencyModules.SourceGenerator/Conventions/ConventionGenerator.cs b/src/DependencyModules.SourceGenerator/Conventions/ConventionGenerator.cs index 238589c..94fb9f8 100644 --- a/src/DependencyModules.SourceGenerator/Conventions/ConventionGenerator.cs +++ b/src/DependencyModules.SourceGenerator/Conventions/ConventionGenerator.cs @@ -230,7 +230,11 @@ private void Generate( var claimed = new HashSet(); - foreach (var entryPointModel in entryPointList) { + // An auto-generated module deferring to a declared one is not among these, so its decorations + // and convention registrations are emitted once rather than alongside an identical copy on + // the module it defers to. It can never be a convention module itself - IConventionModule is + // implemented by hand - so nothing here goes unclaimed as a result. + foreach (var entryPointModel in EntryModelUtil.RegistrationTargets(entryPointList)) { context.CancellationToken.ThrowIfCancellationRequested(); var conventionModule = conventionModules.FirstOrDefault( diff --git a/src/DependencyModules.SourceGenerator/InterceptorSourceGenerator.cs b/src/DependencyModules.SourceGenerator/InterceptorSourceGenerator.cs index f8eb480..5eefc73 100644 --- a/src/DependencyModules.SourceGenerator/InterceptorSourceGenerator.cs +++ b/src/DependencyModules.SourceGenerator/InterceptorSourceGenerator.cs @@ -74,7 +74,7 @@ protected override void GenerateSourceOutput( } // One registration file per module, so every wrapper is applied wherever its service is. - foreach (var entryPointModel in entryPointList) { + foreach (var entryPointModel in EntryModelUtil.RegistrationTargets(entryPointList)) { var registrationWriter = new InterceptorRegistrationWriter(); context.AddSource( diff --git a/src/DependencyModules.SourceGenerator/ServiceSourceGenerator.cs b/src/DependencyModules.SourceGenerator/ServiceSourceGenerator.cs index a61de34..8d39c5e 100644 --- a/src/DependencyModules.SourceGenerator/ServiceSourceGenerator.cs +++ b/src/DependencyModules.SourceGenerator/ServiceSourceGenerator.cs @@ -49,7 +49,7 @@ protected override void GenerateSourceOutput(SourceProductionContext context, var (entryPointList, configurationModel) = EntryModelUtil.ConsolidateEntryPointModels(inputData.Left); - foreach (var entryPointModel in entryPointList) { + foreach (var entryPointModel in EntryModelUtil.RegistrationTargets(entryPointList)) { context.CancellationToken.ThrowIfCancellationRequested(); GenerateSourceOutput(context, entryPointModel, configurationModel, serviceModels, logger); } diff --git a/tests/DependencyModules.Tests/GeneratorTests/AutoModuleDelegationTests.cs b/tests/DependencyModules.Tests/GeneratorTests/AutoModuleDelegationTests.cs new file mode 100644 index 0000000..74b35c6 --- /dev/null +++ b/tests/DependencyModules.Tests/GeneratorTests/AutoModuleDelegationTests.cs @@ -0,0 +1,248 @@ +using System.Reflection; +using DependencyModules.Runtime; +using DependencyModules.Runtime.Interfaces; +using DependencyModules.Tests.Infrastructure; +using Microsoft.CodeAnalysis; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace DependencyModules.Tests.GeneratorTests; + +/// +/// A project with a Program.cs gets an ApplicationModule whether or not it declares a +/// module of its own. Both are modules with no realm restriction, so both register every service in +/// the compilation - and the generator used to emit that registration body twice, byte for byte. +/// In a 200 service project the duplicate was 5,413 bytes of IL, 44% of the assembly and 21% of the +/// ReadyToRun image, dead in every application that never names ApplicationModule. +/// +/// It now defers instead: the auto module returns the declared one from InternalGetModules +/// and the runtime loads it. These tests pin both halves - that the duplicate is gone, and that +/// AddModule<ApplicationModule>() still registers exactly what it did before. +/// +public class AutoModuleDelegationTests { + + [Fact] + public void ApplicationModule_DoesNotRepeatTheRegistrationsOfADeclaredModule() { + var result = Run(TopLevelProgramWith( + """ + public interface IThing; + + [SingletonService] + public class Thing : IThing; + + [DependencyModule] + public partial class TestModule; + """)); + + result.AssertNoErrors(); + + Assert.Contains(result.GeneratedSources.Keys, key => key.Contains("TestModule.Dependencies")); + Assert.DoesNotContain(result.GeneratedSources.Keys, key => key.Contains("ApplicationModule.Dependencies")); + } + + /// + /// The class is still generated, and still reachable - only its registrations moved. + /// + [Fact] + public void ApplicationModule_NamesTheModuleItDefersTo() { + var result = Run(TopLevelProgramWith( + """ + public interface IThing; + + [SingletonService] + public class Thing : IThing; + + [DependencyModule] + public partial class TestModule; + """)); + + result.AssertNoErrors(); + Assert.Contains("new global::TestNamespace.TestModule()", result.SourceContaining("ApplicationModule.Module")); + } + + /// + /// Decorations and interceptions travelled with the registrations, so they were duplicated the + /// same way and have to stop being duplicated the same way. + /// + [Fact] + public void ApplicationModule_DoesNotRepeatDecorationsEither() { + var result = Run(TopLevelProgramWith( + """ + public interface IThing; + + [SingletonService] + public class Thing : IThing; + + [Decorator] + public class ThingDecorator(IThing inner) : IThing; + + [DependencyModule] + public partial class TestModule; + """)); + + result.AssertNoErrors(); + + Assert.Contains(result.GeneratedSources.Keys, key => key.Contains("TestModule.Decorators")); + Assert.DoesNotContain(result.GeneratedSources.Keys, key => key.Contains("ApplicationModule.Decorators")); + } + + /// + /// With nothing to defer to, the auto module carries its own registrations exactly as before. + /// + [Fact] + public void ApplicationModule_KeepsItsRegistrationsWhenNoModuleIsDeclared() { + var result = Run(TopLevelProgramWith( + """ + public interface IThing; + + [SingletonService] + public class Thing : IThing; + """)); + + result.AssertNoErrors(); + Assert.Contains(result.GeneratedSources.Keys, key => key.Contains("ApplicationModule.Dependencies")); + } + + /// + /// A realm-restricted module takes only the registrations aimed at it, so deferring to one would + /// drop everything else. The auto module keeps its own registrations in that case. + /// + [Fact] + public void ApplicationModule_KeepsItsRegistrationsWhenTheOnlyModuleIsRealmRestricted() { + var result = Run(TopLevelProgramWith( + """ + public interface IThing; + + [SingletonService] + public class Thing : IThing; + + [DependencyModule(OnlyRealm = true)] + public partial class RealmModule; + """)); + + result.AssertNoErrors(); + Assert.Contains(result.GeneratedSources.Keys, key => key.Contains("ApplicationModule.Dependencies")); + } + + /// + /// The point of the whole exercise: what reaches the service collection is unchanged. + /// + [Fact] + public void ApplicationModule_RegistersTheSameServicesAsTheModuleItDefersTo() { + var assembly = Compile(TopLevelProgramWith( + """ + public interface IThing; + + [SingletonService] + public class Thing : IThing; + + [DependencyModule] + public partial class TestModule; + """)); + + var viaApplicationModule = Apply(assembly, "TestNamespace.ApplicationModule"); + var viaDeclaredModule = Apply(assembly, "TestNamespace.TestModule"); + + var thing = assembly.GetType("TestNamespace.IThing")!; + + Assert.Equal( + Describe(viaDeclaredModule, thing), + Describe(viaApplicationModule, thing)); + + Assert.NotNull(viaApplicationModule.BuildServiceProvider().GetService(thing)); + } + + /// + /// Loading both used to apply every registration twice, because the two modules carried + /// independent copies of it. The auto module now names the declared one, so the runtime's + /// deduplication sees them as one. + /// + [Fact] + public void LoadingBothModules_RegistersEachServiceOnce() { + var assembly = Compile(TopLevelProgramWith( + """ + public interface IThing; + + [SingletonService] + public class Thing : IThing; + + [DependencyModule] + public partial class TestModule; + """)); + + var both = new ServiceCollection(); + both.AddModules(Module(assembly, "TestNamespace.ApplicationModule"), Module(assembly, "TestNamespace.TestModule")); + + var thing = assembly.GetType("TestNamespace.IThing")!; + + Assert.Single(both, descriptor => descriptor.ServiceType == thing); + } + + private static string Describe(IServiceCollection services, Type serviceType) => + string.Join( + ", ", + services + .Where(descriptor => descriptor.ServiceType == serviceType) + .Select(descriptor => $"{descriptor.Lifetime}:{descriptor.ImplementationType?.FullName}")); + + private static IServiceCollection Apply(Assembly assembly, string moduleName) { + var services = new ServiceCollection(); + + services.AddModules(Module(assembly, moduleName)); + + return services; + } + + private static IDependencyModule Module(Assembly assembly, string moduleName) { + var type = assembly.GetType(moduleName) + ?? throw new InvalidOperationException( + $"No type '{moduleName}'. Present: " + + string.Join(", ", assembly.GetTypes().Select(t => t.FullName))); + + return (IDependencyModule)Activator.CreateInstance(type)!; + } + + private static Assembly Compile(IReadOnlyDictionary sources) { + var result = GeneratorTestHarness.Run( + sources, + null, + OutputKind.ConsoleApplication, + assemblyName: "AutoModuleDelegation" + Interlocked.Increment(ref _counter)); + + result.AssertNoErrors(); + + using var stream = new MemoryStream(); + var emitted = result.Compilation.Emit(stream); + + Assert.True( + emitted.Success, + string.Join( + Environment.NewLine, + emitted.Diagnostics + .Where(diagnostic => diagnostic.Severity == DiagnosticSeverity.Error) + .Select(diagnostic => $" {diagnostic.Id} {diagnostic.GetMessage()}"))); + + return Assembly.Load(stream.ToArray()); + } + + private static int _counter; + + private static GeneratorResult Run(IReadOnlyDictionary sources) => + GeneratorTestHarness.Run(sources, null, OutputKind.ConsoleApplication); + + private static Dictionary TopLevelProgramWith(string services) => + new() { + ["Program.cs"] = + """ + System.Console.WriteLine("hello"); + """, + ["Services.cs"] = + $$""" + using DependencyModules.Runtime.Attributes; + + namespace TestNamespace; + + {{services}} + """ + }; +} diff --git a/tests/DependencyModules.Tests/RuntimeTests/DependencyRegistryTests.cs b/tests/DependencyModules.Tests/RuntimeTests/DependencyRegistryTests.cs index cafad8b..04350fe 100644 --- a/tests/DependencyModules.Tests/RuntimeTests/DependencyRegistryTests.cs +++ b/tests/DependencyModules.Tests/RuntimeTests/DependencyRegistryTests.cs @@ -285,4 +285,77 @@ private class ModuleConcurrencyMarker; private class StubModule : IDependencyModule { public void PopulateServiceCollection(IServiceCollection serviceCollection) { } } + + /// + /// The single-argument overloads used to refuse any collection with an IModuleEnvironment in it, + /// including one registered in the only form they accept, because the guard ran before the + /// lookup. Every test above passes a collection with no environment at all, which is why it went + /// unnoticed. + /// + [Fact] + public void ApplyServices_UsesAnEnvironmentAlreadyInTheCollection() { + DependencyRegistry.Add( + (services, environment) => { + if (environment.EnvironmentName == "Development") { + services.AddSingleton(); + } + }); + + var collection = new ServiceCollection(); + collection.AddSingleton(new StubEnvironment("Development")); + + DependencyRegistry.ApplyServices(collection); + + Assert.Contains(collection, descriptor => descriptor.ImplementationType == typeof(OtherThing)); + } + + private class SuppliedEnvironmentMarker; + + [Fact] + public void ApplyDecorators_UsesAnEnvironmentAlreadyInTheCollection() { + var seen = ""; + + DependencyRegistry.AddDecorator( + (EnvironmentRegistryFunc)((_, environment) => seen = environment.EnvironmentName)); + + var collection = new ServiceCollection(); + collection.AddSingleton(new StubEnvironment("Staging")); + + DependencyRegistry.ApplyDecorators(collection); + + Assert.Equal("Staging", seen); + } + + private class SuppliedDecoratorEnvironmentMarker; + + /// + /// The guard still has to fire. An environment the container would build rather than hand back + /// cannot decide registrations, because there is no provider yet to build it with. + /// + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ApplyServices_RefusesAnEnvironmentItCannotUse(bool registeredByType) { + var collection = new ServiceCollection(); + + if (registeredByType) { + collection.AddSingleton(); + } + else { + collection.AddSingleton(_ => new StubEnvironment("Development")); + } + + Assert.Throws( + () => DependencyRegistry.ApplyServices(collection)); + } + + private class RefusedEnvironmentMarker; + + private class StubEnvironment(string name) : IModuleEnvironment { + public string EnvironmentName => name; + + public string? Value(string valueName) => null; + } + + private class DefaultStubEnvironment() : StubEnvironment("Development"); } diff --git a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.RuntimeApi.verified.txt b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.RuntimeApi.verified.txt index 6a069d6..8bb382a 100644 --- a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.RuntimeApi.verified.txt +++ b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.RuntimeApi.verified.txt @@ -190,7 +190,7 @@ namespace DependencyModules.Runtime.Helpers public static void Decorate(Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Type decoratorIdentity, System.Func decoratorFactory) where TService : class { } } - public readonly struct DecoratorRegistration + public sealed class DecoratorRegistration { public DecoratorRegistration(int order, DependencyModules.Runtime.Helpers.EnvironmentRegistryFunc registryFunc) { } public DecoratorRegistration(int order, DependencyModules.Runtime.Helpers.RegistryFunc registryFunc) { } diff --git a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt index afcf16d..0ad69bc 100644 --- a/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt +++ b/tests/DependencyModules.Tests/Snapshots/PublicApiTests.SourceGeneratorApi.verified.txt @@ -1586,8 +1586,10 @@ namespace DependencyModules.SourceGenerator.Impl.Utilities public static System.ValueTuple, DependencyModules.SourceGenerator.Impl.Models.DependencyModuleConfigurationModel> ConsolidateEntryPointModels([System.Runtime.CompilerServices.TupleElementNames(new string[] { "Left", "Right"})] System.Collections.Immutable.ImmutableArray> entryPointList) { } + public static CSharpAuthor.ITypeDefinition? DelegateTargetFor(DependencyModules.SourceGenerator.Impl.Models.ModuleEntryPointModel entryPointModel, System.Collections.Generic.IEnumerable allEntryPoints) { } public static DependencyModules.SourceGenerator.Impl.Models.ModuleEntryPointModel EnsureNamespace(DependencyModules.SourceGenerator.Impl.Models.ModuleEntryPointModel entryPointModel, DependencyModules.SourceGenerator.Impl.Models.DependencyModuleConfigurationModel configurationModel) { } public static string GenerateFileName(DependencyModules.SourceGenerator.Impl.Models.ModuleEntryPointModel entryPointModel, string uniquePortion) { } + public static System.Collections.Generic.IList RegistrationTargets(System.Collections.Generic.IList entryPoints) { } } public static class EnvironmentConditionUtility {