Add async methods for creating Reader object #44
Conversation
MaxMind.Db/ArrayBuffer.cs
Outdated
| byte[] bytes; | ||
|
|
||
| // If the stream is within "int" max size, we can read it at once without requiring MemoryStream | ||
| if (stream.CanSeek && stream.Length <= int.MaxValue) |
There was a problem hiding this comment.
These checks need to be removed or updated. We aren't seeking any stream positions and calling stream.Length I believe will go out and buffer entire stream contents to get the length (it isn't cheap). If we are going to do this, we should see if the stream is a MemoryStream first :)
There was a problem hiding this comment.
This branch was added as part of #38 - I added the additional check for bytesRead != stream.Length on lines 57 and 97 because Read/ReadAsync aren't guaranteed to fill the buffer with the requested number of bytes (I guess the likelihood of that depends a lot on the type of Stream one passes in).
I'm happy to just use the MemoryStream implementation, but I'm curious to hear @maartenba's thoughts on the matter.
Potentially replacing the ToArray call with TryGetBuffer is a good idea.
There was a problem hiding this comment.
Yeah, I'm just saying calling length is not a good idea on any stream (minus in memory) as it will cause the whole stream to be consumed to get the length.
…yStream implementation
|
LGTM |
|
Thanks! This looks good to me. I'd also be curious to hear @maartenba's thoughts on this. |
|
Thanks! I merged this from the command line. I apologize for leaving this open for so long. |
Implements #11