C# support for directly reading and writing to memory other than byte[] #4886
Conversation
net/FlatBuffers/FlatBufferBuilder.cs
Outdated
| public void Add<T>(Span<T> x) | ||
| where T : struct | ||
| { | ||
| if (x.Length == 0) |
There was a problem hiding this comment.
Is this worth special casing? If it is not a common occurrences, just letting Prep/Put do nothing may on average be faster and clearer.
There was a problem hiding this comment.
I removed the check.
| // Get the data of a vector whoses offset is stored at "offset" in this object as an | ||
| // Spant<byte>. If the vector is not present in the ByteBuffer, | ||
| // then an empty span will be returned. | ||
| public Span<byte> __vector_as_span(int offset) |
There was a problem hiding this comment.
this is pretty awesome.
| #else | ||
| // Get the data of a vector whoses offset is stored at "offset" in this object as an | ||
| // ArraySegment<byte>. If the vector is not present in the ByteBuffer, | ||
| // then a null value will be returned. |
There was a problem hiding this comment.
Should this function maybe always be available, for backwards compatibility?
There was a problem hiding this comment.
actually never mind, the user has to explicitly enable ENABLE_SPAN_T so it is ok
There was a problem hiding this comment.
the user has to explicitly enable ENABLE_SPAN_T
Given that this is a preprocessor directive, how would this surface to the user?
There was a problem hiding this comment.
You will have to build the library with ENABLE_SPAN_T defined in much the same way to need to build the library with UNSAFE_BYTEBUFFER defined to get the performance benefits.
| } | ||
|
|
||
|
|
||
| #if ENABLE_SPAN_T |
There was a problem hiding this comment.
If C# supports it, can we have these directives indented, here and elsewhere?
There was a problem hiding this comment.
I can indent them, but it seems like this would be inconsistent with the rest of the #ifdef's in the project. Do you want me to indent every #ifdef?
There was a problem hiding this comment.
Ah I guess leave it for now then.
| string filename = @"Resources/monsterdata_cstest" + (sizePrefix ? "_sp" : "") + ".mon"; | ||
| File.WriteAllBytes(filename, data); | ||
| #else | ||
| // Dump to output directory so we can inspect later, if needed |
There was a problem hiding this comment.
move comment out of #if
…e[]. For example, ByteBuffer can be initialized with a custom allocator which uses shared memory / memory mapped files. Public access to the backing buffer uses Span<T> instead of ArraySegment<T>. Writing to the buffer now supports Span<T> in addition to T[]. To maintain backwards compatibility ENABLE_SPAN_T must be defined.
|
Thanks! |
…e[]. For example, ByteBuffer can be initialized with a custom allocator which uses shared memory / memory mapped files. (google#4886) Public access to the backing buffer uses Span<T> instead of ArraySegment<T>. Writing to the buffer now supports Span<T> in addition to T[]. To maintain backwards compatibility ENABLE_SPAN_T must be defined.
3 participants
C# support for directly reading and writing to memory other than byte[].
For example, ByteBuffer can be initialized with a custom allocator which uses shared memory / memory mapped files.
Public access to the backing buffer uses Span instead of ArraySegment.
Writing to the buffer now supports Span in addition to T[].
To maintain backwards compatibility ENABLE_SPAN_T must be defined.
Maintaining compatibility was not easy and I am not sure how much to use #ifdef in the code. Let me know what you think of my approach. I was not to happy having to add an #ifdef to the generated C# code but I could not think of another way.
All of the automated tests pass with and without ENABLE_SPAN_T defined and with and without UNSAFE_BYTEBUFFER. I have tested it within the product that I am working on as well and it all seems to be working correctly.