You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2015/01/16 18:32:18 UTC
svn commit: r1652468 -
/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
Author: remm
Date: Fri Jan 16 17:32:18 2015
New Revision: 1652468
URL: http://svn.apache.org/r1652468
Log:
- Initially after accept, do regular processing rather than awaitBytes, since awaitBytes is not as light as it used to be and the bytes could be there. Maybe it could be configurable.
- Don't always fork a new thread after awaitBytes. If it didn't complete inline then it is supposed to be useless.
- This however caused problems with write notifications. Although I do understand the changes that were made, a notification should not be recursive, so use a new thread in that case.
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=1652468&r1=1652467&r2=1652468&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Fri Jan 16 17:32:18 2015
@@ -461,7 +461,6 @@ public class Nio2Endpoint extends Abstra
return new Acceptor();
}
-
/**
* Process the specified connection.
*/
@@ -502,13 +501,8 @@ public class Nio2Endpoint extends Abstra
socketWrapper.reset(channel, getSocketProperties().getSoTimeout());
socketWrapper.setKeepAliveLeft(Nio2Endpoint.this.getMaxKeepAliveRequests());
socketWrapper.setSecure(isSSLEnabled());
- if (sslContext != null) {
- // Use the regular processing, as the first handshake needs to be done there
- processSocket(socketWrapper, SocketStatus.OPEN_READ, true);
- } else {
- // Wait until some bytes are available to start the real processing
- awaitBytes(socketWrapper);
- }
+ // Continue processing on another thread
+ processSocket(socketWrapper, SocketStatus.OPEN_READ, true);
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
try {
@@ -549,7 +543,6 @@ public class Nio2Endpoint extends Abstra
return true;
}
-
@Override
public void processSocket(SocketWrapperBase<Nio2Channel> socketWrapper,
SocketStatus socketStatus, boolean dispatch) {
@@ -739,13 +732,13 @@ public class Nio2Endpoint extends Abstra
= new CompletionHandler<Integer, SocketWrapperBase<Nio2Channel>>() {
@Override
- public synchronized void completed(Integer nBytes, SocketWrapperBase<Nio2Channel> attachment) {
+ public void completed(Integer nBytes, SocketWrapperBase<Nio2Channel> attachment) {
if (nBytes.intValue() < 0) {
failed(new ClosedChannelException(), attachment);
return;
}
readPending.release();
- getEndpoint().processSocket(attachment, SocketStatus.OPEN_READ, true);
+ getEndpoint().processSocket(attachment, SocketStatus.OPEN_READ, Nio2Endpoint.isInline());
}
@Override
@@ -755,8 +748,6 @@ public class Nio2Endpoint extends Abstra
}
};
-
-
public Nio2SocketWrapper(Nio2Channel channel, Nio2Endpoint endpoint) {
super(channel, endpoint);
@@ -840,7 +831,7 @@ public class Nio2Endpoint extends Abstra
}
}
if (writeNotify && nestedWriteCompletionCount.get().get() == 0) {
- endpoint.processSocket(Nio2SocketWrapper.this, SocketStatus.OPEN_WRITE, false);
+ endpoint.processSocket(Nio2SocketWrapper.this, SocketStatus.OPEN_WRITE, Nio2Endpoint.isInline());
}
}
@@ -894,7 +885,7 @@ public class Nio2Endpoint extends Abstra
}
}
if (writeNotify && nestedWriteCompletionCount.get().get() == 0) {
- endpoint.processSocket(Nio2SocketWrapper.this, SocketStatus.OPEN_WRITE, false);
+ endpoint.processSocket(Nio2SocketWrapper.this, SocketStatus.OPEN_WRITE, Nio2Endpoint.isInline());
}
}
@@ -1133,7 +1124,7 @@ public class Nio2Endpoint extends Abstra
*/
@Override
protected void writeNonBlocking(byte[] buf, int off, int len) throws IOException {
- // FIXME: Possible new behavior:
+ // Note: Possible alternate behavior:
// If there's non blocking abuse (like a test writing 1MB in a single
// "non blocking" write), then block until the previous write is
// done rather than continue buffering
@@ -1227,13 +1218,17 @@ public class Nio2Endpoint extends Abstra
}
bufferedWrites.clear();
ByteBuffer[] array = arrayList.toArray(new ByteBuffer[arrayList.size()]);
+ Nio2Endpoint.startInline();
getSocket().write(array, 0, array.length, getTimeout(),
TimeUnit.MILLISECONDS, array, gatheringWriteCompletionHandler);
+ Nio2Endpoint.endInline();
} else if (socketBufferHandler.getWriteBuffer().hasRemaining()) {
// Regular write
+ Nio2Endpoint.startInline();
getSocket().write(socketBufferHandler.getWriteBuffer(), getTimeout(),
TimeUnit.MILLISECONDS, socketBufferHandler.getWriteBuffer(),
writeCompletionHandler);
+ Nio2Endpoint.endInline();
} else {
// Nothing was written
if (!hasPermit) {
@@ -1296,17 +1291,19 @@ public class Nio2Endpoint extends Abstra
// NO-OP. Appropriate handlers will already have been registered.
}
-
public void awaitBytes() {
if (getSocket() == null) {
return;
}
if (readPending.tryAcquire()) {
getSocket().getBufHandler().configureReadBufferForWrite();
+ Nio2Endpoint.startInline();
getSocket().read(getSocket().getBufHandler().getReadBuffer(),
getTimeout(), TimeUnit.MILLISECONDS, this, awaitBytesHandler);
+ Nio2Endpoint.endInline();
}
}
+
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1652468 - /tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
Posted by Mark Thomas <ma...@apache.org>.
On 19/01/2015 10:52, Rémy Maucherat wrote:
> 2015-01-17 2:41 GMT+01:00 Mark Thomas <ma...@apache.org>:
>
>> The other thought I had (but haven't had time to test for performance
>> impact) is that the readPending semaphore is really only useful when the
>> previous or current read is non-blocking. A blocking read followed by a
>> blocking read doesn't need it.
>
> I tested it (commenting it out in flushBlocking), the impact is rather
> marginal with ab.
Thanks for testing that. I won't investigate this idea further then.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1652468 - /tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
Posted by Rémy Maucherat <re...@apache.org>.
2015-01-17 2:41 GMT+01:00 Mark Thomas <ma...@apache.org>:
> The other thought I had (but haven't had time to test for performance
> impact) is that the readPending semaphore is really only useful when the
> previous or current read is non-blocking. A blocking read followed by a
> blocking read doesn't need it.
>
> I tested it (commenting it out in flushBlocking), the impact is rather
marginal with ab.
Rémy
Re: svn commit: r1652468 - /tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
Posted by Mark Thomas <ma...@apache.org>.
On 16/01/2015 17:32, remm@apache.org wrote:
> Author: remm
> Date: Fri Jan 16 17:32:18 2015
> New Revision: 1652468
>
> URL: http://svn.apache.org/r1652468
> Log:
> - Initially after accept, do regular processing rather than awaitBytes, since awaitBytes is not as light as it used to be and the bytes could be there. Maybe it could be configurable.
> - Don't always fork a new thread after awaitBytes. If it didn't complete inline then it is supposed to be useless.
> - This however caused problems with write notifications. Although I do understand the changes that were made, a notification should not be recursive, so use a new thread in that case.
The other thought I had (but haven't had time to test for performance
impact) is that the readPending semaphore is really only useful when the
previous or current read is non-blocking. A blocking read followed by a
blocking read doesn't need it.
Mark
>
> 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=1652468&r1=1652467&r2=1652468&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Fri Jan 16 17:32:18 2015
> @@ -461,7 +461,6 @@ public class Nio2Endpoint extends Abstra
> return new Acceptor();
> }
>
> -
> /**
> * Process the specified connection.
> */
> @@ -502,13 +501,8 @@ public class Nio2Endpoint extends Abstra
> socketWrapper.reset(channel, getSocketProperties().getSoTimeout());
> socketWrapper.setKeepAliveLeft(Nio2Endpoint.this.getMaxKeepAliveRequests());
> socketWrapper.setSecure(isSSLEnabled());
> - if (sslContext != null) {
> - // Use the regular processing, as the first handshake needs to be done there
> - processSocket(socketWrapper, SocketStatus.OPEN_READ, true);
> - } else {
> - // Wait until some bytes are available to start the real processing
> - awaitBytes(socketWrapper);
> - }
> + // Continue processing on another thread
> + processSocket(socketWrapper, SocketStatus.OPEN_READ, true);
> } catch (Throwable t) {
> ExceptionUtils.handleThrowable(t);
> try {
> @@ -549,7 +543,6 @@ public class Nio2Endpoint extends Abstra
> return true;
> }
>
> -
> @Override
> public void processSocket(SocketWrapperBase<Nio2Channel> socketWrapper,
> SocketStatus socketStatus, boolean dispatch) {
> @@ -739,13 +732,13 @@ public class Nio2Endpoint extends Abstra
> = new CompletionHandler<Integer, SocketWrapperBase<Nio2Channel>>() {
>
> @Override
> - public synchronized void completed(Integer nBytes, SocketWrapperBase<Nio2Channel> attachment) {
> + public void completed(Integer nBytes, SocketWrapperBase<Nio2Channel> attachment) {
> if (nBytes.intValue() < 0) {
> failed(new ClosedChannelException(), attachment);
> return;
> }
> readPending.release();
> - getEndpoint().processSocket(attachment, SocketStatus.OPEN_READ, true);
> + getEndpoint().processSocket(attachment, SocketStatus.OPEN_READ, Nio2Endpoint.isInline());
> }
>
> @Override
> @@ -755,8 +748,6 @@ public class Nio2Endpoint extends Abstra
> }
> };
>
> -
> -
> public Nio2SocketWrapper(Nio2Channel channel, Nio2Endpoint endpoint) {
> super(channel, endpoint);
>
> @@ -840,7 +831,7 @@ public class Nio2Endpoint extends Abstra
> }
> }
> if (writeNotify && nestedWriteCompletionCount.get().get() == 0) {
> - endpoint.processSocket(Nio2SocketWrapper.this, SocketStatus.OPEN_WRITE, false);
> + endpoint.processSocket(Nio2SocketWrapper.this, SocketStatus.OPEN_WRITE, Nio2Endpoint.isInline());
> }
> }
>
> @@ -894,7 +885,7 @@ public class Nio2Endpoint extends Abstra
> }
> }
> if (writeNotify && nestedWriteCompletionCount.get().get() == 0) {
> - endpoint.processSocket(Nio2SocketWrapper.this, SocketStatus.OPEN_WRITE, false);
> + endpoint.processSocket(Nio2SocketWrapper.this, SocketStatus.OPEN_WRITE, Nio2Endpoint.isInline());
> }
> }
>
> @@ -1133,7 +1124,7 @@ public class Nio2Endpoint extends Abstra
> */
> @Override
> protected void writeNonBlocking(byte[] buf, int off, int len) throws IOException {
> - // FIXME: Possible new behavior:
> + // Note: Possible alternate behavior:
> // If there's non blocking abuse (like a test writing 1MB in a single
> // "non blocking" write), then block until the previous write is
> // done rather than continue buffering
> @@ -1227,13 +1218,17 @@ public class Nio2Endpoint extends Abstra
> }
> bufferedWrites.clear();
> ByteBuffer[] array = arrayList.toArray(new ByteBuffer[arrayList.size()]);
> + Nio2Endpoint.startInline();
> getSocket().write(array, 0, array.length, getTimeout(),
> TimeUnit.MILLISECONDS, array, gatheringWriteCompletionHandler);
> + Nio2Endpoint.endInline();
> } else if (socketBufferHandler.getWriteBuffer().hasRemaining()) {
> // Regular write
> + Nio2Endpoint.startInline();
> getSocket().write(socketBufferHandler.getWriteBuffer(), getTimeout(),
> TimeUnit.MILLISECONDS, socketBufferHandler.getWriteBuffer(),
> writeCompletionHandler);
> + Nio2Endpoint.endInline();
> } else {
> // Nothing was written
> if (!hasPermit) {
> @@ -1296,17 +1291,19 @@ public class Nio2Endpoint extends Abstra
> // NO-OP. Appropriate handlers will already have been registered.
> }
>
> -
> public void awaitBytes() {
> if (getSocket() == null) {
> return;
> }
> if (readPending.tryAcquire()) {
> getSocket().getBufHandler().configureReadBufferForWrite();
> + Nio2Endpoint.startInline();
> getSocket().read(getSocket().getBufHandler().getReadBuffer(),
> getTimeout(), TimeUnit.MILLISECONDS, this, awaitBytesHandler);
> + Nio2Endpoint.endInline();
> }
> }
> +
> }
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org