Skip to content

Materialize trigger collections to eliminate ConcatIterator CPU waste #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PhenX wants to merge 1 commit into
base: master
Choose a base branch
from
+54 −48
Open

Materialize trigger collections to eliminate ConcatIterator CPU waste #215

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 54 additions & 48 deletions src/EntityFrameworkCore.Triggered/Infrastructure/Internal/TriggersOptionExtension.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public class TriggersOptionExtension : IDbContextOptionsExtension
sealed class ExtensionInfo : DbContextOptionsExtensionInfo
{
private string? _logFragment;
private int? _serviceProviderHashCode;
public ExtensionInfo(IDbContextOptionsExtension extension) : base(extension)
{
}
Expand Down Expand Up @@ -44,14 +45,19 @@ public override void PopulateDebugInfo(IDictionary<string, string> debugInfo)
throw new ArgumentNullException(nameof(debugInfo));
}

debugInfo["Triggers:TriggersCount"] = (Extension._triggers?.Count() ?? 0).ToString();
debugInfo["Triggers:TriggerTypesCount"] = (Extension._triggerTypes?.Count() ?? 0).ToString();
debugInfo["Triggers:TriggersCount"] = (Extension._triggers?.Count ?? 0).ToString();
debugInfo["Triggers:TriggerTypesCount"] = (Extension._triggerTypes?.Count ?? 0).ToString();
debugInfo["Triggers:MaxCascadeCycles"] = Extension._maxCascadeCycles.ToString();
debugInfo["Triggers:CascadeBehavior"] = Extension._cascadeBehavior.ToString();
}

public override int GetServiceProviderHashCode()
{
if (_serviceProviderHashCode.HasValue)
{
return _serviceProviderHashCode.Value;
}

var hashCode = new HashCode();

if (Extension._triggers != null)
Expand All @@ -78,28 +84,56 @@ public override int GetServiceProviderHashCode()
hashCode.Add(Extension._serviceProviderTransform);
}

return hashCode.ToHashCode();
_serviceProviderHashCode = hashCode.ToHashCode();
return _serviceProviderHashCode.Value;
}

public override bool ShouldUseSameServiceProvider(DbContextOptionsExtensionInfo other)
=> other is ExtensionInfo otherInfo
&& Enumerable.SequenceEqual(Extension._triggers ?? Enumerable.Empty<ValueTuple<object, ServiceLifetime>>(), otherInfo.Extension._triggers ?? Enumerable.Empty<ValueTuple<object, ServiceLifetime>>())
&& Enumerable.SequenceEqual(Extension._triggerTypes ?? Enumerable.Empty<Type>(), otherInfo.Extension._triggerTypes ?? Enumerable.Empty<Type>())
&& Extension._maxCascadeCycles == otherInfo.Extension._maxCascadeCycles
&& Extension._cascadeBehavior == otherInfo.Extension._cascadeBehavior
&& Extension._serviceProviderTransform == otherInfo.Extension._serviceProviderTransform;
{
if (other is not ExtensionInfo otherInfo)
{
return false;
}

// Check cheap scalar comparisons first
if (Extension._maxCascadeCycles != otherInfo.Extension._maxCascadeCycles
|| Extension._cascadeBehavior != otherInfo.Extension._cascadeBehavior
|| Extension._serviceProviderTransform != otherInfo.Extension._serviceProviderTransform)
{
return false;
}

// Check list counts before doing full sequence comparison
var triggersCount = Extension._triggers?.Count ?? 0;
var otherTriggersCount = otherInfo.Extension._triggers?.Count ?? 0;
if (triggersCount != otherTriggersCount)
{
return false;
}

var triggerTypesCount = Extension._triggerTypes?.Count ?? 0;
var otherTriggerTypesCount = otherInfo.Extension._triggerTypes?.Count ?? 0;
if (triggerTypesCount != otherTriggerTypesCount)
{
return false;
}

// Full sequence comparison only when counts match
return Enumerable.SequenceEqual(Extension._triggers ?? Enumerable.Empty<ValueTuple<object, ServiceLifetime>>(), otherInfo.Extension._triggers ?? Enumerable.Empty<ValueTuple<object, ServiceLifetime>>())
&& Enumerable.SequenceEqual(Extension._triggerTypes ?? Enumerable.Empty<Type>(), otherInfo.Extension._triggerTypes ?? Enumerable.Empty<Type>());
}
}

private ExtensionInfo? _info;
private IEnumerable<(object typeOrInstance, ServiceLifetime lifetime)>? _triggers;
private IEnumerable<Type> _triggerTypes;
private List<(object typeOrInstance, ServiceLifetime lifetime)>? _triggers;
private List<Type> _triggerTypes;
Comment on lines +128 to +129
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
private int _maxCascadeCycles = 100;
private CascadeBehavior _cascadeBehavior = CascadeBehavior.EntityAndType;
private Func<IServiceProvider, IServiceProvider>? _serviceProviderTransform;

public TriggersOptionExtension()
{
_triggerTypes = new[] {
_triggerTypes = new List<Type> {
typeof(IBeforeSaveTrigger<>),
typeof(IBeforeSaveAsyncTrigger<>),
typeof(IAfterSaveTrigger<>),
Expand All @@ -125,10 +159,10 @@ public TriggersOptionExtension(TriggersOptionExtension copyFrom)
{
if (copyFrom._triggers != null)
{
_triggers = copyFrom._triggers;
_triggers = new List<(object typeOrInstance, ServiceLifetime lifetime)>(copyFrom._triggers);
}

_triggerTypes = copyFrom._triggerTypes;
_triggerTypes = new List<Type>(copyFrom._triggerTypes);
_maxCascadeCycles = copyFrom._maxCascadeCycles;
_cascadeBehavior = copyFrom._cascadeBehavior;
_serviceProviderTransform = copyFrom._serviceProviderTransform;
Expand Down Expand Up @@ -263,17 +297,8 @@ public TriggersOptionExtension WithAdditionalTrigger(Type triggerType, ServiceLi
}

var clone = Clone();
var triggerEnumerable = Enumerable.Repeat(((object)triggerType, lifetime), 1);

if (clone._triggers == null)
{
clone._triggers = triggerEnumerable;
}
else
{
clone._triggers = clone._triggers.Concat(triggerEnumerable);
}

clone._triggers ??= new List<(object typeOrInstance, ServiceLifetime lifetime)>();
clone._triggers.Add(((object)triggerType, lifetime));

return clone;
}
Expand All @@ -291,17 +316,8 @@ public TriggersOptionExtension WithAdditionalTrigger(object instance)
}

var clone = Clone();
var triggersEnumerable = Enumerable.Repeat((instance, ServiceLifetime.Singleton), 1);

if (clone._triggers == null)
{
clone._triggers = triggersEnumerable;
}
else
{
clone._triggers = clone._triggers.Concat(triggersEnumerable);
}

clone._triggers ??= new List<(object typeOrInstance, ServiceLifetime lifetime)>();
clone._triggers.Add((instance, ServiceLifetime.Singleton));

return clone;
}
Expand All @@ -313,19 +329,9 @@ public TriggersOptionExtension WithAdditionalTriggerType(Type triggerType)
throw new ArgumentNullException(nameof(triggerType));
}


var clone = Clone();
var triggerTypesEnumerable = Enumerable.Repeat(triggerType, 1);

if (clone._triggerTypes == null)
{
clone._triggerTypes = triggerTypesEnumerable;
}
else
{
clone._triggerTypes = clone._triggerTypes.Concat(triggerTypesEnumerable);
}

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

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_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 uses AI. Check for mistakes.
clone._triggerTypes.Add(triggerType);

return clone;
}
Expand Down
Loading