FastBinaryEncoding icon indicating copy to clipboard operation
FastBinaryEncoding copied to clipboard

generate csharp partials

Open fuocor opened this issue 2 years ago • 8 comments

have you thought about generating the structs as partial, so that they can be extended?

fuocor avatar Apr 05 '22 16:04 fuocor

I think it's possible to generate c# structs as partial if it not reduce the performance.

Also I'm thinking about experimenting with new c# records instead of structs, because:

  1. FBE are usually big enougth
  2. Can have fields with containers (Arrays, Dictionaries, etc)
  3. Need to support inheritance, which now implemented ugly as parent filed

But of course we need to create a POC and measure it's performance over strucs.

chronoxor avatar Apr 06 '22 17:04 chronoxor

One of the issues I noticed is that the type models are not thread safe. Concurrent writes can collide on the underlying buffer. To get around this I would eliminate FBE.Buffer all together and rework the serialization to use IBufferWriter and Span or ReadonlySpan for deserialization.

The Serialization method would require passing in bufferWriter. I created an unsafe extension class that is compatible with your primative type serialization that foregoes boundary checks and is likely a lot faster.

From there I would try an inline the serialization into the generated class in order to automatically support polymorphism.

    ctor(ReadonlySpan<byte> buffer);
    virtual void Serialize(IBufferWriter<byte> writer);
    virtual void Serialize(Span<byte> buffer);
    virtual int GetBufferSize();

If the types were generated as immutable then the GetBufferSize() can be optimized. To make the types immutable you would need to replace arrays with IReadonlyList<T> and construct the appropriate constructors. The nice thing is that if you implement record they can mutate by copy with the with {} clause.

A PipeWriter is problematic if someone decided to use the WriteAsync because it would advance it. But your sender/receiver pattern should be reworkable to support 'Pipe`

I do have considerable experience with Roslyn code generation and can write an analyzer to generate the c# code. I just need to find the time and knowledge that I will not be the only one using it.

fuocor avatar Apr 13 '22 16:04 fuocor

Here are the serialization benchmarks when bypassing Buffer and using Span<T>

Method Mean Error StdDev
Deserialize 201.61 ns 3.869 ns 4.140 ns
FastSerialize 26.19 ns 0.421 ns 0.394 ns
Serialize 218.72 ns 3.560 ns 3.330 ns
Verify 99.02 ns 0.398 ns 0.373 ns

fuocor avatar Apr 14 '22 22:04 fuocor

If your curious this is what the write looks like. I updated the extensions to a fluent model that automatically advances. I will add deserialization benchmarks tomorrow.

    public static Span<byte> FastWrite(this DeviceMotion value, Span<byte> span)
    {
        return span.Write(StructSize)
                .Write(Type)
                .Write(value.ts)
                .Write(value.acceleration)
                .Write(value.orientation)
                .Write(value.rotation);
    }

fuocor avatar Apr 15 '22 04:04 fuocor

Method Mean Error StdDev
Deserialize 197.73 ns 2.439 ns 2.281 ns
FastDeserialize 48.00 ns 0.203 ns 0.190 ns
FastSerialize 27.09 ns 0.275 ns 0.257 ns
Serialize 226.64 ns 2.316 ns 2.166 ns
Verify 96.43 ns 1.205 ns 1.127 ns

fuocor avatar Apr 15 '22 12:04 fuocor

    public DeviceMotion(ReadOnlySpan<byte> buffer)
    {
        _ = buffer.ReadInt64(out this.ts)
            .ReadSingleArray(out this.acceleration)
            .ReadSingleArray(out this.orientation)
            .ReadSingleArray(out this.rotation);
    }

fuocor avatar Apr 15 '22 12:04 fuocor

@fuocor if you'd like some help I can find time to work on this since I have great interest in this library and in the optimizations you suggested. I don't have specific experience with code generation in C# with Roslyn but I know a good deal of C#.

alandemaria avatar Nov 14 '22 22:11 alandemaria

@alandemaria. I have a great deal of experience with Roslyn and would appreciate the help but would like the buy-in of @chronoxor before committing to the effort.

fuocor avatar Nov 15 '22 00:11 fuocor