2.x.x - Wrap InboundStream iterator in request body #369
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## 2.x.x #369 +/- ##
==========================================
+ Coverage 84.07% 84.44% +0.36%
==========================================
Files 93 95 +2
Lines 5099 5207 +108
==========================================
+ Hits 4287 4397 +110
+ Misses 812 810 -2 ☔ View full report in Codecov by Sentry. |
|
Pull request benchmark comparison [ubuntu-latest] with '2.x.x' run at 2024-02-05T09:42:38+00:00 |
| public struct HBStreamedRequestBody: Sendable, AsyncSequence { | ||
| public typealias Element = ByteBuffer | ||
| public typealias InboundStream = NIOAsyncChannelInboundStream<HTTPRequestPart> |
There was a problem hiding this comment.
We should prevent exposing this type, as we're locking ourselves in a corner
There was a problem hiding this comment.
At some point we are going to have to expose a public way to create a HBRequestBody that is initialised with a stream of data and at that time we'll have to expose the internal type to avoid having to do type conversions.
adam-fowler
force-pushed
the
2.x.x-requestbody
branch
from
192fea7 to
384e35d
Compare
…te of HBStreamedRequestBody
adam-fowler
force-pushed
the
2.x.x-requestbody
branch
from
384e35d to
0dca77a
Compare
* Use AnyAsyncSequence to hold request body async sequence * Make contents of HBRequestBody internal * Remove HBRequestBody.collate(maxSize) as we can use `collect` now
Wrap NIOAsyncChannelInboundStream.Iterator in request body in another AsyncSequence. This removes the creation of an AsyncChannel and gets rid of a TaskGroup. Performance improvement is about 12%.
Issues:
AnyAsyncSequencefor the request body.HBStreamedRequestBodya reference type and writing back into it that the sequence is done once the first iteration is done and not allow another iteration. This complicated the flush request body code at the end of the loop.EDIT:
HBStreamedRequestBodya reference type so we can record if an iterator has already been created from it. This resolves my second issue. This reduces the performance gain to about 10-11%ByteBufferback to AsyncSequence using themakeTestingStream. I know this uses AsyncStream internally which has back pressure issues but I only stick one ByteBuffer on it and then finish it.cc @FranzBusch