patterncsharpMinor
Trampoline Interceptor
Viewed 0 times
interceptortrampolinestackoverflow
Problem
Aim
In C#, the following method will cause a
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:
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();
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
I also considered getting rid of
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.)
Why is this a custom interface and not a delegate (
Commonly, a space is written between
Not sure why you're using an array initializer here, I think
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 _methodFilterWhy 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 _methodFilterif(!_methodFilter.ShouldIntercept(invocation))makeDefault.MakeGenericMethod(returnType).Invoke(null, new object[] { });Context
StackExchange Code Review Q#57839, answer score: 4
Revisions (0)
No revisions yet.