ByteBufferAllocator shouldn't have a pointer property, but instead use Memory<byte> [C#, all OS, master] #5181
Description
Looking at the C# support for directly reading and writting to memory other than byte[]. For example, ByteBuffer can be initialized with a custom allocator which uses shared memory / memory mapped files, it has an issue with using pointers.
flatbuffers/net/FlatBuffers/ByteBuffer.cs
Lines 51 to 58 in e1defaa
flatbuffers/net/FlatBuffers/ByteBuffer.cs
Lines 137 to 149 in e1defaa
Looking at the documentation for AddrOfPinnedObject, it says:
This method is used to get a stable pointer to the object. Pinning an object prevents the garbage collector from moving it around in memory, thereby reducing the efficiency of the garbage collector.
The way this is working, it will pin the byte[] of the ByteArrayAllocator for the entire lifetime of the object. Which means the garbage collector can't move this memory around, as it sees fit.
Proposal
Instead of exposing a byte* pointer directly from ByteBufferAllocator, it would be better to expose a Memory<byte> property instead. Callers can get a byte* out of a Memory<byte> with code like the following:
Memory<byte> buffer = ...;
fixed (byte* ptr = buffer.Span)
{
// use `ptr` as usual
}The fixed keyword will pin any managed objects (like a byte[]) only for the amount of time the fixed statement is in scope. Once it goes out of scope, the managed object will be unpinned, and can be moved by the GC freely.
Another advantage of using a Memory<byte> instead, is that ByteBuffer can have a ToMemory method, like it has ToSpan and ToArraySegment methods. In certain situations it is necessary to get a Memory<T> instead of a Span<T>. For example, if you are using async code, you cannot use a Span<T>. A concrete example of this scenario is in the Apache Arrow C# library, where it is attempting to write to a Stream object. In order to modify this code so it can handle reading and writing to memory other than byte[], it will need a ToMemory method on ByteBuffer.