RFR: 8192966 HttpClient should reuse TCP connection for h2c connections

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8192966 HttpClient should reuse TCP connection for h2c connections

Michael McMahon
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192966 HttpClient should reuse TCP connection for h2c connections

Daniel Fuchs
Hi Michael,

I wonder whether Http2Connection::closeStream would be
a better place to call Stream::checkConnectionClosure?
If not then shouldn't it be called in Stream::release
as well?

best regards,

-- daniel

On 12/12/2017 10:07, Michael McMahon wrote:
>
> http://cr.openjdk.java.net/~michaelm/8192966/webrev.1/
>
> Thanks,
> Michael.

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192966 HttpClient should reuse TCP connection for h2c connections

Michael McMahon
Thanks Daniel. I'm looking at this change again as I have
noticed the behavior when multiple https requests are initiated
might not be what we want. I'll post another webrev later.

- Michael

On 12/12/2017, 10:32, Daniel Fuchs wrote:

> Hi Michael,
>
> I wonder whether Http2Connection::closeStream would be
> a better place to call Stream::checkConnectionClosure?
> If not then shouldn't it be called in Stream::release
> as well?
>
> best regards,
>
> -- daniel
>
> On 12/12/2017 10:07, Michael McMahon wrote:
>>
>> http://cr.openjdk.java.net/~michaelm/8192966/webrev.1/
>>
>> Thanks,
>> Michael.
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192966 HttpClient should reuse TCP connection for h2c connections [jdk10]

Michael McMahon
This is follow on from a previous review of the same bug fix. The fix
is now targeted specifically for jdk10.

A slightly different approach is taken this time. But, your comment
below is incorporated into the new version Daniel

Webrev: http://cr.openjdk.java.net/~michaelm/8192966/webrev.2/

Thanks
Michael

On 12/12/2017, 13:32, Michael McMahon wrote:

> Thanks Daniel. I'm looking at this change again as I have
> noticed the behavior when multiple https requests are initiated
> might not be what we want. I'll post another webrev later.
>
> - Michael
>
> On 12/12/2017, 10:32, Daniel Fuchs wrote:
>> Hi Michael,
>>
>> I wonder whether Http2Connection::closeStream would be
>> a better place to call Stream::checkConnectionClosure?
>> If not then shouldn't it be called in Stream::release
>> as well?
>>
>> best regards,
>>
>> -- daniel
>>
>> On 12/12/2017 10:07, Michael McMahon wrote:
>>>
>>> http://cr.openjdk.java.net/~michaelm/8192966/webrev.1/
>>>
>>> Thanks,
>>> Michael.
>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8192966 HttpClient should reuse TCP connection for h2c connections [jdk10]

Daniel Fuchs
Hi Michael,

On 19/12/2017 14:20, Michael McMahon wrote:
> This is follow on from a previous review of the same bug fix. The fix
> is now targeted specifically for jdk10.
>
> A slightly different approach is taken this time. But, your comment
> below is incorporated into the new version Daniel
>
> Webrev: http://cr.openjdk.java.net/~michaelm/8192966/webrev.2/

This looks much better to me!

best regards,

-- daniel

>
> Thanks
> Michael
>
> On 12/12/2017, 13:32, Michael McMahon wrote:
>> Thanks Daniel. I'm looking at this change again as I have
>> noticed the behavior when multiple https requests are initiated
>> might not be what we want. I'll post another webrev later.
>>
>> - Michael
>>
>> On 12/12/2017, 10:32, Daniel Fuchs wrote:
>>> Hi Michael,
>>>
>>> I wonder whether Http2Connection::closeStream would be
>>> a better place to call Stream::checkConnectionClosure?
>>> If not then shouldn't it be called in Stream::release
>>> as well?
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>> On 12/12/2017 10:07, Michael McMahon wrote:
>>>>
>>>> http://cr.openjdk.java.net/~michaelm/8192966/webrev.1/
>>>>
>>>> Thanks,
>>>> Michael.
>>>