Performance optimization: focusing on allocation reduction

[ProfFrugal]

Even in the non failure case, FluentValidation has too many allocations. For the Customer example, single validation has 44 allocations, 2,172 bytes in total. This can be reduced to 10 allocations (including the Customer object), 604 bytes, 72% reduction.

[ProfFrugal]

@dotnet-policy-service agree

[ProfFrugal]

For detailed analysis, read Frugal Cafe postings 60,61,62:

https://frugalcafe.beehiiv.com/p/fc60-fluentvalidation-engineered-not-recommended
https://frugalcafe.beehiiv.com/p/fc61-complete-allocation-sequence
https://frugalcafe.beehiiv.com/p/fc62-peel-fluentvalidation-onion

[JeremySkinner]

Hello, Thank you for the suggestion. There are currently 13 tests failing introduced by these changes. Please resolve all test failures - once this is done I'll then review the PR. Thanks

[JeremySkinner]

Hi @paulomorgado glad you managed to get the failures to show up locally. I've added some comments to show where the bugs are, as well as some other changes I'd like to see. Thanks!

[JeremySkinner]

I had some time today to implement the necessary fixes to the PR. I have made the following changes:

• Resolved bug introduced in NotEmptyValidator which was causing test failures
• Updated EmptyValidator to be consistent with NotEmptyValidator
• Resolved bug introduced in AbstractValidator.SetExecutedRuleSets which was causing test failures
• Revert the changes to PropertyChain. Changing the base class for a public type causes baseline validation to fail. We adhere to semver so this can't be done as part of 11.x but can be re-visited as part of the 12.0 release. I have created an issue to track this Change PropertyChain to inherit from List<string> to avoid allocations #2163
• Moved the caching of the GetDisplayName func out of PropertyRule and into RuleBase so it can also be used by CollectionPropertyRule
• Fixed various indentation/formatting issues and added some comments

This has now been merged with these changes. Thanks for contributing.

[billrob]

@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

[JeremySkinner]

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