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

Drop AsyncWrite usage in h3-quinn once GATs land #55

Closed
stammw opened this issue Sep 30, 2021 · 3 comments
Closed

Drop AsyncWrite usage in h3-quinn once GATs land #55

stammw opened this issue Sep 30, 2021 · 3 comments
Labels
C-performance Category: performance. This is making existing behavior go faster. C-refactor Category: refactor. This would improve the clarity of internal code. E-medium Effort: medium. Some knowledge of how the internals work would be useful.

Comments

@stammw
Copy link
Contributor

stammw commented Sep 30, 2021

We were forced to use AsyncWrite in h3_quinn::SendStream because Quinn only offers lifetime-bound Futures to poll for writes:
Write<'a, S>.

Generic associated type could cancel this limitation, allowing to store them into h3_quinn::SendStream.

This will have positive implication for performance and will remove the necessity to Pin the stream and to downcast the error type from IOError to quinn::WriteError.

@stammw stammw added E-medium Effort: medium. Some knowledge of how the internals work would be useful. C-performance Category: performance. This is making existing behavior go faster. C-refactor Category: refactor. This would improve the clarity of internal code. labels Sep 30, 2021
@Ralith
Copy link

Ralith commented Jan 1, 2022

quinn could also be modified to expose raw poll_* style methods alongside important futures, if a better approach is required in the mean time. That's assuming it's not practical to modify h3 to push the pinning/lifetime issues downstream.

@stammw
Copy link
Contributor Author

stammw commented Jan 3, 2022

I remember that I thought of using the poll_* methods, but can't recall why I didn't ask for them to be made available.
I guess this solution will cancel the need of GATs whatsoever.

@inflation
Copy link

This should be closed after quinn 0.9.0 upgrade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-performance Category: performance. This is making existing behavior go faster. C-refactor Category: refactor. This would improve the clarity of internal code. E-medium Effort: medium. Some knowledge of how the internals work would be useful.
Projects
None yet
Development

No branches or pull requests

4 participants