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

Improve performance by removing sync-over-async by generating sync methods using Zomp.SyncMethodGenerator #2136

Open
wants to merge 4 commits into
base: 12.x-dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Expand Up @@ -31,7 +31,7 @@ jobs:
with:
dotnet-version: |
8.0.100
7.0.100
7.0.x
6.0.x

- name: Build and Test
Expand Down
26 changes: 6 additions & 20 deletions src/FluentValidation/AbstractValidator.cs
Expand Up @@ -33,7 +33,7 @@ namespace FluentValidation;
/// Base class for object validators.
/// </summary>
/// <typeparam name="T">The type of the object being validated</typeparam>
public abstract class AbstractValidator<T> : IValidator<T>, IEnumerable<IValidationRule> {
public abstract partial class AbstractValidator<T> : IValidator<T>, IEnumerable<IValidationRule> {
internal TrackingCollection<IValidationRuleInternal<T>> Rules { get; } = new();
private Func<CascadeMode> _classLevelCascadeMode = () => ValidatorOptions.Global.DefaultClassLevelCascadeMode;
private Func<CascadeMode> _ruleLevelCascadeMode = () => ValidatorOptions.Global.DefaultRuleLevelCascadeMode;
Expand Down Expand Up @@ -115,23 +115,8 @@ public Task<ValidationResult> ValidateAsync(T instance, CancellationToken cancel
public virtual ValidationResult Validate(ValidationContext<T> context) {
ArgumentNullException.ThrowIfNull(context);

// Note: Sync-over-async is OK in this scenario.
// The use of the `useAsync` parameter ensures that no async code is
// actually run, and we're using ValueTask
// which is optimised for synchronous execution of tasks.
// Unlike 'real' sync-over-async, we can never run into deadlocks as we're not actually invoking anything asynchronously.
// See RuleComponent.ValidateAsync for the lowest level.
// This technique is used by Microsoft within the .net runtime to avoid duplicate code paths for sync/async.
// See https://www.thereformedprogrammer.net/using-valuetask-to-create-methods-that-can-work-as-sync-or-async/
try {
ValueTask<ValidationResult> completedValueTask
= ValidateInternalAsync(context, useAsync: false, default);

// Sync tasks should always be completed.
System.Diagnostics.Debug.Assert(completedValueTask.IsCompleted);

// GetResult() will also bubble up any exceptions correctly.
return completedValueTask.GetAwaiter().GetResult();
return ValidateInternal(context);
}
catch (AsyncValidatorInvokedSynchronouslyException) {
// If we attempted to execute an async validator, re-create the exception with more useful info.
Expand All @@ -149,10 +134,11 @@ ValueTask<ValidationResult> completedValueTask
public virtual async Task<ValidationResult> ValidateAsync(ValidationContext<T> context, CancellationToken cancellation = default) {
ArgumentNullException.ThrowIfNull(context);
context.IsAsync = true;
return await ValidateInternalAsync(context, useAsync: true, cancellation);
return await ValidateInternalAsync(context, cancellation);
}

private async ValueTask<ValidationResult> ValidateInternalAsync(ValidationContext<T> context, bool useAsync, CancellationToken cancellation) {
[Zomp.SyncMethodGenerator.CreateSyncVersion(OmitNullableDirective = true)]
private async ValueTask<ValidationResult> ValidateInternalAsync(ValidationContext<T> context, CancellationToken cancellation) {
var result = new ValidationResult(context.Failures);
bool shouldContinue = PreValidate(context, result);

Expand All @@ -175,7 +161,7 @@ ValueTask<ValidationResult> completedValueTask
cancellation.ThrowIfCancellationRequested();
var totalFailures = context.Failures.Count;

await Rules[i].ValidateAsync(context, useAsync, cancellation);
await Rules[i].ValidateAsync(context, cancellation);

if (ClassLevelCascadeMode == CascadeMode.Stop && result.Errors.Count > totalFailures) {
// Bail out if we're "failing-fast". Check to see if the number of failures
Expand Down
1 change: 1 addition & 0 deletions src/FluentValidation/FluentValidation.csproj
Expand Up @@ -28,6 +28,7 @@ Full release notes can be found at https://github.com/FluentValidation/FluentVal
<ItemGroup>
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.3" PrivateAssets="all" />
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.*" PrivateAssets="All" />
<PackageReference Include="Zomp.SyncMethodGenerator" Version="1.3.8-beta" PrivateAssets="All" />
</ItemGroup>
<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
<PackageReference Include="System.Threading.Tasks.Extensions" Version="4.5.4" />
Expand Down
3 changes: 2 additions & 1 deletion src/FluentValidation/IValidationRuleInternal.cs
Expand Up @@ -24,7 +24,8 @@ namespace FluentValidation;
using Internal;

internal interface IValidationRuleInternal<T> : IValidationRule<T> {
ValueTask ValidateAsync(ValidationContext<T> context, bool useAsync, CancellationToken cancellation);
ValueTask ValidateAsync(ValidationContext<T> context, CancellationToken cancellation);
void Validate(ValidationContext<T> context);

void AddDependentRules(IEnumerable<IValidationRuleInternal<T>> rules);
}
Expand Down
39 changes: 19 additions & 20 deletions src/FluentValidation/Internal/CollectionPropertyRule.cs
Expand Up @@ -33,7 +33,7 @@ namespace FluentValidation.Internal;
/// </summary>
/// <typeparam name="TElement"></typeparam>
/// <typeparam name="T"></typeparam>
internal class CollectionPropertyRule<T, TElement> : RuleBase<T, IEnumerable<TElement>, TElement>, ICollectionRule<T, TElement>, IValidationRuleInternal<T, TElement> {
internal partial class CollectionPropertyRule<T, TElement> : RuleBase<T, IEnumerable<TElement>, TElement>, ICollectionRule<T, TElement>, IValidationRuleInternal<T, TElement> {
/// <summary>
/// Initializes new instance of the CollectionPropertyRule class
/// </summary>
Expand Down Expand Up @@ -61,7 +61,9 @@ public CollectionPropertyRule(MemberInfo member, Func<T, IEnumerable<TElement>>
return new CollectionPropertyRule<T, TElement>(member, x => compiled(x), expression, cascadeModeThunk, typeof(TElement));
}

async ValueTask IValidationRuleInternal<T>.ValidateAsync(ValidationContext<T> context, bool useAsync, CancellationToken cancellation) {

[Zomp.SyncMethodGenerator.CreateSyncVersion(OmitNullableDirective = true)]
async ValueTask IValidationRuleInternal<T>.ValidateAsync(ValidationContext<T> context, CancellationToken cancellation) {
string displayName = GetDisplayName(context);

if (PropertyName == null && displayName == null) {
Expand Down Expand Up @@ -89,17 +91,15 @@ public CollectionPropertyRule(MemberInfo member, Func<T, IEnumerable<TElement>>
}

if (AsyncCondition != null) {
if (useAsync) {
if (!await AsyncCondition(context, cancellation)) {
return;
}
}
else {
throw new AsyncValidatorInvokedSynchronouslyException();
if (!await AsyncCondition(context, cancellation)) {
return;
}
#if SYNC_ONLY
throw new AsyncValidatorInvokedSynchronouslyException();
#endif
}

var filteredValidators = await GetValidatorsToExecuteAsync(context, useAsync, cancellation);
var filteredValidators = await GetValidatorsToExecuteAsync(context, cancellation);

if (filteredValidators.Count == 0) {
// If there are no property validators to execute after running the conditions, bail out.
Expand Down Expand Up @@ -152,7 +152,7 @@ public CollectionPropertyRule(MemberInfo member, Func<T, IEnumerable<TElement>>
context.MessageFormatter.Reset();
context.MessageFormatter.AppendArgument("CollectionIndex", index);

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

if (!valid) {
PrepareMessageFormatterForValidationError(context, valueToValidate);
Expand All @@ -177,7 +177,7 @@ public CollectionPropertyRule(MemberInfo member, Func<T, IEnumerable<TElement>>
if (context.Failures.Count <= totalFailures && DependentRules != null) {
foreach (var dependentRule in DependentRules) {
cancellation.ThrowIfCancellationRequested();
await dependentRule.ValidateAsync(context, useAsync, cancellation);
await dependentRule.ValidateAsync(context, cancellation);
}
}
}
Expand All @@ -187,7 +187,8 @@ public CollectionPropertyRule(MemberInfo member, Func<T, IEnumerable<TElement>>
DependentRules.AddRange(rules);
}

private async ValueTask<List<RuleComponent<T,TElement>>> GetValidatorsToExecuteAsync(ValidationContext<T> context, bool useAsync, CancellationToken cancellation) {
[Zomp.SyncMethodGenerator.CreateSyncVersion(OmitNullableDirective = true)]
private async ValueTask<List<RuleComponent<T, TElement>>> GetValidatorsToExecuteAsync(ValidationContext<T> context, CancellationToken cancellation) {
// Loop over each validator and check if its condition allows it to run.
// This needs to be done prior to the main loop as within a collection rule
// validators' conditions still act upon the root object, not upon the collection property.
Expand All @@ -204,14 +205,12 @@ public CollectionPropertyRule(MemberInfo member, Func<T, IEnumerable<TElement>>
}

if (component.HasAsyncCondition) {
if (useAsync) {
if (!await component.InvokeAsyncCondition(context, cancellation)) {
validators.Remove(component);
}
}
else {
throw new AsyncValidatorInvokedSynchronouslyException();
if (!await component.InvokeAsyncCondition(context, cancellation)) {
validators.Remove(component);
}
#if SYNC_ONLY
throw new AsyncValidatorInvokedSynchronouslyException();
#endif
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/FluentValidation/Internal/IncludeRule.cs
Expand Up @@ -13,7 +13,7 @@ public interface IIncludeRule { }
/// <summary>
/// Include rule
/// </summary>
internal class IncludeRule<T> : PropertyRule<T, T>, IIncludeRule {
internal partial class IncludeRule<T> : PropertyRule<T, T>, IIncludeRule {
/// <summary>
/// Creates a new IncludeRule
/// </summary>
Expand Down Expand Up @@ -52,7 +52,8 @@ public static IncludeRule<T> Create<TValidator>(Func<T, TValidator> func, Func<C
return new IncludeRule<T>((ctx, _) => func(ctx.InstanceToValidate), cascadeModeThunk, typeof(T), typeof(TValidator));
}

public override async ValueTask ValidateAsync(ValidationContext<T> context, bool useAsync, CancellationToken cancellation) {
[Zomp.SyncMethodGenerator.CreateSyncVersion(OmitNullableDirective = true)]
public override async ValueTask ValidateAsync(ValidationContext<T> context, CancellationToken cancellation) {
// Special handling for the MemberName selector.
// We need to disable the MemberName selector's cascade functionality whilst executing
// an include rule, as an include rule should be have as if its children are actually children of the parent.
Expand All @@ -66,7 +67,7 @@ public static IncludeRule<T> Create<TValidator>(Func<T, TValidator> func, Func<C
context.RootContextData[MemberNameValidatorSelector.DisableCascadeKey] = true;
}

await base.ValidateAsync(context, useAsync, cancellation);
await base.ValidateAsync(context, cancellation);

if (shouldAddStateKey) {
context.RootContextData.Remove(MemberNameValidatorSelector.DisableCascadeKey);
Expand Down
34 changes: 12 additions & 22 deletions src/FluentValidation/Internal/PropertyRule.cs
Expand Up @@ -28,7 +28,7 @@ namespace FluentValidation.Internal;
/// <summary>
/// Defines a rule associated with a property.
/// </summary>
internal class PropertyRule<T, TProperty> : RuleBase<T, TProperty, TProperty>, IValidationRuleInternal<T, TProperty> {
internal partial class PropertyRule<T, TProperty> : RuleBase<T, TProperty, TProperty>, IValidationRuleInternal<T, TProperty> {

public PropertyRule(MemberInfo member, Func<T, TProperty> propertyFunc, LambdaExpression expression, Func<CascadeMode> cascadeModeThunk, Type typeToValidate)
: base(member, propertyFunc, expression, cascadeModeThunk, typeToValidate) {
Expand All @@ -47,15 +47,9 @@ public PropertyRule(MemberInfo member, Func<T, TProperty> propertyFunc, LambdaEx
/// Performs validation using a validation context and adds collected validation failures to the Context.
/// </summary>
/// <param name="context">Validation Context</param>
/// <param name="useAsync">
/// Whether asynchronous components are allowed to execute.
/// This will be set to True when ValidateAsync is called on the root validator.
/// This will be set to False when Validate is called on the root validator.
/// When set to True, asynchronous components and asynchronous conditions will be executed.
/// When set to False, an exception will be thrown if a component can only be executed asynchronously or if a component has an async condition associated with it.
/// </param>
/// <param name="cancellation"></param>
public virtual async ValueTask ValidateAsync(ValidationContext<T> context, bool useAsync, CancellationToken cancellation) {
[Zomp.SyncMethodGenerator.CreateSyncVersion(OmitNullableDirective = true)]
public virtual async ValueTask ValidateAsync(ValidationContext<T> context, CancellationToken cancellation) {
string displayName = GetDisplayName(context);

if (PropertyName == null && displayName == null) {
Expand All @@ -79,14 +73,12 @@ public PropertyRule(MemberInfo member, Func<T, TProperty> propertyFunc, LambdaEx
}

if (AsyncCondition != null) {
if (useAsync) {
if (!await AsyncCondition(context, cancellation)) {
return;
}
}
else {
#if SYNC_ONLY
throw new AsyncValidatorInvokedSynchronouslyException();
}
#endif
}

bool first = true;
Expand All @@ -107,14 +99,12 @@ public PropertyRule(MemberInfo member, Func<T, TProperty> propertyFunc, LambdaEx
}

if (component.HasAsyncCondition) {
if (useAsync) {
if (!await component.InvokeAsyncCondition(context, cancellation)) {
continue;
}
}
else {
throw new AsyncValidatorInvokedSynchronouslyException();
if (!await component.InvokeAsyncCondition(context, cancellation)) {
continue;
}
#if SYNC_ONLY
throw new AsyncValidatorInvokedSynchronouslyException();
#endif
}

if (first) {
Expand All @@ -127,7 +117,7 @@ public PropertyRule(MemberInfo member, Func<T, TProperty> propertyFunc, LambdaEx
}
}

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

if (!valid) {
PrepareMessageFormatterForValidationError(context, propValue);
Expand All @@ -145,7 +135,7 @@ public PropertyRule(MemberInfo member, Func<T, TProperty> propertyFunc, LambdaEx
if (context.Failures.Count <= totalFailures && DependentRules != null) {
foreach (var dependentRule in DependentRules) {
cancellation.ThrowIfCancellationRequested();
await dependentRule.ValidateAsync(context, useAsync, cancellation);
await dependentRule.ValidateAsync(context, cancellation);
}
}
}
Expand Down
26 changes: 13 additions & 13 deletions src/FluentValidation/Internal/RuleComponent.cs
Expand Up @@ -30,7 +30,7 @@ namespace FluentValidation.Internal;
/// In a rule definition such as RuleFor(x => x.Name).NotNull().NotEqual("Foo")
/// the NotNull and the NotEqual are both rule components.
/// </summary>
public class RuleComponent<T,TProperty> : IRuleComponent<T,TProperty> {
public partial class RuleComponent<T, TProperty> : IRuleComponent<T, TProperty> {
private string _errorMessage;
private Func<ValidationContext<T>, TProperty, string> _errorMessageFactory;
private Func<ValidationContext<T>, bool> _condition;
Expand Down Expand Up @@ -63,20 +63,20 @@ private protected virtual bool SupportsAsynchronousValidation
private protected virtual bool SupportsSynchronousValidation
=> _propertyValidator != null;

internal async ValueTask<bool> ValidateAsync(ValidationContext<T> context, TProperty value, bool useAsync, CancellationToken cancellation) {
if (useAsync) {
// If ValidateAsync has been called on the root validator, then always prefer
// the asynchronous property validator (if available).
if (SupportsAsynchronousValidation) {
return await InvokePropertyValidatorAsync(context, value, cancellation);
}

// If it doesn't support Async validation, then this means
// the property validator is a Synchronous.
// We don't need to explicitly check SupportsSynchronousValidation.
return InvokePropertyValidator(context, value);
internal async ValueTask<bool> ValidateAsync(ValidationContext<T> context, TProperty value, CancellationToken cancellation) {
// If ValidateAsync has been called on the root validator, then always prefer
// the asynchronous property validator (if available).
if (SupportsAsynchronousValidation) {
return await InvokePropertyValidatorAsync(context, value, cancellation);
}

// If it doesn't support Async validation, then this means
// the property validator is a Synchronous.
// We don't need to explicitly check SupportsSynchronousValidation.
return InvokePropertyValidator(context, value);
}

internal bool Validate(ValidationContext<T> context, TProperty value) {
// If Validate has been called on the root validator, then always prefer
// the synchronous property validator.
if (SupportsSynchronousValidation) {
Expand Down