Tobias, Michael,

I did a few rounds of iteration on this, that address Tobias’s comments ( 
modulo the variable buffer sizing, which we agreed to defer for now unless 
there is a compelling use case ). With this change, there is zero-copy of data. 
The composite container, List, is relatively lightweight ( and we need some 
type for composition anyway ( no need to invent something new )).

I pushed this version to the sandbox, where can get further experience with it 



hg clone
cd sandbox
bash common/bin/ -s update http-client-branch
# configure, make, etc

> On 4 Aug 2017, at 15:05, Michael McMahon <> wrote:
> Hi Tobias,
> On 04/08/2017, 10:22, Tobias Thierer wrote:
>> Hi Michael -
>> thanks for your work! The fact that BufferingProcessor wraps another 
>> BodyProcessor looks like it could be quite useful/flexible.
>> Some things I noticed upon a quick glance:
>>      • BufferingProcessor's documentation cuts out mid-sentence.
> That could be just how the unformatted text looks. I've uploaded the HTML 
> apidoc
> for the two interfaces to:
> which should be easier to read. Links won't work but you can just search for 
> the buffering() method in both docs.
>>      • BufferingProcessor passes modifiable LinkedList instances. Why? Do 
>> you want to guarantee that remove-from-front is efficient? If not, consider 
>> singleton and array-based unmodifiable instances since they tend to be 
>> faster, more memory efficient, and don't risk applications starting to 
>> accidentally rely on the lists being modifiable? 
> That seems like a good idea.
>>      • Unsurprisingly, getBuffersOf() performs a copy via getNBytesFrom() - 
>> just a thing to be aware of, but I don't expect it can be avoided.
>>      • The buffer size is fixed, so one can't change the buffer size 
>> dynamically (e.g. in response to bitrate changes in a video player app) or 
>> even make the first buffer a different size than the later ones (e.g. file 
>> header vs. later chunks). I don't know a good solution to the latter either, 
>> since Flow.request(long) documents that the long refers to the number of 
>> calls; the former looks like it could be made more flexible in a future 
>> version of BufferingProcessor, but is probably okay for now.
> At the very least it would require defining a new subtype of 
> HttpResponse.BufferingProcessor and it complicates the Flow semantics
> a bit, because you could be changing the buffersize while there is 
> outstanding demand at the old buffersize. But, it could be usable
> if you stick to doing request(1) all the time. I think we should defer on 
> that for now though.
>>      • So far it looks like the new code will interoperate with blocking 
>> facades such as PipedResponseStream, which is nice. I hope to verify this 
>> empirically next week.
>> I intend to write more comments later, just wanted to send a quick first 
>> impression. Thanks for the promising update!
> Great thanks for the first impression.
> Michael
>> Tobias
>> On Thu, Aug 3, 2017 at 10:02 AM, Michael McMahon 
>> <> wrote:
>> Hi,
>> The HTTP client work is continuing in a new branch of the JDK 10 sandbox 
>> forest (http-client-branch),
>> and here is the first of a number of changes we want to make.
>> This one is to address the feedback we received where 
>> HttpResponse.BodyProcessors would
>> be easier to implement if there was control over the size of buffers being 
>> supplied.
>> To that end we have added APIs for creating buffered response processors 
>> (and handlers)
>> So, HttpResponse.BodyProcessor has a new static method with the following 
>> signature
>> public static <T> BodyProcessor<T> buffering(BodyProcessor<T> downstream, 
>> long buffersize) {}
>> This returns a new processor which delivers data to the supplied downstream 
>> processor, buffered
>> by the 'buffersize' parameter. It guarantees that all data is delivered in 
>> chunks of that size
>> until the final chunk, which may be smaller.
>> This should allow other BodyProcessor implementations that require buffering 
>> to wrap themselves
>> in this way, be guaranteed that the data they receive is buffered, and then 
>> return that composite
>> processor to their user.
>> A similar method is added to HttpResponse.BodyHandler.
>> Note also, that we have changed HttpResponse.BodyProcessor from being a 
>> Flow.Subscriber<ByteBuffer>
>> to Flow.Subscriber(List<ByteBuffer>). That change is technically orthogonal 
>> to this one, but is motivated
>> by it. By transferring ByteBuffers in lists makes it easier to buffer them 
>> efficiently.
>> The webrev is at:
>> Thanks,
>> Michael

Reply via email to