msgpack-cli icon indicating copy to clipboard operation
msgpack-cli copied to clipboard

Consider adding ArraySegment support to ByteArrayUnpacker

Open davidfowl opened this issue 6 years ago • 4 comments

Today the ByteArrayUnpacker supports passing in a byte[] or byte[] and startOffset but it doesn't support limiting the unpacker's view of the array (e.g. there's no way to pass in a count). This works fine if I'm reading elements in a specific order and never fear going into other data that might be in the array. But, if I wanted to write a generic reader loop over the unpacker, I could end up reading other data from the array which was never intended for the Unpacker.

Ideally MsgPack-Cli would support passing an ArraySegment<byte> and eventually a Memory<byte>/Span<byte> (new in .NET but not RTM yet) so we could limit the view of the array from the unpacker.

davidfowl avatar Mar 22 '18 09:03 davidfowl

Thank you for feedback!

I have two question about this:

  1. Is ArraySegment<byte> required even when Memory<byte> and Span<byte> are released?
  2. In fact, I removed ArraySegment<byte> based implementation in a99bfed4004bf4b5080ade5ff33b158dab2b3dc2 because it hurt performance. I guess it was occurred by size of ArraySegment<byte> struct. Do you have any idea?

yfakariya avatar Mar 26 '18 14:03 yfakariya

Is ArraySegment required even when Memory and Span are released?

No, Memory<byte> or Span<byte> would be better (when they are released).

In fact, I removed ArraySegment based implementation in a99bfed because it hurt performance. I guess it was occurred by size of ArraySegment struct. Do you have any idea?

It could have been struct copies, you could try using the C# in syntax (C# 7.2 or 7.3?) to reduce those copies. Alternatively, you can add an int count parameter to the array overload and it would suffice until those other types are widely available.

davidfowl avatar Mar 26 '18 14:03 davidfowl

I also need that overload. In my application I have several buffer but I always need to create a copy from some ArraySegments using ToArray which is allocating useless memory.

I also tried to use the Unpacker with the original array and offset 0. But the Unpacker is not able to deserialize the object (returns null). Maybe because the buffer is 4k and there is no length property available!?

chkr1011 avatar Apr 23 '19 13:04 chkr1011

I see, self managed buffer scenario is absolutely good point to start.

yfakariya avatar Apr 23 '19 22:04 yfakariya