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 2020/03/17 17:06:42 UTC
[tomcat] branch master updated: Remove sync
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 404a3a6 Remove sync
404a3a6 is described below
commit 404a3a6366c0a8ba3e747db57e24dee271c44ac3
Author: remm <re...@apache.org>
AuthorDate: Tue Mar 17 18:06:23 2020 +0100
Remove sync
keyFor syncs on a global lock, so remove it when processing sockets.
---
java/org/apache/tomcat/util/net/NioEndpoint.java | 50 +++++++++++-------------
webapps/docs/changelog.xml | 4 ++
2 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index f753b9a..7012782 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -1530,33 +1530,29 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
socketWrapper.close();
return;
}
- SelectionKey key = socket.getIOChannel().keyFor(poller.getSelector());
try {
int handshake = -1;
-
try {
- if (key != null) {
- if (socket.isHandshakeComplete()) {
- // No TLS handshaking required. Let the handler
- // process this socket / event combination.
- handshake = 0;
- } else if (event == SocketEvent.STOP || event == SocketEvent.DISCONNECT ||
- event == SocketEvent.ERROR) {
- // Unable to complete the TLS handshake. Treat it as
- // if the handshake failed.
- handshake = -1;
- } else {
- handshake = socket.handshake(key.isReadable(), key.isWritable());
- // The handshake process reads/writes from/to the
- // socket. status may therefore be OPEN_WRITE once
- // the handshake completes. However, the handshake
- // happens when the socket is opened so the status
- // must always be OPEN_READ after it completes. It
- // is OK to always set this as it is only used if
- // the handshake completes.
- event = SocketEvent.OPEN_READ;
- }
+ if (socket.isHandshakeComplete()) {
+ // No TLS handshaking required. Let the handler
+ // process this socket / event combination.
+ handshake = 0;
+ } else if (event == SocketEvent.STOP || event == SocketEvent.DISCONNECT ||
+ event == SocketEvent.ERROR) {
+ // Unable to complete the TLS handshake. Treat it as
+ // if the handshake failed.
+ handshake = -1;
+ } else {
+ handshake = socket.handshake(event == SocketEvent.OPEN_READ, event == SocketEvent.OPEN_WRITE);
+ // The handshake process reads/writes from/to the
+ // socket. status may therefore be OPEN_WRITE once
+ // the handshake completes. However, the handshake
+ // happens when the socket is opened so the status
+ // must always be OPEN_READ after it completes. It
+ // is OK to always set this as it is only used if
+ // the handshake completes.
+ event = SocketEvent.OPEN_READ;
}
} catch (IOException x) {
handshake = -1;
@@ -1573,23 +1569,23 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
state = getHandler().process(socketWrapper, event);
}
if (state == SocketState.CLOSED) {
- poller.cancelledKey(key, socketWrapper);
+ poller.cancelledKey(socket.getIOChannel().keyFor(poller.getSelector()), socketWrapper);
}
} else if (handshake == -1 ) {
getHandler().process(socketWrapper, SocketEvent.CONNECT_FAIL);
- poller.cancelledKey(key, socketWrapper);
+ poller.cancelledKey(socket.getIOChannel().keyFor(poller.getSelector()), socketWrapper);
} else if (handshake == SelectionKey.OP_READ){
socketWrapper.registerReadInterest();
} else if (handshake == SelectionKey.OP_WRITE){
socketWrapper.registerWriteInterest();
}
} catch (CancelledKeyException cx) {
- poller.cancelledKey(key, socketWrapper);
+ poller.cancelledKey(socket.getIOChannel().keyFor(poller.getSelector()), socketWrapper);
} catch (VirtualMachineError vme) {
ExceptionUtils.handleThrowable(vme);
} catch (Throwable t) {
log.error(sm.getString("endpoint.processing.fail"), t);
- poller.cancelledKey(key, socketWrapper);
+ poller.cancelledKey(socket.getIOChannel().keyFor(poller.getSelector()), socketWrapper);
} finally {
socketWrapper = null;
event = null;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 69ffa99..34d3cd6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -67,6 +67,10 @@
for <code>URIEncoding</code> is a superset of US-ASCII as required by
RFC7230. (markt)
</add>
+ <fix>
+ Avoid always retrieving the NIO poller selection key when processing
+ to reduce sync. (remm)
+ </fix>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch master updated: Remove sync
Posted by Rémy Maucherat <re...@apache.org>.
On Tue, Mar 17, 2020 at 6:06 PM <re...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> remm pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new 404a3a6 Remove sync
> 404a3a6 is described below
>
> commit 404a3a6366c0a8ba3e747db57e24dee271c44ac3
> Author: remm <re...@apache.org>
> AuthorDate: Tue Mar 17 18:06:23 2020 +0100
>
> Remove sync
>
> keyFor syncs on a global lock, so remove it when processing sockets.
>
Actually I misread the code and the lock is only tied to the channel and
not the selector, so it's very cheap. I'll amend the changelog. Let me know
if this breaks something and I'll revert this, it's clearly has zero
measureable impact at this point.
Rémy
> ---
> java/org/apache/tomcat/util/net/NioEndpoint.java | 50
> +++++++++++-------------
> webapps/docs/changelog.xml | 4 ++
> 2 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java
> b/java/org/apache/tomcat/util/net/NioEndpoint.java
> index f753b9a..7012782 100644
> --- a/java/org/apache/tomcat/util/net/NioEndpoint.java
> +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
> @@ -1530,33 +1530,29 @@ public class NioEndpoint extends
> AbstractJsseEndpoint<NioChannel,SocketChannel>
> socketWrapper.close();
> return;
> }
> - SelectionKey key =
> socket.getIOChannel().keyFor(poller.getSelector());
>
> try {
> int handshake = -1;
> -
> try {
> - if (key != null) {
> - if (socket.isHandshakeComplete()) {
> - // No TLS handshaking required. Let the
> handler
> - // process this socket / event combination.
> - handshake = 0;
> - } else if (event == SocketEvent.STOP || event ==
> SocketEvent.DISCONNECT ||
> - event == SocketEvent.ERROR) {
> - // Unable to complete the TLS handshake.
> Treat it as
> - // if the handshake failed.
> - handshake = -1;
> - } else {
> - handshake =
> socket.handshake(key.isReadable(), key.isWritable());
> - // The handshake process reads/writes from/to
> the
> - // socket. status may therefore be OPEN_WRITE
> once
> - // the handshake completes. However, the
> handshake
> - // happens when the socket is opened so the
> status
> - // must always be OPEN_READ after it
> completes. It
> - // is OK to always set this as it is only
> used if
> - // the handshake completes.
> - event = SocketEvent.OPEN_READ;
> - }
> + if (socket.isHandshakeComplete()) {
> + // No TLS handshaking required. Let the handler
> + // process this socket / event combination.
> + handshake = 0;
> + } else if (event == SocketEvent.STOP || event ==
> SocketEvent.DISCONNECT ||
> + event == SocketEvent.ERROR) {
> + // Unable to complete the TLS handshake. Treat it
> as
> + // if the handshake failed.
> + handshake = -1;
> + } else {
> + handshake = socket.handshake(event ==
> SocketEvent.OPEN_READ, event == SocketEvent.OPEN_WRITE);
> + // The handshake process reads/writes from/to the
> + // socket. status may therefore be OPEN_WRITE once
> + // the handshake completes. However, the handshake
> + // happens when the socket is opened so the status
> + // must always be OPEN_READ after it completes. It
> + // is OK to always set this as it is only used if
> + // the handshake completes.
> + event = SocketEvent.OPEN_READ;
> }
> } catch (IOException x) {
> handshake = -1;
> @@ -1573,23 +1569,23 @@ public class NioEndpoint extends
> AbstractJsseEndpoint<NioChannel,SocketChannel>
> state = getHandler().process(socketWrapper,
> event);
> }
> if (state == SocketState.CLOSED) {
> - poller.cancelledKey(key, socketWrapper);
> +
> poller.cancelledKey(socket.getIOChannel().keyFor(poller.getSelector()),
> socketWrapper);
> }
> } else if (handshake == -1 ) {
> getHandler().process(socketWrapper,
> SocketEvent.CONNECT_FAIL);
> - poller.cancelledKey(key, socketWrapper);
> +
> poller.cancelledKey(socket.getIOChannel().keyFor(poller.getSelector()),
> socketWrapper);
> } else if (handshake == SelectionKey.OP_READ){
> socketWrapper.registerReadInterest();
> } else if (handshake == SelectionKey.OP_WRITE){
> socketWrapper.registerWriteInterest();
> }
> } catch (CancelledKeyException cx) {
> - poller.cancelledKey(key, socketWrapper);
> +
> poller.cancelledKey(socket.getIOChannel().keyFor(poller.getSelector()),
> socketWrapper);
> } catch (VirtualMachineError vme) {
> ExceptionUtils.handleThrowable(vme);
> } catch (Throwable t) {
> log.error(sm.getString("endpoint.processing.fail"), t);
> - poller.cancelledKey(key, socketWrapper);
> +
> poller.cancelledKey(socket.getIOChannel().keyFor(poller.getSelector()),
> socketWrapper);
> } finally {
> socketWrapper = null;
> event = null;
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index 69ffa99..34d3cd6 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -67,6 +67,10 @@
> for <code>URIEncoding</code> is a superset of US-ASCII as
> required by
> RFC7230. (markt)
> </add>
> + <fix>
> + Avoid always retrieving the NIO poller selection key when
> processing
> + to reduce sync. (remm)
> + </fix>
> </changelog>
> </subsection>
> </section>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>