davidfowl / BedrockFramework

High performance, low level networking APIs for building custom servers and clients.
MIT License
1.05k stars 153 forks source link

(may be OT) recursive struct scenarios - discussion #78

Closed mgravell closed 4 years ago

mgravell commented 4 years ago

In protocol handling, I frequently find myself having to deal with potentially recursive payload structures; I'd like to be able to do this in a zero-alloc way.

to set the scene, something like:

    readonly struct SomeFrame
    {
        public readonly int Whatever;
        public readonly ReadOnlySequence<byte> BinaryPayload;
        // sub-elements
        public readonly ReadOnlyMemory<SomeFrame> Children;
    }

This is simple for a class SomeFrame, but gets complex for struct SomeFrame because the runtime / type-loader gets fussy about the size of the element. To be explicit, you can't have a struct Foo that contains SomeGenericStruct<Foo> because it is worried that SomeGenericStruct<T> could (now or at a later date) contain a field T, making it a recursive struct aka "not good".

(assume for this discussion that I can actually construct the tree allocations themselves; that's not a factor here)

The only mechanism in the existing code that works reliably for this is a SomeFrame[] (plus offset/length as needed) - the addition of a ref-type acts as a c-c-c-combo-breaker. It would be nice if this could be abstracted to ReadOnlyMemory<SomeFrame> or ReadOnlySequence<SomeFrame>


I have some mechanisms for doing this, but it gets a little ... messy. In particular, in "Pipelines.Sockets.Unofficial", I have a non-generic Sequence that works a lot like Sequence<T>, and allows type-less storage and retrieval essentially of the start/end position payloads via a simple cast:

private readonly Sequence _items;
internal int ItemsCount => (int)_items.Length;
internal Sequence<RawResult> GetItems() => _items.Cast<RawResult>();

I guess my question here is: do you see this as an interesting scenario for Bedrock, and if so: how should it be addressed? I hacked it by having my own sequence type, but the above isn't supported on ReadOnlySequence<T>, and neither Memory<T> nor ReadOnlySequence<T> have a suitable "give me the data in an untyped form, that I can reconstruct back from later".

In particular, the SequencePosition start/end-based constructor on ReadOnlySequence<T> is not exposed, so we can't just store the .Start and .End as SequencePosition and rehydrate from there.

Thoughts? Or is this just not an interesting scenario in general?


(for immediate context; this came up while trying to write a RESP protocol handler based on Bedrock where I'm essentially building a map of the received payload as lookups into the "live" pipe data, i.e. before we call Advance)

YairHalberstadt commented 4 years ago

In particular, the SequencePosition start/end-based constructor on ReadOnlySequence is not exposed, so we can't just store the .Start and .End as SequencePosition and rehydrate from there.

As a 'mechanism of last resort' it looks like by doing type checks on the objects in the SequencePositions you should be able to rehydrate them, although this is EXTREMELY BRITTLE.

See the constructors starting at: https://github.com/dotnet/runtime/blob/334391bdc9d4e9e20ea8b8a19ec5788da680b850/src/libraries/System.Memory/src/System/Buffers/ReadOnlySequence.cs#L93

Something like this ought to work:

        public ReadOnlySequence<SomeFrame> Rehydrate(SequencePosition start, SequencePosition end)
        {
            if (start.GetObject() is ReadOnlySequenceSegment<SomeFrame> rossStart && end.GetObject() is ReadOnlySequenceSegment<SomeFrame> rossEnd)
            {
                return new ReadOnlySequence<SomeFrame>(rossStart, start.GetInteger(), rossEnd, end.GetInteger());
            }
            if (start.GetObject() is SomeFrame[] arrStart && end.GetObject() is SomeFrame[] arrEnd)
            {
                Debug.Assert(arrStart == arrEnd);
                return new ReadOnlySequence<SomeFrame>(arrStart, start.GetInteger(), end.GetInteger());
            }
            if (start.GetObject() is MemoryManager<SomeFrame> mmStart && end.GetObject() is MemoryManager<SomeFrame> mmEnd)
            {
                Debug.Assert(mmStart == mmEnd);
                return new ReadOnlySequence<SomeFrame>(mmStart.Memory.Slice(start.GetInteger(), end.GetInteger()));
            }
            throw new InvalidOperationException();
        }

And if anyone asks, I definitely did not suggest this, and have no idea who Yair Halberstadt is :-)

mgravell commented 4 years ago

Yeah, that brittle fun is basically what my untyped Sequence does - maybe I'll port that code "from an unknown author, I didn't get their name, can't remember what they looked like" above into something similar, thanks.

YairHalberstadt commented 4 years ago

:-)

You could use reflection to have a sanity check that no new constructors have been added, which would run once every time the process is run.

mgravell commented 4 years ago

@YairHalberstadt best to do that as a unit test IMO. It isn't as though I can do anything about it at runtime anyway!