On 08/01/2015 14:06, Mark Thomas wrote: > On 08/01/2015 13:50, Rémy Maucherat wrote: >> 2015-01-08 14:11 GMT+01:00 <ma...@apache.org>: >> >>> Author: markt >>> Date: Thu Jan 8 13:10:59 2015 >>> New Revision: 1650280 >>> >>> URL: http://svn.apache.org/r1650280 >>> Log: >>> Fix failing unit test >>> testMessagesBlocking(org.apache.coyote.http11.upgrade.TestUpgrade) >>> >>> Modified: >>> tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java >>> >>> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java >>> URL: >>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1650280&r1=1650279&r2=1650280&view=diff >>> >>> ============================================================================== >>> --- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java >>> (original) >>> +++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Thu >>> Jan 8 13:10:59 2015 >>> @@ -1099,9 +1099,7 @@ public class Nio2Endpoint extends Abstra >>> int thisTime = transfer(buf, off, len, >>> socketWriteBuffer); >>> len = len - thisTime; >>> off = off + thisTime; >>> - if (socketWriteBuffer.remaining() == 0) { >>> - flush(true); >>> - } >>> + flush(true); >>> } >>> >> This doesn't look very good. Removing buffering = worse performance. > > Agreed. > >> This is probably because upgrade wouldn't deal with buffering as the message >> implies, so this could need an extra flag. > > On taking another look the problem is further up the call stack - the > patch above was just working around it. I need to look into why I only > saw the issue with NIO2.
The reason I only saw the issue with NIO2 is that only NIO2 was buffering the ServletOutputStream for upgraded connections. The patch above aligned NIO2 with NIO and APR - neither of which buffer upgraded connections. I took a look at the spec and it wasn't completely clear if the ServletOutputStream in an upgraded connection should be buffered or not. Against buffering is that all of the control is on the response - which isn't available for an upgraded connection. For buffering is the performance benefits and that nowhere does it say that ServletOutputStream is not buffered for upgrade. I can see the benefits of buffering here but given the typical use of upgraded connections I wonder if it isn't better handled at the application layer as and when required. Maybe something to clarify with the Servlet EG? Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
