6
\$\begingroup\$

I wrote this code to De- and serialise structures afap using IL generation (comments describes C# analogue). What can be improved here?

using System;
using System.Reflection;
using System.Reflection.Emit;
using System.Runtime.InteropServices;

namespace StructInterop
{
    public static class StructInterOp
    {
        static readonly ConstructorInfo IntPtrCtor = typeof(IntPtr).GetConstructor(new[] { typeof(void*) });
        static readonly MethodInfo MarshalCopy = typeof(Marshal).GetMethod("Copy", new[] { typeof(IntPtr), typeof(byte[]), typeof(int), typeof(int) });
        private static class DelegateHolder<T> where T : struct
        {
            // ReSharper disable MemberHidesStaticFromOuterClass
            // ReSharper disable StaticMemberInGenericType
            public static readonly Type TypeOfT = typeof(T);
            public static readonly int SizeInBytes = Marshal.SizeOf(TypeOfT);

            public static readonly Func<T, byte[]> Serialize = CreateSerializationDelegate();
            public static readonly Func<byte[], T> Deserialize = CreateDeserializationDelegate();


            //public static byte[] Serialize(T value)
            //{
            //    IntPtr p = new IntPtr(&value);
            //    byte[] result = new byte[sizeof(T)];
            //    Marshal.Copy(p, result, 0, result.Length);
            //    return result;
            //}
            private static Func<T, byte[]> CreateSerializationDelegate()
            {
                var dm = new DynamicMethod("Serialize" + TypeOfT.Name,
                    typeof(byte[]),
                    new[] { TypeOfT },
                    Assembly.GetExecutingAssembly().ManifestModule);
                dm.DefineParameter(1, ParameterAttributes.None, "value");

                var generator = dm.GetILGenerator();
                generator.DeclareLocal(typeof(byte[]));

                //IntPtr p = new IntPtr(&value);
                generator.Emit(OpCodes.Ldarga_S, (byte)0);
                generator.Emit(OpCodes.Conv_U);
                generator.Emit(OpCodes.Newobj, IntPtrCtor);

                //byte[] result = new byte[sizeof(T)]; 
                OpCode ldcStructSize = SizeInBytes < sbyte.MaxValue ? OpCodes.Ldc_I4_S : OpCodes.Ldc_I4;
                generator.Emit(ldcStructSize, SizeInBytes);
                generator.Emit(OpCodes.Newarr, typeof(byte));

                //Marshal.Copy(p, result, 0, result.Length);
                generator.Emit(OpCodes.Stloc_0);
                generator.Emit(OpCodes.Ldloc_0);
                generator.Emit(OpCodes.Ldc_I4_0);
                generator.Emit(OpCodes.Ldloc_0);
                generator.Emit(OpCodes.Ldlen);
                generator.Emit(OpCodes.Conv_I4);
                generator.EmitCall(OpCodes.Call, MarshalCopy, null);

                //return result
                generator.Emit(OpCodes.Ldloc_0);
                generator.Emit(OpCodes.Ret);

                return (Func<T, byte[]>)dm.CreateDelegate(typeof(Func<T, byte[]>));
            }

            //public static T Deserialize(byte[] data)
            //{
            //    fixed (byte* pData = &data[0])
            //    {
            //        return *(T*)pData;
            //    }
            //}
            private static Func<byte[], T> CreateDeserializationDelegate()
            {
                var dm = new DynamicMethod("Deserialize" + TypeOfT.Name,
                                            TypeOfT,
                                            new[] { typeof(byte[]) },
                                            Assembly.GetExecutingAssembly().ManifestModule);
                dm.DefineParameter(1, ParameterAttributes.None, "data");
                var generator = dm.GetILGenerator();
                generator.DeclareLocal(typeof(byte).MakePointerType(), pinned: true);
                generator.DeclareLocal(TypeOfT);

                //fixed (byte* pData = &data[0])
                generator.Emit(OpCodes.Ldarg_0);
                generator.Emit(OpCodes.Ldc_I4_0);
                generator.Emit(OpCodes.Ldelema, typeof(byte));
                generator.Emit(OpCodes.Stloc_0);

                // return *(T*)pData;
                generator.Emit(OpCodes.Ldloc_0);
                generator.Emit(OpCodes.Conv_I);
                generator.Emit(OpCodes.Ldobj, TypeOfT);
                generator.Emit(OpCodes.Ret);
                return (Func<byte[], T>)dm.CreateDelegate(typeof(Func<byte[], T>));
            }
        }


        /// <summary>
        /// Do not check array bounds, possible buffer overflow
        /// </summary>
        public static T Deserialize<T>(byte[] data) where T : struct
        {
            return DelegateHolder<T>.Deserialize(data);
        }

        /// <summary>
        /// Check array bounds
        /// </summary>
        public static T DeserializeSafe<T>(byte[] data) where T : struct
        {
            if (DelegateHolder<T>.SizeInBytes != data.Length)
                throw new ArgumentException(string.Format("Struct size is {0} bytes but array is {1} bytes length", DelegateHolder<T>.SizeInBytes, data.Length));
            return DelegateHolder<T>.Deserialize(data);
        }

        /// <summary>
        /// Marshal struct in byte array without any type information
        /// </summary>
        public static byte[] Serialize<T>(this T value) where T : struct
        {
            return DelegateHolder<T>.Serialize(value);
        }
    }
}
\$\endgroup\$
7
\$\begingroup\$

Naming

Interop should be treated as a single word like in System.Runtime.InteropServices. You should also not name a class the same as its namespace. Which is why this is really not good:

namespace StructInterop
{
    public static class StructInterOp
    {

Given that the StructInterOp class has methods like Serialize and Deserialize - why not call it StructSerializer?

Structure

You could remove a lot of your comments by spliting up the generation a bit. (The names for the new methods aren't great...)

//public static byte[] Serialize(T value)
//{
//    IntPtr p = new IntPtr(&value);
//    byte[] result = new byte[sizeof(T)];
//    Marshal.Copy(p, result, 0, result.Length);
//    return result;
//}
private static Func<T, byte[]> CreateSerializationDelegate()
{
    var dm = new DynamicMethod("Serialize" + TypeOfT.Name,
        typeof(byte[]),
        new[] { TypeOfT },
        Assembly.GetExecutingAssembly().ManifestModule);
    dm.DefineParameter(1, ParameterAttributes.None, "value");

    var generator = dm.GetILGenerator();
    generator.DeclareLocal(typeof(byte[]));

    GenerateIntPtr(generator);
    GenerateResultArray(generator);
    GenerateCopy(generator);
    GenerateReturn(generator);

    return (Func<T, byte[]>)dm.CreateDelegate(typeof(Func<T, byte[]>));
}

private static void GenerateReturn(ILGenerator generator)
{
    generator.Emit(OpCodes.Ldloc_0);
    generator.Emit(OpCodes.Ret);
}

private static void GenerateCopy(ILGenerator generator)
{
    generator.Emit(OpCodes.Stloc_0);
    generator.Emit(OpCodes.Ldloc_0);
    generator.Emit(OpCodes.Ldc_I4_0);
    generator.Emit(OpCodes.Ldloc_0);
    generator.Emit(OpCodes.Ldlen);
    generator.Emit(OpCodes.Conv_I4);
    generator.EmitCall(OpCodes.Call, MarshalCopy, null);
}

private static void GenerateResultArray(ILGenerator generator)
{
    OpCode ldcStructSize = SizeInBytes < sbyte.MaxValue ? OpCodes.Ldc_I4_S : OpCodes.Ldc_I4;
    generator.Emit(ldcStructSize, SizeInBytes);
    generator.Emit(OpCodes.Newarr, typeof(byte));
}

private static void GenerateIntPtr(ILGenerator generator)
{
    generator.Emit(OpCodes.Ldarga_S, (byte)0);
    generator.Emit(OpCodes.Conv_U);
    generator.Emit(OpCodes.Newobj, IntPtrCtor);
}

Comments

/// <summary>
/// Do not check array bounds, possible buffer overflow
/// </summary>
public static T Deserialize<T>(byte[] data) where T : struct

That's not a good comment. It should summarise what the method does:

/// <summary>
/// Deserializes the given data to the specified type.
/// </summary>
/// <paramref name="data">The serialized struct data.</paramref>
/// <typeparam name="T">The type of the struct to be deserialized.</typeparam>
/// <returns>An instance of type <typeparamref name="T"/></returns>
\$\endgroup\$
  • \$\begingroup\$ >> why not call it StructSerializer - I was assuming add more methods to interop with structs. >> You could remove a lot of your comments by spliting up the generation a bit - I had same idea but I found comments makes code "cleaner", than method extract \$\endgroup\$ – Alex Zhukovskiy Jul 16 '15 at 11:46
  • \$\begingroup\$ @AlexZhukovskiy - Serializing structs is enough for one class to do. It's easy enough to create more classes for other things. Fair enough about the comments explaining the IL generation; it's very much personal preference there. \$\endgroup\$ – RobH Jul 16 '15 at 11:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.