chore: update ChunkSegmenter to optionally allow a limit on the number of bytes it should consume - #3279
Conversation
Sorry, something went wrong.
| long overallRemaining = totalRemaining - consumedSoFar; | ||
| if (overallRemaining < blockSize && currentBlockPending == blockSize) { | ||
| break; | ||
| break outerloop; |
There was a problem hiding this comment.
ooo, label break, those're rare!
Sorry, something went wrong.
| } | ||
| if (numBytesConsumable <= 0) { | ||
| continue; | ||
| break outerloop; |
There was a problem hiding this comment.
K...now I'm starting to worry a bit about cyclomatic complexity
Sorry, something went wrong.
There was a problem hiding this comment.
Prior to the appendable upload stuff we generally had a pretty firm bulkhead on the number of buffers that would be passed into these methods outside of tests. With the appendable addition we place less emphasis on early buffering in favor of passing things through wherever possible, so if multiple buffers are passed in here, logically the conditions would prevent consuming any bytes once a break from the while takes place, but by breaking the for as well we avoid the cycles performing work that isn't productive.
And, refactoring everything to nested method calls to allow early returns instead of break to label didn't seem worth it to me.
Sorry, something went wrong.
There was a problem hiding this comment.
What's the follow up here?
Sorry, something went wrong.
There was a problem hiding this comment.
Nothing from my perspective. There isn't a functional difference here between the label break and a separate method with early returns.
Sorry, something went wrong.
…r of bytes it should consume Update BidiAppendableUnbufferedWritableByteChannel to only attempt to consume as many bytes as are available according to the stream -- this prevents over packing of segments we will for sure never be able to use.
"Child" change after #3278
Update BidiAppendableUnbufferedWritableByteChannel to only attempt to consume as many bytes as are available according to the stream -- this prevents over packing of segments we will for sure never be able to use.
With both this PR and the contents of #3278 our latency now looks like the following, where the v2.56.0 line represents the workload running using version 2.56.0 and v2.56.1+bw.2025_09_04_1715 represents the same workload bu using a version build from this PR.
