-
Notifications
You must be signed in to change notification settings - Fork 51
Faster ChunkedStreamReader. #182
Faster ChunkedStreamReader. #182
Conversation
* Add an internal `_offset` to track offset in `_buffer`, reducing the number of times we need to create a sublist internally. * Specialize to handle cases where `_buffer` is a `Uint8List` by creating a `Uint8List.sublistView` when we need to split a chunk. Fixes #181.
lrhn
left a comment
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 buffering is fine, the specialization seems half-baked. I'd do it with a specialized class for Uint8List streams.
| if (_buffer.length - _offset > 0) { | ||
| if (size < _buffer.length - _offset) { | ||
| late List<T> output; | ||
| if (_buffer is Uint8List) { |
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.
I'd prefer a specialized ChunkedByteStreamReader with special code for Uint8List everywhere.
The readChunk(size) above is not particularly efficient when it dumps all these Uint8Lists into a plain List<int>.
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.
That's why we have ChunkedStreamReaderByteStreamExt, but yes, in hindsight... maybe we should only have made ChunkedByteStreamReader -- because in practice most other streams aren't chunked.
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 ones that are ... are probably Stream<String>, not list based. So yes.
|
I wish we had benchmarks for these types of things. 🤷 |
| if (_buffer.length - _offset > 0) { | ||
| if (size < _buffer.length - _offset) { | ||
| late List<T> output; | ||
| if (_buffer is Uint8List) { |
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 ones that are ... are probably Stream<String>, not list based. So yes.
* Faster ChunkedStreamReader. * Add an internal `_offset` to track offset in `_buffer`, reducing the number of times we need to create a sublist internally. * Specialize to handle cases where `_buffer` is a `Uint8List` by creating a `Uint8List.sublistView` when we need to split a chunk.
_offsetto track offset in_buffer, reducing thenumber of times we need to create a sublist internally.
_bufferis aUint8Listbycreating a
Uint8List.sublistViewwhen we need to split a chunk.Fixes dart-lang/core#343.