Materialize trigger collections to eliminate ConcatIterator CPU waste by PhenX · Pull Request #215 · EFNext/EntityFrameworkCore.Triggered

PhenX · 2026-03-21T20:26:12Z

…r CPU waste

Replace IEnumerable fields with List to avoid deeply nested ConcatIterator chains from repeated .Concat() calls. Each WithAdditionalTrigger call now uses List.Add() instead. Also cache the service provider hash code, use O(1) List.Count property instead of LINQ .Count(), and add count-based fast paths in ShouldUseSameServiceProvider to short-circuit before SequenceEqual.

(cherry picked from commit 0cc6aa1)

Benchmarks

Before:

Runtime:

Setup:

After:

Runtime:

Setup:

After, v2:

Runtime:

Setup:

…r CPU waste Replace IEnumerable<T> fields with List<T> to avoid deeply nested ConcatIterator chains from repeated .Concat() calls. Each WithAdditionalTrigger call now uses List.Add() instead. Also cache the service provider hash code, use O(1) List.Count property instead of LINQ .Count(), and add count-based fast paths in ShouldUseSameServiceProvider to short-circuit before SequenceEqual. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> (cherry picked from commit 0cc6aa1)

Copilot

Pull request overview

This PR optimizes TriggersOptionExtension to avoid CPU overhead from repeatedly chaining IEnumerable.Concat() (creating deep ConcatIterator trees) by materializing trigger collections into List<T>, and adds a couple of small fast paths around service-provider caching comparisons.

Changes:

Replace trigger registries from IEnumerable<T> to List<T> and switch WithAdditional* methods to List.Add().
Cache GetServiceProviderHashCode() result and use List.Count / count-based short-circuiting in ShouldUseSameServiceProvider().
Avoid LINQ .Count() in debug info population.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/EntityFrameworkCore.Triggered/Infrastructure/Internal/TriggersOptionExtension.cs

Copilot · 2026-03-21T20:29:21Z

src/EntityFrameworkCore.Triggered/Infrastructure/Internal/TriggersOptionExtension.cs

+        private List<(object typeOrInstance, ServiceLifetime lifetime)>? _triggers;
+        private List<Type> _triggerTypes;


Changing _triggers to List<...> means the public Triggers property now returns a reference to a mutable internal collection (it’s exposed as IEnumerable, but consumers can still cast back to List and mutate). That can invalidate the new GetServiceProviderHashCode() cache and break the expectation that option extensions are effectively immutable. Consider returning a read-only wrapper (AsReadOnly()), an IReadOnlyList, or otherwise preventing external mutation.

Copilot · 2026-03-21T20:29:21Z

src/EntityFrameworkCore.Triggered/Infrastructure/Internal/TriggersOptionExtension.cs

-                clone._triggerTypes = clone._triggerTypes.Concat(triggerTypesEnumerable);
-            }
-
+            clone._triggerTypes ??= new List<Type>();


_triggerTypes is non-nullable and always initialized in constructors, so clone._triggerTypes ??= new List<Type>(); is redundant and suggests the field can be null. Either remove the null-coalescing assignment here, or (if null is a valid state) make _triggerTypes nullable consistently and handle that throughout.

Suggested change

clone._triggerTypes ??= new List<Type>();

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-22T10:46:10Z

src/EntityFrameworkCore.Triggered/Infrastructure/Internal/TriggersOptionExtension.cs

            if (!TypeIsValidTrigger(triggerType))
            {
                throw new ArgumentException("A trigger needs to implement either or both IBeforeSaveChangeTrigger or IAfterSaveChangeTriggerHandler", nameof(triggerType));
            }


The ArgumentException message thrown for invalid triggers references IBeforeSaveChangeTrigger/IAfterSaveChangeTriggerHandler, which don't appear to exist in this codebase and don't match the current lifecycle interface names. Since TypeIsValidTrigger is now stricter, this message is more likely to surface—please update it to mention the actual supported trigger interfaces (or a generic “known trigger interface” wording).

Copilot · 2026-03-22T10:46:10Z

src/EntityFrameworkCore.Triggered.Extensions/TriggersContextOptionsBuilderExtensions.cs

+                // Only register types that actually implement a known trigger interface
+                if (TriggerTypeHelper.GetTriggerInterfaces(assemblyType).Length > 0)
+                {
+                    builder.AddTrigger(assemblyType, lifetime);
+                }


AddAssemblyTriggers now filters types using TriggerTypeHelper.GetTriggerInterfaces(), which is a hard-coded list of “known” lifecycle interfaces. This means triggers for lifecycle interfaces added via AddTriggerType (e.g., transaction trigger interfaces from UseTransactionTriggers) will be silently skipped during assembly scanning, even though they’re valid trigger types once configured.

Suggested change

// Only register types that actually implement a known trigger interface

if (TriggerTypeHelper.GetTriggerInterfaces(assemblyType).Length > 0)

{

builder.AddTrigger(assemblyType, lifetime);

}

builder.AddTrigger(assemblyType, lifetime);

Copilot · 2026-03-22T10:46:10Z

src/EntityFrameworkCore.Triggered.Extensions/TriggerTypeHelper.cs

+        // Open generic trigger interfaces — add new generic lifecycle interfaces here
+        private readonly static HashSet<Type> _genericTriggerTypes = new HashSet<Type>
+        {
+            typeof(IBeforeSaveTrigger<>),
+            typeof(IBeforeSaveAsyncTrigger<>),
+            typeof(IAfterSaveTrigger<>),
+            typeof(IAfterSaveAsyncTrigger<>),
+            typeof(IAfterSaveFailedTrigger<>),
+            typeof(IAfterSaveFailedAsyncTrigger<>),
+        };
+
+        // Non-generic lifecycle interfaces — add new non-generic lifecycle interfaces here
+        private readonly static HashSet<Type> _nonGenericTriggerTypes = new HashSet<Type>
+        {
+            typeof(IBeforeSaveStartingTrigger),
+            typeof(IBeforeSaveStartingAsyncTrigger),
+            typeof(IBeforeSaveCompletedTrigger),
+            typeof(IBeforeSaveCompletedAsyncTrigger),
+            typeof(IAfterSaveFailedStartingTrigger),
+            typeof(IAfterSaveFailedStartingAsyncTrigger),
+            typeof(IAfterSaveFailedCompletedTrigger),
+            typeof(IAfterSaveFailedCompletedAsyncTrigger),
+            typeof(IAfterSaveStartingTrigger),
+            typeof(IAfterSaveStartingAsyncTrigger),
+            typeof(IAfterSaveCompletedTrigger),
+            typeof(IAfterSaveCompletedAsyncTrigger),
+        };


TriggerTypeHelper’s interface sets are hard-coded to the base save-change lifecycles. If a consumer adds additional lifecycle interfaces via options (e.g., EntityFrameworkCore.Triggered.Transactions lifecycles or other custom lifecycles), GetTriggerInterfaces will return an empty array and assembly scanning helpers will not register those triggers. Consider making the “known trigger types” list extensible (e.g., allow registering additional interface types) or avoid using this helper for options-builder scanning where the valid trigger-type list is configurable.

Copilot · 2026-03-22T10:46:10Z

src/EntityFrameworkCore.Triggered/Infrastructure/Internal/TriggersOptionExtension.cs

            else if (_triggerTypes != null)
            {
-                return _triggerTypes.Any(triggerType => TypeHelpers.FindGenericInterfaces(type, triggerType) != null);
+                return _triggerTypes.Any(triggerType => TypeHelpers.FindGenericInterfaces(type, triggerType).Any());


TypeIsValidTrigger now uses TypeHelpers.FindGenericInterfaces(...) for entries in _triggerTypes, but FindGenericInterfaces only matches generic interfaces. This makes any non-generic lifecycle trigger interfaces (e.g., IBeforeSaveStartingTrigger, IAfterSaveCompletedTrigger, etc.) fail validation and prevents registering those triggers via WithAdditionalTrigger/AddTrigger.

Suggested change

return _triggerTypes.Any(triggerType => TypeHelpers.FindGenericInterfaces(type, triggerType).Any());

return _triggerTypes.Any(triggerType =>

triggerType != null &&

(

triggerType.IsGenericTypeDefinition

? TypeHelpers.FindGenericInterfaces(type, triggerType).Any()

: triggerType.IsAssignableFrom(type)

));

PhenX requested a review from Copilot March 21, 2026 20:26

Copilot started reviewing on behalf of PhenX March 21, 2026 20:26 View session

PhenX mentioned this pull request Mar 21, 2026

fix(perf): Eliminate ConcatIterator CPU waste in DI resolution #213

Closed

2 tasks

Copilot AI reviewed Mar 21, 2026

View reviewed changes

fabien.menager added 2 commits March 22, 2026 09:14

Improve triggers resolution performance

d777400

Improve tigger resolution

c6b323e

PhenX requested a review from Copilot March 22, 2026 10:39

Copilot started reviewing on behalf of PhenX March 22, 2026 10:39 View session

Copilot AI reviewed Mar 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Materialize trigger collections to eliminate ConcatIterator CPU waste#215

Materialize trigger collections to eliminate ConcatIterator CPU waste#215
PhenX wants to merge 3 commits intomasterfrom
fix/concat-iterator-perf

PhenX commented Mar 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Mar 21, 2026

Uh oh!

Copilot AI Mar 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		private List<(object typeOrInstance, ServiceLifetime lifetime)>? _triggers;
		private List<Type> _triggerTypes;

-                return _triggerTypes.Any(triggerType => TypeHelpers.FindGenericInterfaces(type, triggerType).Any());
+                return _triggerTypes.Any(triggerType =>
+                    triggerType != null &&
+                    (
+                        triggerType.IsGenericTypeDefinition
+                            ? TypeHelpers.FindGenericInterfaces(type, triggerType).Any()
+                            : triggerType.IsAssignableFrom(type)
+                    ));

Conversation

PhenX commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PhenX commented Mar 21, 2026 •

edited

Loading