Re: NIO 2 connector
2014-03-08 4:41 GMT+04:00 Rémy Maucherat r...@apache.org: 2014-03-07 23:16 GMT+01:00 Konstantin Kolinko knst.koli...@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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: NIO 2 connector
2014-03-10 14:03 GMT+01:00 Konstantin Kolinko knst.koli...@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
2014-03-04 17:16 GMT+01:00 Rémy Maucherat r...@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
2014-03-07 19:17 GMT+04:00 Rémy Maucherat r...@apache.org: 2014-03-04 17:16 GMT+01:00 Rémy Maucherat r...@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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: NIO 2 connector
2014-03-07 23:16 GMT+01:00 Konstantin Kolinko knst.koli...@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
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 signature.asc Description: OpenPGP digital signature
Re: NIO 2 connector
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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: NIO 2 connector
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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: NIO 2 connector
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 FutureInteger. 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
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 FutureInteger. 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: NIO 2 connector
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
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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
