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 5bdd7d4 Simplify blocking read and write for NIO
5bdd7d4 is described below
commit 5bdd7d4712fac4e1af47421c3600b18fabc22ed6
Author: remm <[email protected]>
AuthorDate: Mon Dec 9 15:15:00 2019 +0100
Simplify blocking read and write for NIO
This does not remove or cleanup any of the code that is now unused
(NioSelectorPool, NioBlockingSlector, channel flush method, fields,
etc), it will be done after actual review.
I do not see any negative performance impact. Note: for performance
testing, use HTTP/1.1 (avoiding sendfile).
---
java/org/apache/tomcat/util/net/NioEndpoint.java | 106 ++++++++++++++++-------
webapps/docs/changelog.xml | 7 ++
2 files changed, 84 insertions(+), 29 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java
b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 6a0bfdc..7d4104a 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -40,6 +40,7 @@ import java.util.Iterator;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
import javax.net.ssl.SSLEngine;
@@ -773,6 +774,12 @@ public class NioEndpoint extends
AbstractJsseEndpoint<NioChannel,SocketChannel>
if
(!socketWrapper.readOperation.process()) {
closeSocket = true;
}
+ } else if (socketWrapper.blockReadDone !=
null) {
+ if
(socketWrapper.blockReadDone.compareAndSet(false, true)) {
+ synchronized
(socketWrapper.blockReadDone) {
+
socketWrapper.blockReadDone.notify();
+ }
+ }
} else if (!processSocket(socketWrapper,
SocketEvent.OPEN_READ, true)) {
closeSocket = true;
}
@@ -782,6 +789,12 @@ public class NioEndpoint extends
AbstractJsseEndpoint<NioChannel,SocketChannel>
if
(!socketWrapper.writeOperation.process()) {
closeSocket = true;
}
+ } else if (socketWrapper.blockWriteDone !=
null) {
+ if
(socketWrapper.blockWriteDone.compareAndSet(false, true)) {
+ synchronized
(socketWrapper.blockWriteDone) {
+
socketWrapper.blockWriteDone.notify();
+ }
+ }
} else if (!processSocket(socketWrapper,
SocketEvent.OPEN_WRITE, true)) {
closeSocket = true;
}
@@ -1025,6 +1038,9 @@ public class NioEndpoint extends
AbstractJsseEndpoint<NioChannel,SocketChannel>
private volatile long lastRead = System.currentTimeMillis();
private volatile long lastWrite = lastRead;
+ private AtomicBoolean blockReadDone = null;
+ private AtomicBoolean blockWriteDone = null;
+
public NioSocketWrapper(NioChannel channel, NioEndpoint endpoint) {
super(channel, endpoint);
pool = endpoint.getSelectorPool();
@@ -1215,24 +1231,37 @@ public class NioEndpoint extends
AbstractJsseEndpoint<NioChannel,SocketChannel>
if (socket instanceof ClosedNioChannel) {
throw new ClosedChannelException();
}
- if (block) {
- Selector selector = null;
- try {
- selector = pool.get();
- } catch (IOException x) {
- // Ignore
- }
+ nRead = socket.read(to);
+ if (nRead == -1) {
+ throw new EOFException();
+ }
+ if (block && nRead == 0) {
+ long timeout = getReadTimeout();
try {
- nRead = pool.read(to, socket, selector, getReadTimeout());
- } finally {
- if (selector != null) {
- pool.put(selector);
+ blockReadDone = new AtomicBoolean(false);
+ registerReadInterest();
+ synchronized (blockReadDone) {
+ if (!blockReadDone.get()) {
+ try {
+ if (timeout > 0) {
+ blockReadDone.wait(timeout);
+ } else {
+ blockReadDone.wait();
+ }
+ } catch (InterruptedException e) {
+ // Continue ...
+ }
+ if (!blockReadDone.get()) {
+ throw new SocketTimeoutException();
+ }
+ }
}
- }
- } else {
- nRead = socket.read(to);
- if (nRead == -1) {
- throw new EOFException();
+ nRead = socket.read(to);
+ if (nRead == -1) {
+ throw new EOFException();
+ }
+ } finally {
+ blockReadDone = null;
}
}
return nRead;
@@ -1246,22 +1275,41 @@ public class NioEndpoint extends
AbstractJsseEndpoint<NioChannel,SocketChannel>
throw new ClosedChannelException();
}
if (block) {
- long writeTimeout = getWriteTimeout();
- Selector selector = null;
+ long timeout = getWriteTimeout();
try {
- selector = pool.get();
- } catch (IOException x) {
- // Ignore
- }
- try {
- pool.write(from, socket, selector, writeTimeout);
- // Make sure we are flushed
+ int n = 0;
do {
- } while (!socket.flush(true, selector, writeTimeout));
+ n = socket.write(from);
+ if (n == -1) {
+ throw new EOFException();
+ }
+ if (n == 0) {
+ if (blockWriteDone == null) {
+ blockWriteDone = new AtomicBoolean(false);
+ } else {
+ blockWriteDone.set(false);
+ }
+ registerWriteInterest();
+ synchronized (blockWriteDone) {
+ if (!blockWriteDone.get()) {
+ try {
+ if (timeout > 0) {
+ blockWriteDone.wait(timeout);
+ } else {
+ blockWriteDone.wait();
+ }
+ } catch (InterruptedException e) {
+ // Continue ...
+ }
+ if (!blockWriteDone.get()) {
+ throw new SocketTimeoutException();
+ }
+ }
+ }
+ }
+ } while (from.hasRemaining());
} finally {
- if (selector != null) {
- pool.put(selector);
- }
+ blockWriteDone = null;
}
// If there is data left in the buffer the socket will be
registered for
// write further up the stack. This is to ensure the socket is
only
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index f9eaaeb..5e48ebf 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -45,6 +45,13 @@
issues do not "pop up" wrt. others).
-->
<section name="Tomcat 9.0.31 (markt)" rtext="in development">
+ <subsection name="Coyote">
+ <changelog>
+ <update>
+ Simplify NIO blocking read and write. (remm)
+ </update>
+ </changelog>
+ </subsection>
</section>
<section name="Tomcat 9.0.30 (markt)" rtext="release in progress">
<subsection name="Catalina">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]