0

I am currently working on a project that generates code based on other code. Essentially using the programming language itself as a DSL.

One of the generator targets is a binary DataContract serializer and the generated ToBytes for the following class

[DataContract]
public partial class Root
{
    [DataMember]
    public int Number { get; set; }

    [DataMember]
    public Partial[] Partials { get; set; }

    [DataMember]
    public IList<ulong> Numbers { get; set; }
}

becomes this:

public int Size
{
    get 
    { 
        var size = 8;
        // Add size for collections and strings
        size += Partials.Sum(entry => entry.Size);
        size += Numbers.Count * 8;

        return size;              
    }
}
public byte[] ToBytes()
{
    var index = 0;
    var bytes = new byte[Size];

    return ToBytes(bytes, ref index);
}
public byte[] ToBytes(byte[] bytes, ref int index)
{
    // Convert Number
    Buffer.BlockCopy(BitConverter.GetBytes(Number), 0, bytes, index, 4);;
    index += 4;
    // Convert Partials
    // Two bytes length information for each dimension
    Buffer.BlockCopy(BitConverter.GetBytes((ushort)(Partials == null ? 0 : Partials.Length)), 0, bytes, index, 2);
    index += 2;
    foreach(var value in Partials ?? Enumerable.Empty<Partial>())
    {
        value.ToBytes(bytes, ref index);
    }
    // Convert Numbers
    // Two bytes length information for each dimension
    Buffer.BlockCopy(BitConverter.GetBytes((ushort)(Numbers == null ? 0 : Numbers.Count)), 0, bytes, index, 2);
    index += 2;
    foreach(var value in Numbers ?? Enumerable.Empty<ulong>())
    {
        Buffer.BlockCopy(BitConverter.GetBytes(value), 0, bytes, index, 8);;
        index += 8;
    }
    return bytes;
}

Now even tough this is really fast I am looking for a way to speed up all the uses of Buffer.BlockCopy(BitConverter.GetBytes. It still seems to be waste of resources to create a new small byte[] for every conversion and then copy that when I allready have the byte[] and the position.

Any ideas how to best improve the code?

Update: Based on @adrianm comment I will replace foreach with for-loops for arrays and wrap nullable type in if statements. Using structs as in another thread is not wanted. I would rather use classes and [DataContract] attributes. Also for Linux compliance I can't use WinApi.

Update2: Added the rest of the generated code. Thanks to a comment future version will include

if (index + Size > bytes.Length)
    // Some error handling
Community
  • 1
  • 1
Toxantron
  • 2,218
  • 12
  • 23
  • 1
    Allocations and byte copy are really fast. My guess is that you could increase speed more if you focus on other parts in the code like using `for loops` instead of `foreach` and `if` instead of `?? Enumerable.Empty` – adrianm Apr 07 '16 at 07:52
  • Typically it's `struct` and [`Marshal.StructureToPtr`](http://stackoverflow.com/a/3278956/1997232), if you need *fast*, then winapi methods are [the way](http://stackoverflow.com/a/24027952/1997232) to go. – Sinatr Apr 07 '16 at 07:55
  • @adrianm good point. I used `foreach` because it is compatible with all collections but looking at [ArrayEnumerator](https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Array.cs#L2546) this might have been the wring choice. Thanks for the feedback. – Toxantron Apr 07 '16 at 08:03
  • @Sinatr to be honest I didn't know about this but it seems as if it would not fully cover what I want to do. Also using a struct would mean it is copied every time it is assigned somewhere. In case a big instance get's passed around a lot this would be a performance loss, wouldn't it? And using WinApi won't work on Linux/Mono, will it? – Toxantron Apr 07 '16 at 08:09
  • @Sinatr I updated the question so thanks for pointing it out. – Toxantron Apr 07 '16 at 08:14
  • 1
    `BitConverter` is not magic. `MyBitConverter.GetBytes(int, byte[] buffer, int offset)` is trivial to write. Related: http://stackoverflow.com/questions/2527018/ – Jeroen Mostert Apr 07 '16 at 08:24
  • @JeroenMostert this is similar to what I found in my own answer isn't it. I just inlined the code which would make the entire method unsafe. Maybe for now I will create MyBitConverter and create a PR for coreCLR with an IncludeBytes(someValue, byte[] bytes, int pos) method to remove my own class in the future. – Toxantron Apr 07 '16 at 08:40
  • Unsafe code has the obvious drawback of being, well, unsafe. :-P In the code you provided, there's no protection against the array being too small to hold the resulting value. In managed code you're guaranteed an exception if you exceed the bounds. Unless profiling shows the best optimized managed code is still too slow, don't use unsafe code. – Jeroen Mostert Apr 07 '16 at 08:50
  • @JeroenMostert I added the Rest of the generated code. However you brought to my intention I need to add a size check in the overload with `byte[] bytes, ref int index`. – Toxantron Apr 07 '16 at 08:55
  • @JeroenMostert after removing `??` and `foreach` I tried the MyBitConverter solution and that alone created a performance increase of around factor 2. It is unsafe but the performance is just too amazing to go managed again. I did however include checks to make sure the `bytes` has enough space at `index`. – Toxantron Apr 07 '16 at 11:09

1 Answers1

0

So I saw the implementation of BitConverter and their use of unsafe and pointers to create the byte[]. I would like to use that inside the generated code like:

*byte pointer = bytes;
*((int*)pointer + pos) = Number;
pos += 4;

But in that case this would need to be an unsafe method and I am not sure what this will do to the the more managed code in there.

Tassisto
  • 9,877
  • 28
  • 100
  • 157
Toxantron
  • 2,218
  • 12
  • 23
  • 1
    Using `unsafe` methods doesn't do anything to the rest of the managed code. `unsafe` != `unmanaged` – Jcl Apr 07 '16 at 09:01