Repository navigation
Plain guard clauses instead of the fluent condition chain - #12
Merged
Merged
Conversation
Both conversion directions opened with holonsoft.FluentConditions:
IsPrepared.Requires("Prepare()").IsTrue();
data.Requires(nameof(data)).IsNotNull();
That package is published as a non optimized build. Its assembly carries
DebuggableAttribute with IsJITOptimizerDisabled, so the JIT neither optimizes
nor inlines anything inside it, in every process that consumes it.
BenchmarkDotNet refuses to measure against such a reference at all, which is
how this was found.
A fluent validator chain is the wrong tool for a per message guard clause no
matter how it was built, so both directions now use a plain compare with the
throw in a separate non inlined method. Prepare() keeps the fluent version, it
runs once per converter.
The exception types are unchanged, ArgumentNullException and
ArgumentOutOfRangeException, so code that catches them keeps working, and the
original message text is kept as the message.
A/B of the same tree with and without the change, back to back:
Read LE 133.07 ns -> 68.17 ns -48.8%
Read BE 125.90 ns -> 67.37 ns -46.5%
Read reused 125.75 ns -> 59.59 ns -52.6%
Read string 66.76 ns -> 37.70 ns -43.5%
Write LE 129.14 ns -> 110.74 ns -14.2%
Write BE 131.19 ns -> 112.02 ns -14.6%
Write string 138.26 ns -> 119.97 ns -13.2%
Allocation is unchanged in every row, the validator was a struct.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two corrections to the previous commit. The claim that the saving is explained by the reference type overload of Requires being the expensive one does not survive arithmetic: both directions removed the same two guards, yet reading saved about 65 ns and writing about 18 ns. The A/B shows that every row improved and that reading gained far more than writing. It does not show why, and nothing here isolates the rest of it, so the document now says that instead of asserting a mechanism. The array overloads also checked the null argument before the prepared state, which reversed the exception a caller sees when passing null to an unprepared converter. The original order is restored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Both conversion directions opened with
holonsoft.FluentConditions:Replaced with a plain compare and the throw in a separate non inlined method.
Prepare()keeps the fluent version, it runs once per converter.Why
holonsoft.FluentConditions3.0.1 is published to NuGet as a non optimized build. Its assembly carriesDebuggableAttributewithIsJITOptimizerDisabled, so the JIT neither optimizes nor inlines anything inside it, in every process that consumes it. BenchmarkDotNet refuses to measure against such a reference at all, which is how this was found.Separately from that: a fluent validator chain is the wrong tool for a per message guard clause regardless of how it was built.
Numbers
A/B of the same tree with and without the change, back to back on the same machine:
Allocation is unchanged in every row, the validator was a struct.
What the A/B shows is that every row improved and that reading gained far more than writing. What it does not show is why. Both directions removed exactly the same two guards, one on a reference type and one on
bool, yet reading saved about 65 ns and writing about 18 ns, so the saving is not simply the cost of the two calls and nothing measured here isolates the rest of it. The likely remainder is an inlining cascade, where an opaque call at the top of a method blocks the JIT from inlining what follows. That is a hypothesis, not a result.Compatibility
ArgumentNullExceptionfor a null argument,ArgumentOutOfRangeExceptionfor calling beforePrepare(). The existing test that pins the second one still passes.'data' is null!) is kept, now with the parameter name attached as well, which FluentConditions dropped.Heads up beyond this repo
holonsoft.FluentConditions3.0.1,holonsoft.FluentDateTime2.1.1 andholonsoft.Utils1.10.1 are all published as non optimized builds. All three setGeneratePackageOnBuild, so the.nupkgcontains whatever configuration was built last, and an IDE build defaults to Debug. This library packs with an explicit-c Releaseinpublish.ymland is not affected itself.🤖 Generated with Claude Code