Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generate csharp partials #63

Open
fuocor opened this issue Apr 5, 2022 · 8 comments
Open

generate csharp partials #63

fuocor opened this issue Apr 5, 2022 · 8 comments

Comments

@fuocor
Copy link

fuocor commented Apr 5, 2022

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

@chronoxor
Copy link
Owner

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.

@fuocor
Copy link
Author

fuocor commented Apr 13, 2022

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
Copy link
Author

fuocor commented Apr 14, 2022

Here are the serialization benchmarks when bypassing Buffer and using Span

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
Copy link
Author

fuocor commented Apr 15, 2022

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
Copy link
Author

fuocor commented Apr 15, 2022

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
Copy link
Author

fuocor commented Apr 15, 2022

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

@alandemaria
Copy link

@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#.

@fuocor
Copy link
Author

fuocor commented Nov 15, 2022

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants