Skip to content
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

Add a new stream buffer with an adaptative size #263

Open
kompass opened this issue Oct 2, 2019 · 3 comments
Open

Add a new stream buffer with an adaptative size #263

kompass opened this issue Oct 2, 2019 · 3 comments

Comments

@kompass
Copy link

kompass commented Oct 2, 2019

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
Copy link
Author

kompass commented Oct 3, 2019

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.

@Marwes
Copy link
Owner

Marwes commented Oct 7, 2019

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`

@kompass
Copy link
Author

kompass commented Oct 16, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants