NIO 2 connector

[Rémy Maucherat <re...@apache.org>]

Hi,

I've been working on porting a NIO2 connector that was originally developed
for JBoss AS by Nabil Benothman (an intern at Red Hat). Due to the very
different connector structure in Tomcat and my preference for basing it on
the existing NIO1 connector, it is mostly new code, though.

So I would now like to contribute this in trunk as a new experimental
connector. It should have feature parity with the NIO1 connectors. Of
course, while the basics are in, it will need some time to pass the
testsuite, fix issues, improve stability, etc.

Coyote version number could be moved up to 2.0 with this addition along
with all the connector refactorings that Mark did in trunk.

The code is there (rebased to the current trunk):
https://github.com/rmaucher/tomcat

A quick NIO2 (the API) presentation.

Pros:
- Significantly faster, although the API looks slower by design
- Resources friendly
- Seemingly trivial to use
 - Polling is neatly hidden away
- Thread pool is also neatly hidden away
- Per operation timeouts
- Read/Write is symmetric
- Trivial blocking IO

Cons:
- No real non blocking IO
- No concurrency allowed [for the socket impl of NIO2] although the API
looks concurrent
- Simplicity is sometimes misleading, the API doesn't provide some tools
for the needed synchronization, check pending operations and their possible
optimizations
- SSL is not integrated any better than with NIO1, it is still SSLEngine
which leads to creating the obligatory "AsynchronousSSLSocketChannel"
wrapper class yourself
- No real sendfile

Comments ?

Rémy

Re: NIO 2 connector

[jean-frederic clere <jf...@gmail.com>]

On 03/04/2014 07:26 PM, Mark Thomas wrote:
> Can you wait until we split 8.0.x from trunk or did you want to get this
> into 8.0.x?

I would like to see it in  8.0.x so we can have it in the talk Chris and 
I present at the ApacheCon.

Cheers

Jean-Frederic

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

Re: NIO 2 connector

[Rémy Maucherat <re...@apache.org>]

2014-03-04 21:51 GMT+01:00 Mark Thomas <ma...@apache.org>:

> >> Can you wait until we split 8.0.x from trunk or did you want to get this
> >> into 8.0.x?
> >>
> >
> > Depends, if you want to branch soon or not. It would have to be
> > experimental for a while anyway, but it will likely bring something
> useful.
>
> I would like to branch soon. On the other hand if we hold this back
> until 9.0.x then it will be a lot longer before folks can really use it.
>

It's very early in 8.0, so there would be plenty of time to backport if it
is not included before branching. I thought you backported everything
anyway. Basically, I can commit it as soon as there's general agreement on
it, it's very easy in the meantime to keep rebasing on trunk.

I'll try to work on improving the testsuite status as much as possible [I
see some websockets tests failing - not a big surprise -, although a lot of
it is working], and also see how I can remove that isReady from the
AbstractServletInputStream.

>
> > I think the spec says there should be a /0.0 version number, and I like
> the
> > Coyote name, but you can change it.
>
> I'll try to remember to take a look at it. Maybe just change it to match
> the Tomcat major/minor version numbers.
>

Ok.


> Thanks for the clarifications. I'm +1 with the following caveats:
> - no @author tags
> - exclude the change that makes NIO2 the default
> - document that the NIO2 connector is currently experimental
>
> - They came from the NIO1 classes I used as the basis for the new classes,
should I remove them anyway ?
- I changed that already.
- Yes, I copied from the current NIO documentation, but "experimental" is
not mentioned anywhere, which is obviously a mistake. I'll add a log during
the endpoint start, so that it can't be missed, it's the best way (nobody
reads the docs ...).

Rémy

Re: NIO 2 connector

[Mark Thomas <ma...@apache.org>]

On 04/03/2014 19:23, Rémy Maucherat wrote:
> 2014-03-04 19:26 GMT+01:00 Mark Thomas <ma...@apache.org>:
> 
>> On 04/03/2014 16:16, Rémy Maucherat wrote:
>>> Hi,
>>>
>>> I've been working on porting a NIO2 connector that was originally
>> developed
>>> for JBoss AS by Nabil Benothman (an intern at Red Hat). Due to the very
>>> different connector structure in Tomcat and my preference for basing it
>> on
>>> the existing NIO1 connector, it is mostly new code, though.
>>
>> There looks to be opportunities to share code with the current NIO
>> connector.
>>
> 
> Don't know. I used the structure, but the algorithms are quite different.
> You'll have the opportunity to do it if you want of course ;)

You are too kind :)

>> There are some changes to the existing code, such as
>> java/org/apache/coyote/http11/upgrade/AbstractServletInputStream.java
>> that I would like to understand.
>>
> 
> Nothing special, it needs a first read to start its "polling", so that's
> the reason for adding isReady() (normally harmless). I added the third
> event method too for consistency, and that's it.
> 
>>
>>> So I would now like to contribute this in trunk as a new experimental
>>> connector. It should have feature parity with the NIO1 connectors. Of
>>> course, while the basics are in, it will need some time to pass the
>>> testsuite, fix issues, improve stability, etc.
>>
>> Can you wait until we split 8.0.x from trunk or did you want to get this
>> into 8.0.x?
>>
> 
> Depends, if you want to branch soon or not. It would have to be
> experimental for a while anyway, but it will likely bring something useful.

I would like to branch soon. On the other hand if we hold this back
until 9.0.x then it will be a lot longer before folks can really use it.

>>> Coyote version number could be moved up to 2.0 with this addition along
>>> with all the connector refactorings that Mark did in trunk.
>>
>> I'd rather drop the version number entirely as it is pretty much
>> meaningless. If we are going to do that, we may as well change the
>> server header to "Apache Tomcat". I'm neutral on whether to include the
>> major or full version number.
>>
> 
> I think the spec says there should be a /0.0 version number, and I like the
> Coyote name, but you can change it.

I'll try to remember to take a look at it. Maybe just change it to match
the Tomcat major/minor version numbers.

>>> The code is there (rebased to the current trunk):
>>> https://github.com/rmaucher/tomcat
>>>
>>> A quick NIO2 (the API) presentation.
>>>
>>> Pros:
>>> - Significantly faster, although the API looks slower by design
>>> - Resources friendly
>>> - Seemingly trivial to use
>>>  - Polling is neatly hidden away
>>> - Thread pool is also neatly hidden away
>>> - Per operation timeouts
>>> - Read/Write is symmetric
>>> - Trivial blocking IO
>>>
>>> Cons:
>>> - No real non blocking IO
>>
>> Hmm. I was considering dropping the BIO connector entirely for Tomcat 9
>> because of its inability to do non-blocking IO and the way the Servlet
>> API was heading. Does it make sense to add a different non-blocking
>> connector implementation or have I misunderstood your point?
>>
> 
> Well, nobody is going for non blocking IO, people are using async IO.
> 
> But then obviously the read(ByteBuffer) call which used to return an int
> (possibly 0 with non blocking IO) now returns a Future<Integer>. And if you
> want to know the result you have to wait for it and there's an IO operation
> pending the entire time. So that's the "cons".
> 
> It would be nice to have non blocking operations for "peek" scenarios. So
> instead to peek a read, you have to read with a completion handler, then
> see if it returns inline. And if it doesn't there's an async process now
> doing IO (possibly not the greatest thing that could happen).
> 
>>
>>> - No concurrency allowed [for the socket impl of NIO2] although the API
>>> looks concurrent
>>
>> Do you mean no concurrent read and writes? That would be a huge issue.
>>
> 
> No, don't worry, that works. It's concurrency on the same operation (like
> read, write, accept). The API looks relatively magic, so you'd think you
> could do it.

OK. Thanks.

>>> - Simplicity is sometimes misleading, the API doesn't provide some tools
>>> for the needed synchronization, check pending operations and their
>> possible
>>> optimizations
>>> - SSL is not integrated any better than with NIO1, it is still SSLEngine
>>> which leads to creating the obligatory "AsynchronousSSLSocketChannel"
>>> wrapper class yourself
>>> - No real sendfile
>>>
>>> Comments ?
>>
>> The pros look nice but I am worried about the cons.
>>
>> The summary is that in the worst case, it's much better.

Thanks for the clarifications. I'm +1 with the following caveats:
- no @author tags
- exclude the change that makes NIO2 the default
- document that the NIO2 connector is currently experimental

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

Re: NIO 2 connector

[Christopher Schultz <ch...@christopherschultz.net>]

Rémy,

On 3/4/14, 2:23 PM, Rémy Maucherat wrote:
> 2014-03-04 19:26 GMT+01:00 Mark Thomas <ma...@apache.org>:
>>
>> Can you wait until we split 8.0.x from trunk or did you want to get this
>> into 8.0.x?
>>
> 
> Depends, if you want to branch soon or not. It would have to be
> experimental for a while anyway, but it will likely bring something useful.

+1 for putting this into Tomcat 8.0.x as an experimental connector. You
won't get a lot of real-world testing on it if you delay until Tomcat 9,
but you might get some pioneers who will trust the stability of the rest
of Tomcat 8 and might be willing to give a new connector a try -- if
even for a short period of time.

-chris

Re: NIO 2 connector

[Rémy Maucherat <re...@apache.org>]

2014-03-04 19:26 GMT+01:00 Mark Thomas <ma...@apache.org>:

> On 04/03/2014 16:16, Rémy Maucherat wrote:
> > Hi,
> >
> > I've been working on porting a NIO2 connector that was originally
> developed
> > for JBoss AS by Nabil Benothman (an intern at Red Hat). Due to the very
> > different connector structure in Tomcat and my preference for basing it
> on
> > the existing NIO1 connector, it is mostly new code, though.
>
> There looks to be opportunities to share code with the current NIO
> connector.
>

Don't know. I used the structure, but the algorithms are quite different.
You'll have the opportunity to do it if you want of course ;)

>
> There are some changes to the existing code, such as
> java/org/apache/coyote/http11/upgrade/AbstractServletInputStream.java
> that I would like to understand.
>

Nothing special, it needs a first read to start its "polling", so that's
the reason for adding isReady() (normally harmless). I added the third
event method too for consistency, and that's it.

>
> > So I would now like to contribute this in trunk as a new experimental
> > connector. It should have feature parity with the NIO1 connectors. Of
> > course, while the basics are in, it will need some time to pass the
> > testsuite, fix issues, improve stability, etc.
>
> Can you wait until we split 8.0.x from trunk or did you want to get this
> into 8.0.x?
>

Depends, if you want to branch soon or not. It would have to be
experimental for a while anyway, but it will likely bring something useful.

>
> > Coyote version number could be moved up to 2.0 with this addition along
> > with all the connector refactorings that Mark did in trunk.
>
> I'd rather drop the version number entirely as it is pretty much
> meaningless. If we are going to do that, we may as well change the
> server header to "Apache Tomcat". I'm neutral on whether to include the
> major or full version number.
>

I think the spec says there should be a /0.0 version number, and I like the
Coyote name, but you can change it.

>
> > The code is there (rebased to the current trunk):
> > https://github.com/rmaucher/tomcat
> >
> > A quick NIO2 (the API) presentation.
> >
> > Pros:
> > - Significantly faster, although the API looks slower by design
> > - Resources friendly
> > - Seemingly trivial to use
> >  - Polling is neatly hidden away
> > - Thread pool is also neatly hidden away
> > - Per operation timeouts
> > - Read/Write is symmetric
> > - Trivial blocking IO
> >
> > Cons:
> > - No real non blocking IO
>
> Hmm. I was considering dropping the BIO connector entirely for Tomcat 9
> because of its inability to do non-blocking IO and the way the Servlet
> API was heading. Does it make sense to add a different non-blocking
> connector implementation or have I misunderstood your point?
>

Well, nobody is going for non blocking IO, people are using async IO.

But then obviously the read(ByteBuffer) call which used to return an int
(possibly 0 with non blocking IO) now returns a Future<Integer>. And if you
want to know the result you have to wait for it and there's an IO operation
pending the entire time. So that's the "cons".

It would be nice to have non blocking operations for "peek" scenarios. So
instead to peek a read, you have to read with a completion handler, then
see if it returns inline. And if it doesn't there's an async process now
doing IO (possibly not the greatest thing that could happen).

>
> > - No concurrency allowed [for the socket impl of NIO2] although the API
> > looks concurrent
>
> Do you mean no concurrent read and writes? That would be a huge issue.
>

No, don't worry, that works. It's concurrency on the same operation (like
read, write, accept). The API looks relatively magic, so you'd think you
could do it.

>
> > - Simplicity is sometimes misleading, the API doesn't provide some tools
> > for the needed synchronization, check pending operations and their
> possible
> > optimizations
> > - SSL is not integrated any better than with NIO1, it is still SSLEngine
> > which leads to creating the obligatory "AsynchronousSSLSocketChannel"
> > wrapper class yourself
> > - No real sendfile
> >
> > Comments ?
>
> The pros look nice but I am worried about the cons.
>
> The summary is that in the worst case, it's much better.

Rémy

Re: NIO 2 connector

[Mark Thomas <ma...@apache.org>]

On 04/03/2014 16:16, Rémy Maucherat wrote:
> Hi,
> 
> I've been working on porting a NIO2 connector that was originally developed
> for JBoss AS by Nabil Benothman (an intern at Red Hat). Due to the very
> different connector structure in Tomcat and my preference for basing it on
> the existing NIO1 connector, it is mostly new code, though.

There looks to be opportunities to share code with the current NIO
connector.

There are some changes to the existing code, such as
java/org/apache/coyote/http11/upgrade/AbstractServletInputStream.java
that I would like to understand.

> So I would now like to contribute this in trunk as a new experimental
> connector. It should have feature parity with the NIO1 connectors. Of
> course, while the basics are in, it will need some time to pass the
> testsuite, fix issues, improve stability, etc.

Can you wait until we split 8.0.x from trunk or did you want to get this
into 8.0.x?

> Coyote version number could be moved up to 2.0 with this addition along
> with all the connector refactorings that Mark did in trunk.

I'd rather drop the version number entirely as it is pretty much
meaningless. If we are going to do that, we may as well change the
server header to "Apache Tomcat". I'm neutral on whether to include the
major or full version number.

> The code is there (rebased to the current trunk):
> https://github.com/rmaucher/tomcat
> 
> A quick NIO2 (the API) presentation.
> 
> Pros:
> - Significantly faster, although the API looks slower by design
> - Resources friendly
> - Seemingly trivial to use
>  - Polling is neatly hidden away
> - Thread pool is also neatly hidden away
> - Per operation timeouts
> - Read/Write is symmetric
> - Trivial blocking IO
> 
> Cons:
> - No real non blocking IO

Hmm. I was considering dropping the BIO connector entirely for Tomcat 9
because of its inability to do non-blocking IO and the way the Servlet
API was heading. Does it make sense to add a different non-blocking
connector implementation or have I misunderstood your point?

> - No concurrency allowed [for the socket impl of NIO2] although the API
> looks concurrent

Do you mean no concurrent read and writes? That would be a huge issue.

> - Simplicity is sometimes misleading, the API doesn't provide some tools
> for the needed synchronization, check pending operations and their possible
> optimizations
> - SSL is not integrated any better than with NIO1, it is still SSLEngine
> which leads to creating the obligatory "AsynchronousSSLSocketChannel"
> wrapper class yourself
> - No real sendfile
> 
> Comments ?

The pros look nice but I am worried about the cons.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

Re: NIO 2 connector

[Rémy Maucherat <re...@apache.org>]

2014-03-10 14:03 GMT+01:00 Konstantin Kolinko <kn...@gmail.com>:

> There are 52 warning shown by Eclipse IDE.
> Most of them are boxing/unboxing ones.
>
> To remind, the setting are documented here:
> res\ide-support\eclipse\java-compiler-errors-warnings.txt
>

Ahah, ok, NIO2 does a lof of boxing/unboxing indeed. I'll try to improve
that.

>
>
> I would like  "execute.test.nio2"  property to be added to
> build.properties.default
> BUILDING.txt
>
> but that can be postponed a bit. It depends on how well the tests run for
> NIO2.
>
> Don't do it, it will fail. Very minor issues, but not necessarily easy to
fix.

Rémy

Re: NIO 2 connector

[Konstantin Kolinko <kn...@gmail.com>]

2014-03-08 4:41 GMT+04:00 Rémy Maucherat <re...@apache.org>:
> 2014-03-07 23:16 GMT+01:00 Konstantin Kolinko <kn...@gmail.com>:
>
>> 1. It is a month since release 8.0.3 and thus I think 8.0.4 is
>> expected in a week or so..
>>
>> I am -1 to destabilize 8.0.x now.
>>
>
> As a new component, it is not supposed to destabilize the other components
> that are in 8.0.
>

There are 52 warning shown by Eclipse IDE.
Most of them are boxing/unboxing ones.

To remind, the setting are documented here:
res\ide-support\eclipse\java-compiler-errors-warnings.txt


I would like  "execute.test.nio2"  property to be added to
build.properties.default
BUILDING.txt

but that can be postponed a bit. It depends on how well the tests run for NIO2.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

Re: NIO 2 connector

[Rémy Maucherat <re...@apache.org>]

2014-03-07 23:16 GMT+01:00 Konstantin Kolinko <kn...@gmail.com>:

> 1. It is a month since release 8.0.3 and thus I think 8.0.4 is
> expected in a week or so..
>
> I am -1 to destabilize 8.0.x now.
>

As a new component, it is not supposed to destabilize the other components
that are in 8.0.

>
> To this, as you are saying that some tests do not pass. I woudn't want
> to make buildbot fail this close to a release, as we may miss
> something important.
>

It will cause failures only if the tests are enabled for this connector at
build time, there's no reason to do so at the moment since it is
experimental.

>
> So I think the time to merge this is not earlier than 8.0.4 is voted
> and released + some time to cool off. I think that is about 10 days
> from now.
>
> 2. How am I supposed to review this?
> Is there some viewable "patch" against trunk?
>

You just gave the link for the commit below, it shows all possible diffs,
and it is rebased against trunk.

>
> The comments below are from a quick review of
>
>
> https://github.com/rmaucher/tomcat/commit/614d8c43d8d1f3eeb4d5e4c2493ead589a72bf2c
>
> 1) There are a number of unrelated changes, including some comment /
> typo fixes. That does not help reviewing. Why aren't those in Tomcat
> trunk yet?
>

There's only a very small amount of them (maybe 3), it shouldn't hinder
reviewing in any way. Actually, that's a good idea, I'll go ahead and
commit the changes to the existing classes, since I consider they make some
sense on their own.

I'm not asking for a full review anyway, just some general feedback on the
component.

>
> 2). There is a lot of code that from the first glance seems similar to
> Nio1 one.
> Just a matter of preference, not a stopper.
>

The shared code can be move to a superclass eventually. But it is very
different, a lot of major pieces of code from the NIO1 connector have no
equivalent.

>
> (In your opening e-mail you say there are many differences. This
> similarity is my first impression. Maybe there aren't any. I'd need
> some time to compare).
>
> 3). What are the selling points of this implementation?
>

I described some of the pros and cons of the NIO2 API. If the pros end up
in the implementation without being degraded, it means the connector should
be faster, more resources friendly, simpler (no poller, no selector, etc),
since that's what happened with a similar NIO2 connector in JBoss. But I
can't make any big promises yet.

The documentation part of the patch does not say anything in particular.
> Also my -1 that it does not say that this connector is an "experimental"
> one.
>

On startup, it logs: WARNING [main]
org.apache.tomcat.util.net.Nio2Endpoint.startInternal The NIO2 connector is
currently EXPERIMENTAL and should not be used in production
So it is annoyingly visible, but I can add another mention in the docs.

Rémy

Re: NIO 2 connector

[Konstantin Kolinko <kn...@gmail.com>]

2014-03-07 19:17 GMT+04:00 Rémy Maucherat <re...@apache.org>:
> 2014-03-04 17:16 GMT+01:00 Rémy Maucherat <re...@apache.org>:
>
>> The code is there (rebased to the current trunk):
>> https://github.com/rmaucher/tomcat
>>
>> Updated commit here:
> https://github.com/rmaucher/tomcat/commit/614d8c43d8d1f3eeb4d5e4c2493ead589a72bf2c
>
> I have removed the two main hacks and the testsuite status is relatively
> clean (some failures though, but some of theses are tests which have a
> timing which looks a bit too adapted for the other connectors; I did look
> individually at all the problems and made some fixes).
>
> So far there are 3 people in favor of merging this in the current trunk
> (still unbranched 8.0).
>

1. It is a month since release 8.0.3 and thus I think 8.0.4 is
expected in a week or so..

I am -1 to destabilize 8.0.x now.

To this, as you are saying that some tests do not pass. I woudn't want
to make buildbot fail this close to a release, as we may miss
something important.

So I think the time to merge this is not earlier than 8.0.4 is voted
and released + some time to cool off. I think that is about 10 days
from now.

2. How am I supposed to review this?
Is there some viewable "patch" against trunk?

The comments below are from a quick review of

https://github.com/rmaucher/tomcat/commit/614d8c43d8d1f3eeb4d5e4c2493ead589a72bf2c

1) There are a number of unrelated changes, including some comment /
typo fixes. That does not help reviewing. Why aren't those in Tomcat
trunk yet?

2). There is a lot of code that from the first glance seems similar to Nio1 one.
Just a matter of preference, not a stopper.

(In your opening e-mail you say there are many differences. This
similarity is my first impression. Maybe there aren't any. I'd need
some time to compare).

3). What are the selling points of this implementation?
The documentation part of the patch does not say anything in particular.
Also my -1 that it does not say that this connector is an "experimental" one.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

Re: NIO 2 connector

[Rémy Maucherat <re...@apache.org>]

2014-03-04 17:16 GMT+01:00 Rémy Maucherat <re...@apache.org>:

> The code is there (rebased to the current trunk):
> https://github.com/rmaucher/tomcat
>
> Updated commit here:
https://github.com/rmaucher/tomcat/commit/614d8c43d8d1f3eeb4d5e4c2493ead589a72bf2c

I have removed the two main hacks and the testsuite status is relatively
clean (some failures though, but some of theses are tests which have a
timing which looks a bit too adapted for the other connectors; I did look
individually at all the problems and made some fixes).

So far there are 3 people in favor of merging this in the current trunk
(still unbranched 8.0).

Any additional comments ?

Rémy

Re: NIO 2 connector

[jean-frederic clere <jf...@gmail.com>]

On 03/04/2014 05:16 PM, Rémy Maucherat wrote:
> Pros:
> - Significantly faster, although the API looks slower by design

Actually I didn't bench it yet but I benched "something similar" on 
JBossWeb: it performed faster than APR, the implementation is a bit 
different in that area so I will retest it when I have cycles.

Cheers

Jean-Frederic

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org