Tuesday, April 6, 2010

You learn something everyday...if you read the documentation

As I mentioned once, I have a static array of delegates in my Z-machine base class.

The delegate signature looks like this:
protected delegate void Operation(ZMachine machine);

The array is intialized with a series of statements like this:
op[0] = op[32] = op[64] = op[96] = op[192] = z => z.Op0();
op[1] = op[33] = op[65] = op[97] = op[193] = z => z.Op1();
...
op[254] = z => z.Op254();
op[255] = z => z.Op255();

I was forced to create a wrapper method which allows the delegate (which is static) to call an instance method specified at the time of invocation. This is because the actual operations are virtual to deal with the changing behavior of the various Z-machine versions. Using lambdas instead of named methods makes things look cleaner and involves a lot less typing, but always bugged me because of the extra method call needed for each instruction that is executed. I could find no better way though.

Today while reading through some documentation I discovered a way to do this without the wrapper represented by the lambda. About a third of the way down the page I found this:

"When a delegate represents an open instance method, it stores a reference to the method's entry point. The delegate signature must include the hidden this parameter in its formal parameter list; in this case, the delegate does not have a reference to a target object, and a target object must be supplied when the delegate is invoked."

I'd never heard of open instance delegates before. This was exactly what I was looking for!

Now I can write a method like:
private static Operation CreateOperation(string method)
{
return (Operation)Delegate.CreateDelegate(typeof(Operation), typeof(ZmachineV1).GetMethod(method, BindingFlags.NonPublic BindingFlags.Instance));
}


and my array initialization looks like:
op[0] = op[32] = op[64] = op[96] = op[192] = CreateOperation("Operation0");
op[1] = op[33] = op[65] = op[97] = op[193] = CreateOperation("Operation1");
...
op[254] = CreateOperation("Operation254");
op[255] = CreateOperation("Operation255");

This reduces the size of the binary and removes the extra method call when each instruction is executed. I only wish C# had some sort of native support for this type of delegate, as the CreateOperation method is ugly and can fail at runtime if the wrong name is passed whereas delegate creation using a method group name is checked at compile time.

I've nearly finished my refactoring and hope to make some code publicly available soon.

[Update: Thanks for the comments. They inspired me to keep trying.]

I'll admit I'm inexperienced with functional programming. It gives me headaches.
From what I see, expression trees reintroduce the code bloat I had with the lambdas. Honestly I didn't look into it deep enough to see if the extra method call gets reinserted at execution time.

Anyway, I played around with it some more and came up with this (sorry about the formatting):

namespace ConsoleApplication1
{
using System;

///
/// The program.
///

internal class Program
{
///
/// Program entry.
///

private static void Main()
{
var z = new ZmachineV2();
z.Run();
}

///
/// The zmachine V1.
///

private class ZmachineV1
{
///
/// The operations.
///

private static Operation[] operations;

///
/// A fake op (Needed to get the MethodInfo).
///

private delegate void FakeOp();

///
/// An operation.
///

///
/// The zmachine.
///
private delegate void Operation(ZmachineV1 zmachine);

///
/// Gets the operations.
///

///
/// The operations.
///

private Operation[] Operations
{
get
{
return operations ?? (operations = this.InitializeOperations());
}
}

///
/// Runs one cycle of the zmachine.
///

public void Run()
{
this.Operations[0](this);
}

///
/// Operation 0.
///

protected virtual void Operation0()
{
Console.WriteLine("Called ZmachineV1.Operation0.");
}

///
/// Creates an operation.
///

///
/// The fake operation.
///
///
/// An operation.
///

private static Operation CreateOperation(FakeOp fake)
{
return (Operation)Delegate.CreateDelegate(typeof(Operation), fake.Method.GetBaseDefinition());
}

///
/// Initializes the operations.
///

///
/// The operations.
///

private Operation[] InitializeOperations()
{
var ops = new Operation[1];
ops[0] = CreateOperation(this.Operation0);
return ops;
}
}

///
/// The zmachine V2.
///

private class ZmachineV2 : ZmachineV1
{
///
/// Operation 0.
///

protected override void Operation0()
{
Console.WriteLine("Called ZmachineV2.Operation0.");
}
}
}
}


The only downsides to this that I see are that I had to give up the readonly modifier on the delegate array, wrap it in a property, create another delegate type, and the initialization generates a bit of garbage for the FakeOp delegates. On the plus side, the CreateOperation method looks better and I no longer need to refer to anything in the Reflection namespace, so I can drop the using statement for that. I no longer need to pass strings and my method names are verified at compile time. I also don't need any wrapper methods. What do you think?

10 comments:

  1. You might want to look into using expression tree compilation as well. Here's a link to an overly generic (for your situation) solution: http://caliburn.codeplex.com/SourceControl/changeset/view/46636#597833

    ReplyDelete
  2. You could probably use the static reflection technique to make the signature something like CreateOperation(z => z.Op0)

    ReplyDelete
  3. Well, you can make it a bit prettier by using System.Linq.Expressions:

    static Action CreateOperation(System.Linq.Expressions.Expression> callExpr)
    {
    return ( Action)Delegate.CreateDelegate(
    typeof( Action), ((System.Linq.Expressions.MethodCallExpression)callExpr.Body).Method);
    }

    op[254] = CreateOperation(z => z.Operation254());

    ReplyDelete
  4. You can avoid the hard-coded strings if you use something like the following to extract MethodInfo's from compiler-generated Expression Trees:

    public static class Reflect
    {
    public static MethodInfo GetMethod<TClass>(Expression<Action<TClass>> expression)
    {
    var methodCall = expression.Body as MethodCallExpression;
    if (methodCall == null)
    {
    throw new ArgumentException("Expected method call");
    }
    return methodCall.Method;
    }
    }

    and then

    op[254] = CreateOperation(Reflect.GetMethod<ZmachineV1>(z=>z.Operation254(null)));

    with the appropriate changes to CreateOperation().

    Unfortunately it's arguably less readable and you get your code bloat problem back again.

    ReplyDelete
  5. Nice - but i suggest using expression trees to keep the strong typing:

    public Operation CreateOperation(Expression<Operation> expr)
    {
    var method = ((MethodCallExpression) expr.Body).Method;
    return (Operation)Delegate.CreateDelegate(typeof(Operation), method);
    }

    var operation = CreateOperation(z=>z.Op123());

    ReplyDelete
  6. How about using an Expression for compiler support?

    op[0] = op[32] = CreateOperation(z => z.Op0());

    private static Operation CreateOperation(Expression operation)
    {
    var call = (MethodCallExpression) operation.Body;

    return (Operation) Delegate.CreateDelegate(typeof(Operation), call.Method);
    }

    ReplyDelete
  7. Instead of your private delegate, System.Core.dll (.net 3.5) has a delegate named System.Action which is the same signature as your FakeOp. Use that instead.

    ReplyDelete
  8. Anonymous -

    There are several reasons I didn't do that.

    One of my design goals is to avoid reliance on framework classes for my class library. Why reinvent the wheel? First, I plan on porting this to other languages at some future point, so the less I rely on things outside the language itself, the more likely I can do that easily. Even using C# there are framework differences between platforms, such as Mono.
    In particular, I avoid a reference to System.Core.dll because it is different between Xbox360 and Windows projects, meaning if I reference it, suddenly I am forced to create two separate projects for my class library, whereas I currently only need one.

    System.Delegate is currently the only class I use from the framework not directly supported by C# in my entire ZMachine class.

    I'm still not entirely happy with this solution as the array of operations is logically readonly. (In my actual code I also use my own ImmutableArray class instead of a mutable array.) However, it seems this is the best I can do for now as C# lacks a syntax for open instance delegates.

    ReplyDelete
  9. Posting code on here is a real pain. The editor throws out all my formating and comment tags.

    ReplyDelete
  10. @Mike: try this for formatting your source code for display in your posts:

    http://www.manoli.net/csharpformat/

    Glad to see you posting again. Keep up the good work.

    ReplyDelete