feO2x / Light.GuardClauses

A lightweight .NET library for expressive Guard Clauses.
MIT License
87 stars 8 forks source link

Might it be more performant to pass custom exceptions via delegates instead of exception objects? #3

Closed feO2x closed 8 years ago

feO2x commented 8 years ago

I'm reconsidering the design of how custom exceptions are passed into the extension methods of Light.GuardClauses: currently you specify the exception object directly, which usually looks like this in client code:

public void Foo(IList<IBar> bars)
{
    bars.MustNotContainNull(exception: new MyCustomException("The collection must not contain null entries"));

    // other statements in Foo omitted
}

In this example, this implies that every time MustNotContainNull is called, a new exception object is created - which can become a large sum of objects if you do that e.g. in a loop. This creates pressure on the Garbage Collector and the GC will take longer on its runs because it has to enumerate more objects on the heap.

Now one goal of Light.GuardClauses is to be high-performant so that this library can be used in many circumstances, and I think it might be better to provide a delegate for the custom exception instead of the object itself:

private Func<Exception> _createExceptionDelegate = CreateException;

private Exception CreateException() { return new MyCustomException("..."); }

public void Foo(IList<IBar> bars)
{
    bars.MustNotContainNull(exception: _createExceptionDelegate);

    // ...
}

This way, only a reference to an existing delegate is passed to the assertion method, which results in a lot less pressure on the GC when Foo is called because there is no call to new involved. Only when the assertion fails the delegate is actually executed to create and throw the custom exception. I don't know by heart if this is useful for Lambdas, too, i.e. if new delegate objects are created every time a Lambda is passed, but overall I like this design better because it allows you to write high-performant code.

feO2x commented 8 years ago

As it seems, Lambdas that do not capture anything are cached according to this Stack Overflow question, which would mean that the same delegate object is passed when you just create an exception object within a Lambda. But I think in most situations the caller would like to customize the message injected in his custom exception in a way so that this string is parameterized - which would result in a Lambda capturing some information about the parameters.

And I don't know how it's implemented in Roslyn - I should write some tests.

feO2x commented 8 years ago

This repo showcases that the same delegate instance is reused when a Lambda does not capture any of the surrounding variables or fields (even when you use the nameof operator). However, a new delegate instance is created when you incorporate e.g. values of parameters.

FlorianRappl commented 8 years ago

The problem you face here seems to be as follows:

You want to create an object and throw an exception, but just once a certain criteria is matched. Alright, usually you would of course go for a an if statement and the path would be clear. Passing something directly will create the object, independent if its used. You could pass also something cached, which is (as tested) naturally given by a non-capturing delegate. Similarly, one could do the buffering with Exception objects, e.g.,

static readonly Exception fooMustNotContainNull = new MyCustomException("The collection must not contain null entries");

public void Foo(IList<IBar> bars)
{
    bars.MustNotContainNull(exception: fooMustNotContainNull);

    // other statements in Foo omitted
}

This is at least as performing as your delegate, but its not as flexible. The delegate can be more dynamic and may also capture, going directly to a more state-driven mode.

I would totally go for the delegate. Besides the obvious freedom that is given by this option, the other benefit would be the "automatic" caching for such simple objects. The indirect delegate invocation certainly is not really performance relevant, especially since throwing an exception will much more severe.

One remark: The nameof is a compiler function and resolves to a constant string. Hence its clear that the usage of nameof will never result in capturing as its not a runtime (i.e., state-dependent) feature.

feO2x commented 8 years ago

@FlorianRappl Thanks for your input, I will refactor the design.

feO2x commented 8 years ago

Resolved with 188bf3be15237c733da6236440b1b92cbcc3889f