-
Notifications
You must be signed in to change notification settings - Fork 585
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
Sending an ArraySegment object #314
base: master
Are you sure you want to change the base?
Conversation
✅ Build Fleck 0.0.61-ci completed (commit ba7ddb68ff by @kuzux) |
1 similar comment
✅ Build Fleck 0.0.61-ci completed (commit ba7ddb68ff by @kuzux) |
Could you provide context beyond the new code? |
Here's the case where that would be useful: In a codebase using Fleck, I was creating a buffer, filling the buffer in native code and then sending that buffer. Something like
So, I want to send just a portion of that buffer (the part filled by the native code). To extract that part as a new byte array, the only "solution" I was able to find (to creating a smaller byte array from a larger one) was allocating a new buffer and copying the data to that (Causes latency spikes when working with buffers of large data). The way to do that is, I believe, using an |
And you’ve found a measurable improvement with this code? I ask because it still writes to a memory stream that gets written to an array. |
About measurable improvement: Well there was measurable (in fact noticeable) improvement when I eliminated other allocate & copy operations elsewhere in the code. So I decided to go on an allocation-hunting spree, essentially. This was the low-hanging fruit. I know about the memory stream allocation, but eliminating that as well would probably take way too much modification in the source, which I wasn't comfortable doing. After all, having 2 allocation & copy operations made a significant improvement over having 3. Having 1 would still be better than having 2 of them (Even if it's not as good as having 0 :D) |
src/Fleck/Handlers/Hybi13Handler.cs
Outdated
memoryStream.WriteByte((byte)payload.Count); | ||
} | ||
|
||
memoryStream.Write(payload.Array, 0, payload.Count); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should 0
be payload.Offset
so it uses the correct range from the Segment's array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, fixed that
|
||
public static byte[] FrameData(ArraySegment<byte> payload, FrameType frameType) | ||
{ | ||
var memoryStream = new MemoryStream(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another allocation that could be skipped is if the MemoryStream is initialized with a starting buffer then memoryStream.toArray
will just return the underlying array. To determine the buffer length, we'd just need to defer creating it until after calculating lengthBytes
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with that is, the returned array from MemoryStream.ToArray()
still has the length of the length of the underlying buffer (See https://replit.com/@kuzux/memorystreamarray#main.cs). The length of that array is then used by the SendBytes
function. If we want it to function correctly, we'll need to initialize the underlying buffer for each message.
Unless you were talking about the extra allocation that happens after writing a large amount of bytes to the stream.
✅ Build Fleck 0.0.62-ci completed (commit 4d644b9f28 by @kuzux) |
1 similar comment
✅ Build Fleck 0.0.62-ci completed (commit 4d644b9f28 by @kuzux) |
+1 for this, definitely a needed feature |
@kuzux Don't forget to add the |
If we are populating a very large buffer and want to send that via a websocket;
we need that buffer to be of "right" size. To doi that, we need to either create a new
byte[]
for each message, or truncate the large buffer to the message size. If we do the truncration thing for abyte[]
object, what we get is anArraySegment<byte>
object, not a newbyte[]
object. So, it should be possible to pass an ArraySegment instead of just a byte array for binary data.