You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Masaori Koshiba <ma...@apache.org> on 2017/11/28 01:52:04 UTC

Avoid copy data in Http2Stream::update_write_request()

Hi HTTP/2 hackers,

I’m fighting with #2795. And I’m really confused by
`Http2Stream::update_write_request()`.
Why do we need to copy over data from `write_vio` to `response_buffer`?
(
https://github.com/apache/trafficserver/blob/master/proxy/http2/Http2Stream.cc#L538-L556
 )
It looks like this makes things really complicated. I'd like to avoid this
copy.

Backgrounds:
The root cause of #2795 is described on #2837. The solution looks making
`write_vio.ndone` accurate.
If we can avoid the copy, `Http2Stream::update_write_request()` will be
simple and we can easily fix the bug.

- https://github.com/apache/trafficserver/issues/2795
- https://github.com/apache/trafficserver/pull/2837

Thanks,
Masaori

Re: Avoid copy data in Http2Stream::update_write_request()

Posted by Masaori Koshiba <ma...@apache.org>.
All right, I'll try to get rid of the copy. Thanks!

- Masaori

2017年11月28日(火) 11:26 shinrich@ieee.org <sh...@ieee.org>:

> Masori,
>
> We probably don't need to copy.  The copy is just moving blocks between
> lists but I agree that it adds complexity.
>
> At the time I was getting rid of the PluginVC wrapper I was trying to
> understand both sides of the do_io operations.  So things probably ended up
> more complex than they needed to be.
>
> I think you should be able to take the vio from the SM and feed it into
> the Http2ConnectionState operations.  At least it is worth a try.
>
> Susan
>
> Sent from Yahoo Mail on Android
> <https://overview.mail.yahoo.com/mobile/?.src=Android>
>
> On Mon, Nov 27, 2017 at 7:52 PM, Masaori Koshiba
> <ma...@apache.org> wrote:
> Hi HTTP/2 hackers,
>
> I’m fighting with #2795. And I’m really confused by
> `Http2Stream::update_write_request()`.
> Why do we need to copy over data from `write_vio` to `response_buffer`?
> (
>
> https://github.com/apache/trafficserver/blob/master/proxy/http2/Http2Stream.cc#L538-L556
> )
> It looks like this makes things really complicated. I'd like to avoid this
> copy.
>
> Backgrounds:
> The root cause of #2795 is described on #2837. The solution looks making
> `write_vio.ndone` accurate.
> If we can avoid the copy, `Http2Stream::update_write_request()` will be
> simple and we can easily fix the bug.
>
> - https://github.com/apache/trafficserver/issues/2795
> - https://github.com/apache/trafficserver/pull/2837
>
> Thanks,
> Masaori
>
>

Re: Avoid copy data in Http2Stream::update_write_request()

Posted by "shinrich@ieee.org" <sh...@ieee.org>.
Masori, 
We probably don't need to copy.  The copy is just moving blocks between lists but I agree that it adds complexity.
At the time I was getting rid of the PluginVC wrapper I was trying to understand both sides of the do_io operations.  So things probably ended up more complex than they needed to be.
I think you should be able to take the vio from the SM and feed it into the Http2ConnectionState operations.  At least it is worth a try.
Susan

Sent from Yahoo Mail on Android 
 
  On Mon, Nov 27, 2017 at 7:52 PM, Masaori Koshiba<ma...@apache.org> wrote:   Hi HTTP/2 hackers,

I’m fighting with #2795. And I’m really confused by
`Http2Stream::update_write_request()`.
Why do we need to copy over data from `write_vio` to `response_buffer`?
(
https://github.com/apache/trafficserver/blob/master/proxy/http2/Http2Stream.cc#L538-L556
 )
It looks like this makes things really complicated. I'd like to avoid this
copy.

Backgrounds:
The root cause of #2795 is described on #2837. The solution looks making
`write_vio.ndone` accurate.
If we can avoid the copy, `Http2Stream::update_write_request()` will be
simple and we can easily fix the bug.

- https://github.com/apache/trafficserver/issues/2795
- https://github.com/apache/trafficserver/pull/2837

Thanks,
Masaori