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 e9e9b22 Remove acceptorThreadCount attribute e9e9b22 is described below commit e9e9b2201069f8b0857c541018a7aa81a9cebe52 Author: remm <r...@apache.org> AuthorDate: Thu May 9 15:35:56 2019 +0200 Remove acceptorThreadCount attribute Now hardcoded to one. Leave the attribute access methods as deprecated for compatibility. Remove all relevant loops and fields. --- java/org/apache/coyote/AbstractProtocol.java | 5 +- .../apache/tomcat/util/net/AbstractEndpoint.java | 110 +++++++++------------ java/org/apache/tomcat/util/net/AprEndpoint.java | 42 ++++---- java/org/apache/tomcat/util/net/Nio2Endpoint.java | 11 +-- java/org/apache/tomcat/util/net/NioEndpoint.java | 2 +- webapps/docs/changelog.xml | 6 ++ webapps/docs/config/http.xml | 8 -- 7 files changed, 80 insertions(+), 104 deletions(-) diff --git a/java/org/apache/coyote/AbstractProtocol.java b/java/org/apache/coyote/AbstractProtocol.java index 51bdb3b..4e35f43 100644 --- a/java/org/apache/coyote/AbstractProtocol.java +++ b/java/org/apache/coyote/AbstractProtocol.java @@ -310,11 +310,12 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler, return endpoint.getConnectionCount(); } + @Deprecated public void setAcceptorThreadCount(int threadCount) { - endpoint.setAcceptorThreadCount(threadCount); } + @Deprecated public int getAcceptorThreadCount() { - return endpoint.getAcceptorThreadCount(); + return 1; } public void setAcceptorThreadPriority(int threadPriority) { diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java index 8d01292..a33e192 100644 --- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java +++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java @@ -172,9 +172,9 @@ public abstract class AbstractEndpoint<S,U> { } /** - * Threads used to accept new connections and pass them to worker threads. + * Thread used to accept new connections and pass them to worker threads. */ - protected List<Acceptor<U>> acceptors; + protected Acceptor<U> acceptor; /** * Cache for SocketProcessor objects @@ -406,10 +406,10 @@ public abstract class AbstractEndpoint<S,U> { */ protected int acceptorThreadCount = 1; - public void setAcceptorThreadCount(int acceptorThreadCount) { - this.acceptorThreadCount = acceptorThreadCount; - } - public int getAcceptorThreadCount() { return acceptorThreadCount; } + @Deprecated + public void setAcceptorThreadCount(int acceptorThreadCount) {} + @Deprecated + public int getAcceptorThreadCount() { return 1; } /** @@ -923,13 +923,7 @@ public abstract class AbstractEndpoint<S,U> { */ private void unlockAccept() { // Only try to unlock the acceptor if it is necessary - int unlocksRequired = 0; - for (Acceptor<U> acceptor : acceptors) { - if (acceptor.getState() == AcceptorState.RUNNING) { - unlocksRequired++; - } - } - if (unlocksRequired == 0) { + if (acceptor == null || acceptor.getState() != AcceptorState.RUNNING) { return; } @@ -948,46 +942,42 @@ public abstract class AbstractEndpoint<S,U> { try { unlockAddress = getUnlockAddress(localAddress); - for (int i = 0; i < unlocksRequired; i++) { - try (java.net.Socket s = new java.net.Socket()) { - int stmo = 2 * 1000; - int utmo = 2 * 1000; - if (getSocketProperties().getSoTimeout() > stmo) - stmo = getSocketProperties().getSoTimeout(); - if (getSocketProperties().getUnlockTimeout() > utmo) - utmo = getSocketProperties().getUnlockTimeout(); - s.setSoTimeout(stmo); - s.setSoLinger(getSocketProperties().getSoLingerOn(),getSocketProperties().getSoLingerTime()); - if (getLog().isDebugEnabled()) { - getLog().debug("About to unlock socket for:" + unlockAddress); - } - s.connect(unlockAddress,utmo); - if (getDeferAccept()) { - /* - * In the case of a deferred accept / accept filters we need to - * send data to wake up the accept. Send OPTIONS * to bypass - * even BSD accept filters. The Acceptor will discard it. - */ - OutputStreamWriter sw; - - sw = new OutputStreamWriter(s.getOutputStream(), "ISO-8859-1"); - sw.write("OPTIONS * HTTP/1.0\r\n" + - "User-Agent: Tomcat wakeup connection\r\n\r\n"); - sw.flush(); - } - if (getLog().isDebugEnabled()) { - getLog().debug("Socket unlock completed for:" + unlockAddress); - } + try (java.net.Socket s = new java.net.Socket()) { + int stmo = 2 * 1000; + int utmo = 2 * 1000; + if (getSocketProperties().getSoTimeout() > stmo) + stmo = getSocketProperties().getSoTimeout(); + if (getSocketProperties().getUnlockTimeout() > utmo) + utmo = getSocketProperties().getUnlockTimeout(); + s.setSoTimeout(stmo); + s.setSoLinger(getSocketProperties().getSoLingerOn(),getSocketProperties().getSoLingerTime()); + if (getLog().isDebugEnabled()) { + getLog().debug("About to unlock socket for:" + unlockAddress); + } + s.connect(unlockAddress,utmo); + if (getDeferAccept()) { + /* + * In the case of a deferred accept / accept filters we need to + * send data to wake up the accept. Send OPTIONS * to bypass + * even BSD accept filters. The Acceptor will discard it. + */ + OutputStreamWriter sw; + + sw = new OutputStreamWriter(s.getOutputStream(), "ISO-8859-1"); + sw.write("OPTIONS * HTTP/1.0\r\n" + + "User-Agent: Tomcat wakeup connection\r\n\r\n"); + sw.flush(); + } + if (getLog().isDebugEnabled()) { + getLog().debug("Socket unlock completed for:" + unlockAddress); } } // Wait for upto 1000ms acceptor threads to unlock long waitLeft = 1000; - for (Acceptor<U> acceptor : acceptors) { - while (waitLeft > 0 && - acceptor.getState() == AcceptorState.RUNNING) { - Thread.sleep(5); - waitLeft -= 5; - } + while (waitLeft > 0 && + acceptor.getState() == AcceptorState.RUNNING) { + Thread.sleep(5); + waitLeft -= 5; } } catch(Throwable t) { ExceptionUtils.handleThrowable(t); @@ -1209,20 +1199,14 @@ public abstract class AbstractEndpoint<S,U> { } - protected void startAcceptorThreads() { - int count = getAcceptorThreadCount(); - acceptors = new ArrayList<>(count); - - for (int i = 0; i < count; i++) { - Acceptor<U> acceptor = new Acceptor<>(this); - String threadName = getName() + "-Acceptor-" + i; - acceptor.setThreadName(threadName); - acceptors.add(acceptor); - Thread t = new Thread(acceptor, threadName); - t.setPriority(getAcceptorThreadPriority()); - t.setDaemon(getDaemon()); - t.start(); - } + protected void startAcceptorThread() { + acceptor = new Acceptor<>(this); + String threadName = getName() + "-Acceptor"; + acceptor.setThreadName(threadName); + Thread t = new Thread(acceptor, threadName); + t.setPriority(getAcceptorThreadPriority()); + t.setDaemon(getDaemon()); + t.start(); } diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java index 8478e0b..77c0034 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -479,7 +479,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB sendfile.start(); } - startAcceptorThreads(); + startAcceptorThread(); } } @@ -502,27 +502,25 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB // Ignore } } - for (Acceptor<Long> acceptor : acceptors) { - long waitLeft = 10000; - while (waitLeft > 0 && - acceptor.getState() != AcceptorState.ENDED && - serverSock != 0) { - try { - Thread.sleep(50); - } catch (InterruptedException e) { - // Ignore - } - waitLeft -= 50; - } - if (waitLeft == 0) { - log.warn(sm.getString("endpoint.warn.unlockAcceptorFailed", - acceptor.getThreadName())); - // If the Acceptor is still running force - // the hard socket close. - if (serverSock != 0) { - Socket.shutdown(serverSock, Socket.APR_SHUTDOWN_READ); - serverSock = 0; - } + long waitLeft = 10000; + while (waitLeft > 0 && + acceptor.getState() != AcceptorState.ENDED && + serverSock != 0) { + try { + Thread.sleep(50); + } catch (InterruptedException e) { + // Ignore + } + waitLeft -= 50; + } + if (waitLeft == 0) { + log.warn(sm.getString("endpoint.warn.unlockAcceptorFailed", + acceptor.getThreadName())); + // If the Acceptor is still running force + // the hard socket close. + if (serverSock != 0) { + Socket.shutdown(serverSock, Socket.APR_SHUTDOWN_READ); + serverSock = 0; } } try { diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java index c307369..d61803c 100644 --- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java +++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java @@ -34,7 +34,6 @@ import java.nio.channels.NetworkChannel; import java.nio.channels.ReadPendingException; import java.nio.channels.WritePendingException; import java.nio.file.StandardOpenOption; -import java.util.ArrayList; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; @@ -89,8 +88,6 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS */ private SynchronizedStack<Nio2Channel> nioChannels; - private Nio2Acceptor acceptor = null; - // ------------------------------------------------------------- Properties @@ -180,19 +177,17 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS } initializeConnectionLatch(); - startAcceptorThreads(); + startAcceptorThread(); } } @Override - protected void startAcceptorThreads() { + protected void startAcceptorThread() { // Instead of starting a real acceptor thread, this will instead call // an asynchronous accept operation if (acceptor == null) { - acceptors = new ArrayList<>(1); acceptor = new Nio2Acceptor(this); - acceptor.setThreadName(getName() + "-Acceptor-0"); - acceptors.add(acceptor); + acceptor.setThreadName(getName() + "-Acceptor"); } acceptor.state = AcceptorState.RUNNING; getExecutor().execute(acceptor); diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java index 1910bed..1dac62f 100644 --- a/java/org/apache/tomcat/util/net/NioEndpoint.java +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java @@ -300,7 +300,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> pollerThread.start(); } - startAcceptorThreads(); + startAcceptorThread(); } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 273e3d1..f05b40d 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -69,6 +69,12 @@ <bug>63412</bug>: Security manager failure when using the async IO API from a webapp. (remm) </fix> + <fix> + Remove <code>acceptorThreadCount</code> Connector attribute, + one accept thread is sufficient. As documented, value <code>2</code> + was the only other sensible value, but without and impact beyond + certain microbenchmarks. (remm) + </fix> </changelog> </subsection> <subsection name="Other"> diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml index 0470ca7..d028861 100644 --- a/webapps/docs/config/http.xml +++ b/webapps/docs/config/http.xml @@ -294,14 +294,6 @@ value is 100.</p> </attribute> - <attribute name="acceptorThreadCount" required="false"> - <p>The number of threads to be used to accept connections. Increase this - value on a multi CPU machine, although you would never really need more - than <code>2</code>. Also, with a lot of non keep alive connections, you - might want to increase this value as well. Default value is - <code>1</code>.</p> - </attribute> - <attribute name="acceptorThreadPriority" required="false"> <p>The priority of the acceptor threads. The threads used to accept new connections. The default value is <code>5</code> (the value of the --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org