combine icon indicating copy to clipboard operation
combine copied to clipboard

Add a new stream buffer with an adaptative size

Open kompass opened this issue 6 years ago • 3 comments

Hello !

The combination of BufferedStream, ReadStream and BufReader is really great to read data from streaming stdin or else (eg. network input). But BufferedStream has a fixed size and it's complicated to find the good one when we don't know the content we will receive.

So I developed a new Stream, named ElasticBufferedReadStream (I'm not good at finding short names) combining the features of BufferedStream, ReadStream and BufReader and adapting its size according to the checkpoints still owned by the parsers. It uses Rc and Weak to track the checkpoints lifetimes.

You can see my project here : https://github.com/kompass/combine-elastic-buffered-stream

It's still under development, I will add the RangeStream feature and continue to test everything to be sure it's stable. What dou you think about it ? Maybe we can add this stream into your project ?

kompass avatar Oct 02 '19 09:10 kompass

I managed to implement uncons_range as a standalone method, but I can't implement RangeStreamOnce for now because of a lifetime mess with trait implementations and references lifetimes.

It need the associated type Range to have a lifetime associated with the stream's one, but the way you've implemented the traits make it impossible. If you're interrested I can suggest you a modification.

kompass avatar Oct 03 '19 13:10 kompass

Haven't looked at the code thoroughly yet but the gist of it seems like it would make a good addition!

It need the associated type Range to have a lifetime associated with the stream's one, but the way you've implemented the traits make it impossible. If you're interrested I can suggest you a modification.

Guessing it is some variation of http://lukaskalbertodt.github.io/2018/08/03/solving-the-generalized-streaming-iterator-problem-without-gats.html ? Even with one of those techniques, a streaming iterator (streaming combine stream?) is not the right fit as it would prevent parsing from continue while a token/range is still alive.

let t = stream.uncons();
let t2 = stream.uncons(); // Error, can't mutate `stream` since a reference to it is in `t`

Marwes avatar Oct 07 '19 10:10 Marwes

You're right about streaming RangeStream, it's not a good idea ! I changed my mind, I will not implement RangeStream. But for a simple streaming combine stream, it works perfectly since the tokens are passed by value.

I will test it further and when I think it's okay I create a Pull Request.

kompass avatar Oct 16 '19 11:10 kompass