HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Trampoline Interceptor

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
interceptortrampolinestackoverflow

Problem

Aim

In C#, the following method will cause a StackOverFlowException if called with parameter 0:

int Recurse(int i)
{
    return i<100000 ? Recurse(i+1) : i;
}


However, this method actually uses tail-call recursion, meaning that if it ever performs a recursive call, that is always the last operation it performs. For this reason, if the language supported tail call recursion optimization, a stack overflow exception could be prevented by moving down by one stack frame before entering the recursive call. (Alternatively, it could be converted into a loop, but I won't be worrying about that)

While this is not supported by C# (see below), a method that uses tail-call recursion can use a construct called a "trampoline" to run a recursive method without growing the stack. My aim was to create a trampoline which:

  • Requires minimal modification of the recursive method to be trampolined



  • Has minimal performance impact



With the former being the much higher priority. For that reason I chose an interceptor as the trampoline construct, using Castle's dynamic proxies.

(EDIT: As pointed out in the comments, the CLR will sometimes optimize tail-call recursion and avoid stack overflows. However, my understanding is that it does not always do so, and makes no guarantee about ever doing so, and so it's generally not reliable enough to avoid exceptions in many situations)

The Code

```
public class TrampolineInterceptor : IInterceptor
{
private readonly IMethodFilter _methodFilter;
private static bool _inProgress = false;
private static bool _invokeNext = false;
private static bool _first = true;

private static IInvocation _pending = null;
private static object _dummyReturnValue = null;

public TrampolineInterceptor(IMethodFilter methodFilter)
{
_methodFilter = methodFilter;
}

public void Intercept(IInvocation invocation)
{
if(!_methodFilter.ShouldIntercept(invocation))
invocation.Proceed();

Solution

The logic feels like it's overly complex. I'm especially unhappy with the need for three booleans.

Yeah, I agree that especially the logic around _invokeNext is messy, but I think that's required due to the way Proceed() behaves (the invocation is intercepted again if it's not currently being intercepted). But in cases like these, it's important to document why you wrote it that way, for the benefit of any future readers of the code.

I also considered getting rid of _inProgress by setting _pending to null and then checking for that, but that would mean you would have to deal with the return value some other way, so I think it's not worth it.

I think your fields should not be static, that way invoking one tail-recursive method from another tail-recursive method could work, at least if each is on a different object and uses different instance of your interceptor. (To make it work for methods on the same object would require some additional logic in the interceptor.)

IMethodFilter _methodFilter


Why is this a custom interface and not a delegate (Func or Predicate)? Is it because Dependency Injection?

if(!_methodFilter.ShouldIntercept(invocation))


Commonly, a space is written between if and (, since if is not a method.

makeDefault.MakeGenericMethod(returnType).Invoke(null, new object[] { });


Not sure why you're using an array initializer here, I think new object[0] is clearer. Also, according to the documentation, you can pass null instead of the empty array.

Code Snippets

IMethodFilter _methodFilter
if(!_methodFilter.ShouldIntercept(invocation))
makeDefault.MakeGenericMethod(returnType).Invoke(null, new object[] { });

Context

StackExchange Code Review Q#57839, answer score: 4

Revisions (0)

No revisions yet.