You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Holger Sunke <ho...@posteo.de> on 2018/03/15 23:10:11 UTC
Bug 62177 - Servlet 4.0 ServerPush
Hello,
thank you for your quick patches on #62177. I have tested a lot and
found that it now works much better.
During my tests I found there are still unstabilities / issues that
would prevent me from productive use:
- While testing with [localhost], everything is fine. But when loding
the PrimeFaces showcase through a slow internet connection, using
SSL+HTTP/1.1 seemes a lot faster than HTTP2 with serverPush. I also
again get write attempts to closed streams in server log in this case.
- The rendering of base HTML output is blocked by writing the
PUSH_PROMISE headers: CPU-sampling shows that the server spends most
time in Http2[Async]UpgradeHandler.writeHeaders(..) and .push(..).
- Maybe as expected, 'nested' resources don't get pushed at all, because
their PUSH_PROMISE would go into an illegal Stream what your patch
successfully prevents.
Wouldn't it be possible, to ..
- delegate PUSH_PROMISE writing + subsequent internal request and
StramRunnable creation to a "PushRunnable"
- hand over a reference to the real client initiated Stream into the
"PushRunnable"
- FIFO-queue "PushRunnable" instances for execution in another Thread
such that the application does not get blocked for writing the
PUSH_PROMISEs and maybe even nested resources getting pushed?
Or is it that accessing the inital client initiated Stream from
nested-resource pushing Thread is absolutly unsafe / wrong operation?
Kind regards
Holger Sunke
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Bug 62177 - Servlet 4.0 ServerPush
Posted by Rémy Maucherat <re...@apache.org>.
On Fri, Mar 16, 2018 at 12:30 AM, Mark Thomas <ma...@apache.org> wrote:
> On 15/03/18 23:10, Holger Sunke wrote:
> > Hello,
> >
> > thank you for your quick patches on #62177. I have tested a lot and
> > found that it now works much better.
> >
> > During my tests I found there are still unstabilities / issues that
> > would prevent me from productive use:
> >
> > - While testing with [localhost], everything is fine. But when loding
> > the PrimeFaces showcase through a slow internet connection, using
> > SSL+HTTP/1.1 seemes a lot faster than HTTP2 with serverPush.
>
> That would need more research into the why.
>
> > I also
> > again get write attempts to closed streams in server log in this case.
>
> That is probably server push. The server pushes and starts to write but
> the client closes the stream because it has a valid resource in the
> cache. Tomcat has no way to tell the Servlet to stop writing so all that
> effort is wasted.
>
> > - The rendering of base HTML output is blocked by writing the
> > PUSH_PROMISE headers: CPU-sampling shows that the server spends most
> > time in Http2[Async]UpgradeHandler.writeHeaders(..) and .push(..).
>
> Because output is to a single socket from multiple threads it has to be
> synchronized.
>
I agree it's the likely performance killer. The kind of push (ab)use that
is done can be inefficient, that is a known possibility. The framework also
has to figure out what is the best push strategy (IMO).
Also bad is that the fix required taking out a good sync optimization in
Http2AsyncUpgradeHandler, so it contributes to the problem.
>
> > - Maybe as expected, 'nested' resources don't get pushed at all, because
> > their PUSH_PROMISE would go into an illegal Stream what your patch
> > successfully prevents.
>
> Yes, this is expected given the current implementation.
>
> > Wouldn't it be possible, to ..
> >
> > - delegate PUSH_PROMISE writing + subsequent internal request and
> > StramRunnable creation to a "PushRunnable"
> >
> > - hand over a reference to the real client initiated Stream into the
> > "PushRunnable"
> >
> > - FIFO-queue "PushRunnable" instances for execution in another Thread
> >
> > such that the application does not get blocked for writing the
> > PUSH_PROMISEs and maybe even nested resources getting pushed?
>
> See the comment above about multiple threads writing to a single socket.
>
> There are also requirements in HTTP/2 around ordering and keeping
> certain frames together which means there isn't a simple way to do this.
>
> To get an idea of how complex this can get, have a look at the code
> around writing to the response. The co-ordination of one thread writing
> per stream, per stream window sizes, a connection window size and all
> network writes going to a single socket make for some very 'interesting'
> code.
>
I kind of agree with his idea of serializing, as this is usually the way to
solve this sort of issue (vs contending on IO), but I'm not sure how well
it would work though.
>
> > Or is it that accessing the inital client initiated Stream from
> > nested-resource pushing Thread is absolutly unsafe / wrong operation?
>
> There are concurrency issues. You can only push on an open, client
> initiated stream. For a nested push you'd need to make sure that the
> initial stream was still open. There might be a way to do this. It needs
> some thought.
>
I thought the HTTP/2 code was now "done", but looks like there's a lot more
unexpected work left.
Rémy
Re: Bug 62177 - Servlet 4.0 ServerPush
Posted by Mark Thomas <ma...@apache.org>.
On 15/03/18 23:10, Holger Sunke wrote:
> Hello,
>
> thank you for your quick patches on #62177. I have tested a lot and
> found that it now works much better.
>
> During my tests I found there are still unstabilities / issues that
> would prevent me from productive use:
>
> - While testing with [localhost], everything is fine. But when loding
> the PrimeFaces showcase through a slow internet connection, using
> SSL+HTTP/1.1 seemes a lot faster than HTTP2 with serverPush.
That would need more research into the why.
> I also
> again get write attempts to closed streams in server log in this case.
That is probably server push. The server pushes and starts to write but
the client closes the stream because it has a valid resource in the
cache. Tomcat has no way to tell the Servlet to stop writing so all that
effort is wasted.
> - The rendering of base HTML output is blocked by writing the
> PUSH_PROMISE headers: CPU-sampling shows that the server spends most
> time in Http2[Async]UpgradeHandler.writeHeaders(..) and .push(..).
Because output is to a single socket from multiple threads it has to be
synchronized.
> - Maybe as expected, 'nested' resources don't get pushed at all, because
> their PUSH_PROMISE would go into an illegal Stream what your patch
> successfully prevents.
Yes, this is expected given the current implementation.
> Wouldn't it be possible, to ..
>
> - delegate PUSH_PROMISE writing + subsequent internal request and
> StramRunnable creation to a "PushRunnable"
>
> - hand over a reference to the real client initiated Stream into the
> "PushRunnable"
>
> - FIFO-queue "PushRunnable" instances for execution in another Thread
>
> such that the application does not get blocked for writing the
> PUSH_PROMISEs and maybe even nested resources getting pushed?
See the comment above about multiple threads writing to a single socket.
There are also requirements in HTTP/2 around ordering and keeping
certain frames together which means there isn't a simple way to do this.
To get an idea of how complex this can get, have a look at the code
around writing to the response. The co-ordination of one thread writing
per stream, per stream window sizes, a connection window size and all
network writes going to a single socket make for some very 'interesting'
code.
> Or is it that accessing the inital client initiated Stream from
> nested-resource pushing Thread is absolutly unsafe / wrong operation?
There are concurrency issues. You can only push on an open, client
initiated stream. For a nested push you'd need to make sure that the
initial stream was still open. There might be a way to do this. It needs
some thought.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org