Skip to content
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.

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

Performance optimization: focusing on allocation reduction #2158

Merged
merged 10 commits into from Oct 4, 2023
19 changes: 13 additions & 6 deletions src/FluentValidation/AbstractValidator.cs
Expand Up @@ -164,7 +164,7 @@ public abstract class AbstractValidator<T> : IValidator<T>, IEnumerable<IValidat
/// <param name="instance">The object to validate</param>
/// <returns>A ValidationResult object containing any validation failures</returns>
public ValidationResult Validate(T instance)
=> Validate(new ValidationContext<T>(instance, new PropertyChain(), ValidatorOptions.Global.ValidatorSelectors.DefaultValidatorSelectorFactory()));
=> Validate(new ValidationContext<T>(instance, null, ValidatorOptions.Global.ValidatorSelectors.DefaultValidatorSelectorFactory()));

/// <summary>
/// Validates the specified instance asynchronously
Expand All @@ -173,7 +173,7 @@ public ValidationResult Validate(T instance)
/// <param name="cancellation">Cancellation token</param>
/// <returns>A ValidationResult object containing any validation failures</returns>
public Task<ValidationResult> ValidateAsync(T instance, CancellationToken cancellation = new())
=> ValidateAsync(new ValidationContext<T>(instance, new PropertyChain(), ValidatorOptions.Global.ValidatorSelectors.DefaultValidatorSelectorFactory()), cancellation);
=> ValidateAsync(new ValidationContext<T>(instance, null, ValidatorOptions.Global.ValidatorSelectors.DefaultValidatorSelectorFactory()), cancellation);

/// <summary>
/// Validates the specified instance.
Expand Down Expand Up @@ -236,9 +236,12 @@ ValueTask<ValidationResult> completedValueTask
EnsureInstanceNotNull(context.InstanceToValidate);
#pragma warning restore CS0618

foreach (var rule in Rules) {
JeremySkinner marked this conversation as resolved.
Show resolved Hide resolved
int count = Rules.Count;

// Performance: Use for loop rather than foreach to reduce allocations.
for (int i = 0; i < count; i++) {
cancellation.ThrowIfCancellationRequested();
await rule.ValidateAsync(context, useAsync, cancellation);
await Rules[i].ValidateAsync(context, useAsync, cancellation);

if (ClassLevelCascadeMode == CascadeMode.Stop && result.Errors.Count > 0) {
// Bail out if we're "failing-fast".
Expand All @@ -259,8 +262,12 @@ ValueTask<ValidationResult> completedValueTask
}

private void SetExecutedRuleSets(ValidationResult result, ValidationContext<T> context) {
var executed = context.RootContextData.GetOrAdd("_FV_RuleSetsExecuted", () => new HashSet<string>{RulesetValidatorSelector.DefaultRuleSetName});
result.RuleSetsExecuted = executed.ToArray();
if (context.RootContextData.TryGetValue("_FV_RuleSetsExecuted", out var obj) && obj is HashSet<string> set) {
result.RuleSetsExecuted = set.ToArray();
}
else {
result.RuleSetsExecuted = RulesetValidatorSelector.DefaultRuleSetNameInArray;
}
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/FluentValidation/Internal/CollectionPropertyRule.cs
Expand Up @@ -166,7 +166,7 @@ public CollectionPropertyRule(MemberInfo member, Func<T, IEnumerable<TElement>>
var valueToValidate = element;
var propertyPath = context.PropertyChain.ToString();
var totalFailuresInner = context.Failures.Count;
context.InitializeForPropertyValidator(propertyPath, GetDisplayName, PropertyName);
context.InitializeForPropertyValidator(propertyPath, _displayNameFunc, PropertyName);

foreach (var component in filteredValidators) {
context.MessageFormatter.Reset();
Expand Down
2 changes: 1 addition & 1 deletion src/FluentValidation/Internal/PropertyChain.cs
Expand Up @@ -163,4 +163,4 @@ public string BuildPropertyName(string propertyName)
/// Number of member names in the chain
/// </summary>
public int Count => _memberNames.Count;
}
}
18 changes: 13 additions & 5 deletions src/FluentValidation/Internal/PropertyRule.cs
Expand Up @@ -115,10 +115,13 @@ TProperty PropertyFunc(T instance)
}
}

bool first = true;
TProperty propValue = default(TProperty);

var cascade = CascadeMode;
var accessor = new Lazy<TProperty>(() => PropertyFunc(context.InstanceToValidate), LazyThreadSafetyMode.None);
var totalFailures = context.Failures.Count;
context.InitializeForPropertyValidator(propertyPath, GetDisplayName, PropertyName);

context.InitializeForPropertyValidator(propertyPath, _displayNameFunc, PropertyName);

// Invoke each validator and collect its results.
foreach (var component in Components) {
Expand All @@ -140,11 +143,16 @@ TProperty PropertyFunc(T instance)
}
}

bool valid = await component.ValidateAsync(context, accessor.Value, useAsync, cancellation);
if (first) {
first = false;
propValue = PropertyFunc(context.InstanceToValidate);
}

bool valid = await component.ValidateAsync(context, propValue, useAsync, cancellation);

if (!valid) {
PrepareMessageFormatterForValidationError(context, accessor.Value);
var failure = CreateValidationError(context, accessor.Value, component);
PrepareMessageFormatterForValidationError(context, propValue);
var failure = CreateValidationError(context, propValue, component);
context.Failures.Add(failure);
}

Expand Down
5 changes: 5 additions & 0 deletions src/FluentValidation/Internal/RuleBase.cs
Expand Up @@ -38,6 +38,8 @@ internal abstract class RuleBase<T, TProperty, TValue> : IValidationRule<T, TVal
private string _displayName;
private Func<ValidationContext<T>, string> _displayNameFactory;

protected readonly Func<ValidationContext<T>, string> _displayNameFunc;

public List<RuleComponent<T, TValue>> Components => _components;

/// <inheritdoc />
Expand Down Expand Up @@ -140,6 +142,9 @@ internal abstract class RuleBase<T, TProperty, TValue> : IValidationRule<T, TVal
var containerType = typeof(T);
PropertyName = ValidatorOptions.Global.PropertyNameResolver(containerType, member, expression);
_displayNameFactory = context => ValidatorOptions.Global.DisplayNameResolver(containerType, member, expression);

// Performance: Cache the display name function to reduce allocations.
_displayNameFunc = GetDisplayName;
}

public void AddValidator(IPropertyValidator<T, TValue> validator) {
Expand Down
1 change: 1 addition & 0 deletions src/FluentValidation/Internal/RulesetValidatorSelector.cs
Expand Up @@ -13,6 +13,7 @@ public class RulesetValidatorSelector : IValidatorSelector {
readonly IEnumerable<string> _rulesetsToExecute;
public const string DefaultRuleSetName = "default";
public const string WildcardRuleSetName = "*";
internal static readonly string[] DefaultRuleSetNameInArray = new string[] { DefaultRuleSetName };

/// <summary>
/// Rule sets
Expand Down
2 changes: 2 additions & 0 deletions src/FluentValidation/Internal/TrackingCollection.cs
Expand Up @@ -40,6 +40,8 @@ internal class TrackingCollection<T> : IEnumerable<T> {

public int Count => _innerCollection.Count;

public T this[int index] => _innerCollection[index];

public void Remove(T item) {
_innerCollection.Remove(item);
}
Expand Down
21 changes: 14 additions & 7 deletions src/FluentValidation/Validators/EmptyValidator.cs
Expand Up @@ -30,13 +30,20 @@ public class EmptyValidator<T,TProperty> : PropertyValidator<T,TProperty> {
public override string Name => "EmptyValidator";

public override bool IsValid(ValidationContext<T> context, TProperty value) {
switch (value) {
case null:
case string s when string.IsNullOrWhiteSpace(s):
case ICollection {Count: 0}:
case Array {Length: 0}:
case IEnumerable e when !e.GetEnumerator().MoveNext():
return true;
if (value == null) {
return true;
}

if (value is string s) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@JeremySkinner @ProfFrugal

Don't these three also change the behavior of the switch expression above.

Line 35 says: "When the value is a string and is empty, return true, otherwise return the EqualityComparer on line 42/49"

This now says: "When the value is a string, return if the string is empty"

The same for the other 2 ICollection and IEnumerable

To maintain same behavior I suggest

if (value is string s && string.IsNullOrWhiteSpace(s))
   return true;
\\etc

Copy link
Member

Choose a reason for hiding this comment

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

@billrob yes you're right - I didn't catch that. I've pushed out an 11.8.1 release with a fix.

return string.IsNullOrWhiteSpace(s);
}

if (value is ICollection col) {
return col.Count == 0;
}

if (value is IEnumerable e) {
return !e.GetEnumerator().MoveNext();
}

return EqualityComparer<TProperty>.Default.Equals(value, default);
Expand Down
21 changes: 14 additions & 7 deletions src/FluentValidation/Validators/NotEmptyValidator.cs
Expand Up @@ -29,13 +29,20 @@ public class NotEmptyValidator<T,TProperty> : PropertyValidator<T, TProperty>, I
public override string Name => "NotEmptyValidator";

public override bool IsValid(ValidationContext<T> context, TProperty value) {
switch (value) {
case null:
case string s when string.IsNullOrWhiteSpace(s):
case ICollection {Count: 0}:
case Array {Length: 0}:
case IEnumerable e when !e.GetEnumerator().MoveNext():
return false;
if (value == null) {
return false;
}

if (value is string s) {
return !string.IsNullOrWhiteSpace(s);
}

if (value is ICollection col) {
return col.Count > 0;
}

if (value is IEnumerable e) {
return e.GetEnumerator().MoveNext();
}

return !EqualityComparer<TProperty>.Default.Equals(value, default);
Expand Down