[tomcat] branch master updated: Change cache default values for endpoints

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[tomcat] branch master updated: Change cache default values for endpoints

Rémy Maucherat
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 e8bb4b8  Change cache default values for endpoints
e8bb4b8 is described below

commit e8bb4b8672cc06743a4e1950eb30557b277756c8
Author: remm <[hidden email]>
AuthorDate: Tue Jan 14 14:05:08 2020 +0100

    Change cache default values for endpoints
   
    bufferPoolSize was not used at all, so fix that. It is now used
    depending on the bufferPool value. By default it will use a reasonable
    amount of the available memory as cache. Use an upper bound to estimate
    the buffer use for TLS.
    Simple benchmarks did not show any meaningful difference with any
    particular configuration, however it is also rather obvious TLS can lead
    to a lot of buffer allocation and GC without caching.
---
 TOMCAT-NEXT.txt                                    |  2 -
 java/org/apache/tomcat/util/net/Nio2Endpoint.java  |  7 ++-
 java/org/apache/tomcat/util/net/NioEndpoint.java   |  6 +-
 .../apache/tomcat/util/net/SocketProperties.java   | 70 +++++++++++++++-------
 webapps/docs/changelog.xml                         |  7 +++
 webapps/docs/config/http.xml                       | 67 +++++++--------------
 6 files changed, 84 insertions(+), 75 deletions(-)

diff --git a/TOMCAT-NEXT.txt b/TOMCAT-NEXT.txt
index 271ae5e..ffee966 100644
--- a/TOMCAT-NEXT.txt
+++ b/TOMCAT-NEXT.txt
@@ -65,6 +65,4 @@ New items for 10.0.x onwards:
 
 14. Remove unused NIO blocking code.
 
-15. Change SocketProperties cache sizes default values.
-
 16. Share configuration between HTTP/1.1 and nested HTTP/2 rather than duplicating.
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index adafdad..33037fb 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -158,11 +158,12 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS
                 processorCache = new SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE,
                         socketProperties.getProcessorCache());
             }
-            if (socketProperties.getBufferPool() != 0) {
+            int actualBufferPool =
+                    socketProperties.getActualBufferPool(isSSLEnabled() ? getSniParseLimit() * 2 : 0);
+            if (actualBufferPool != 0) {
                 nioChannels = new SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE,
-                        socketProperties.getBufferPool());
+                        actualBufferPool);
             }
-
             // Create worker collection
             if (getExecutor() == null) {
                 createExecutor();
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 9a70075..c72a15c 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -259,9 +259,11 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
                 eventCache = new SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE,
                         socketProperties.getEventCache());
             }
-            if (socketProperties.getBufferPool() != 0) {
+            int actualBufferPool =
+                    socketProperties.getActualBufferPool(isSSLEnabled() ? getSniParseLimit() * 2 : 0);
+            if (actualBufferPool != 0) {
                 nioChannels = new SynchronizedStack<>(SynchronizedStack.DEFAULT_SIZE,
-                        socketProperties.getBufferPool());
+                        actualBufferPool);
             }
 
             // Create worker collection
diff --git a/java/org/apache/tomcat/util/net/SocketProperties.java b/java/org/apache/tomcat/util/net/SocketProperties.java
index bfed3ac..0d6d89a 100644
--- a/java/org/apache/tomcat/util/net/SocketProperties.java
+++ b/java/org/apache/tomcat/util/net/SocketProperties.java
@@ -39,9 +39,8 @@ public class SocketProperties {
      * Default is 500
      * -1 is unlimited
      * 0 is disabled
-     * TODO: The default will be changed to 0 in Tomcat 10
      */
-    protected int processorCache = 500;
+    protected int processorCache = 0;
 
     /**
      * Enable/disable poller event cache, this bounded cache stores
@@ -50,9 +49,8 @@ public class SocketProperties {
      * -1 is unlimited
      * 0 is disabled
      * &gt;0 the max number of objects to keep in cache.
-     * TODO: The default will be changed to 0 in Tomcat 10
      */
-    protected int eventCache = 500;
+    protected int eventCache = 0;
 
     /**
      * Enable/disable direct buffers for the network buffers
@@ -93,29 +91,20 @@ public class SocketProperties {
     /**
      * NioChannel pool size for the endpoint,
      * this value is how many channels
-     * -1 means unlimited cached, 0 means no cache
-     * Default value is 500
-     * TODO: The default should be changed in Tomcat 10, actually it should be
-     *   bufferPoolSize / (appReadBufSize + appWriteBufSize), assuming the SSL
-     *   buffers are ignored (that would be logical), and the value would be 6400.
-     *   So the default value will be changed to a new default value like -2 to
-     *   set a dynamic value based on bufferPoolSize in that case.
+     * -1 means unlimited cached, 0 means no cache,
+     * -2 means bufferPoolSize will be used
+     * Default value is -2
      */
-    protected int bufferPool = 500;
+    protected int bufferPool = -2;
 
     /**
      * Buffer pool size in bytes to be cached
      * -1 means unlimited, 0 means no cache
-     * Default value is 100MB (1024*1024*100 bytes)
-     * TODO: The default value to be used could rather be based on the
-     *   JVM max heap, otherwise it could be a problem in some
-     *   environments. Big servers also need to use a much higher default,
-     *   while small cloud based ones should use 0 instead.
-     *   Possible default value strategy:
-     *     heap inf 1GB: 0
-     *     heap sup 1GB: heap / 32
+     * Default value is based on the max memory reported by the JVM,
+     * if less than 1GB, then 0, else the value divided by 32. This value
+     * will then be used to compute bufferPool if its value is -2
      */
-    protected int bufferPoolSize = 1024*1024*100;
+    protected int bufferPoolSize = -2;
 
     /**
      * TCP_NO_DELAY option. JVM default used if not set.
@@ -445,6 +434,45 @@ public class SocketProperties {
         this.unlockTimeout = unlockTimeout;
     }
 
+    /**
+     * Get the actual buffer pool size to use.
+     * @param bufferOverhead When TLS is enabled, additional network buffers
+     *   are needed and will be added to the application buffer size
+     * @return the actual buffer pool size that will be used
+     */
+    public int getActualBufferPool(int bufferOverhead) {
+        if (bufferPool != -2) {
+            return bufferPool;
+        } else {
+            if (bufferPoolSize == -1) {
+                return -1;
+            } else if (bufferPoolSize == 0) {
+                return 0;
+            } else {
+                long actualBufferPoolSize = bufferPoolSize;
+                long poolSize = 0;
+                if (actualBufferPoolSize == -2) {
+                    long maxMemory = Runtime.getRuntime().maxMemory();
+                    if (maxMemory > 1024 * 1024 * 1024) {
+                        actualBufferPoolSize = maxMemory / 32;
+                    } else {
+                        return 0;
+                    }
+                }
+                int bufSize = appReadBufSize + appWriteBufSize + bufferOverhead;
+                if (bufSize == 0) {
+                    return 0;
+                }
+                poolSize = actualBufferPoolSize / (bufSize);
+                if (poolSize > Integer.MAX_VALUE) {
+                    return Integer.MAX_VALUE;
+                } else {
+                    return (int) poolSize;
+                }
+            }
+        }
+    }
+
     void setObjectName(ObjectName oname) {
         this.oname = oname;
     }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 210194f..8e105d9 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -53,6 +53,13 @@
       </update>
     </changelog>
   </subsection>
+  <subsection name="Coyote">
+    <changelog>
+      <update>
+        Update endpoint cache sizes defaults. (remm)
+      </update>
+    </changelog>
+  </subsection>
 </section>
 </body>
 </document>
diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml
index 057ee47..c33ae2c 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -813,72 +813,45 @@
       </attribute>
 
       <attribute name="socket.bufferPool" required="false">
-        <p>(int)The NIO connector uses a class called NioChannel that holds
-        elements linked to a socket. To reduce garbage collection, the NIO
+        <p>(int)The NIOx connector uses a class called NioXChannel that holds
+        elements linked to a socket. To reduce garbage collection, the NIOx
         connector caches these channel objects. This value specifies the size of
-        this cache. The default value is <code>500</code>, and represents that
-        the cache will hold 500 NioChannel objects. Other values are
-        <code>-1</code> for unlimited cache and <code>0</code> for no cache.</p>
+        this cache. The default value is <code>-2</code>. Special values are
+        <code>-1</code> for unlimited cache, <code>0</code> for no cache,
+        and <code>-2</code> for a value computed using the bufferPoolSize
+        attribute.</p>
       </attribute>
 
       <attribute name="socket.bufferPoolSize" required="false">
-        <p>(int)The NioChannel pool can also be size based, not used object
-        based. The size is calculated as follows:<br/>
-        NioChannel
+        <p>(int)The NioXChannel pool can also be size based, not used object
+        based. If bufferPool is not -2, then this value will not be used.<br/>
+        The value is in bytes except for special values. Special values are
+        <code>-1</code> for unlimited cache, <code>0</code> for no cache,
+        and <code>-2</code> for a value computed as follows:<br/>
+        NioXChannel
         <code>buffer size = read buffer size + write buffer size</code><br/>
-        SecureNioChannel <code>buffer size = application read buffer size +
-        application write buffer size + network read buffer size +
-        network write buffer size</code><br/>
-        The value is in bytes, the default value is <code>1024*1024*100</code>
-        (100MB).</p>
+        SecureNioXChannel <code>buffer size = application read buffer size +
+        application write buffer size + twice the max SNI parse size</code>.
+        If the maximum memory as reported by the runtime is greater than
+        1GB, then the pool size value is the memory divided by the buffer
+        size. Otherwise, it will be 0.
+        </p>
       </attribute>
 
       <attribute name="socket.processorCache" required="false">
         <p>(int)Tomcat will cache SocketProcessor objects to reduce garbage
         collection. The integer value specifies how many objects to keep in the
-        cache at most. The default is <code>500</code>. Other values are
-        <code>-1</code> for unlimited cache and <code>0</code> for no cache.</p>
-      </attribute>
-
-      <attribute name="socket.keyCache" required="false">
-        <p>(int)Tomcat will cache KeyAttachment objects to reduce garbage
-        collection. The integer value specifies how many objects to keep in the
-        cache at most. The default is <code>500</code>. Other values are
+        cache at most. The default is <code>0</code>. Special values are
         <code>-1</code> for unlimited cache and <code>0</code> for no cache.</p>
       </attribute>
 
       <attribute name="socket.eventCache" required="false">
         <p>(int)Tomcat will cache PollerEvent objects to reduce garbage
         collection. The integer value specifies how many objects to keep in the
-        cache at most. The default is <code>500</code>. Other values are
+        cache at most. The default is <code>0</code>. Special values are
         <code>-1</code> for unlimited cache and <code>0</code> for no cache.</p>
       </attribute>
 
-      <attribute name="selectorPool.maxSelectors" required="false">
-        <p>(int)The max selectors to be used in the pool, to reduce selector
-        contention. Use this option when the command line
-        <code>org.apache.tomcat.util.net.NioSelectorShared</code> value is set
-        to false. Default value is <code>200</code>.</p>
-      </attribute>
-
-      <attribute name="selectorPool.maxSpareSelectors" required="false">
-        <p>(int)The max spare selectors to be used in the pool, to reduce
-        selector contention. When a selector is returned to the pool, the system
-        can decide to keep it or let it be GC'd. Use this option when the
-        command line <code>org.apache.tomcat.util.net.NioSelectorShared</code>
-        value is set to false. Default value is <code>-1</code> (unlimited).</p>
-      </attribute>
-
-      <attribute name="selectorPool.shared" required="false">
-        <p>(bool)Set this value to <code>false</code> if you wish to
-        use a selector for each thread. When you set it to <code>false</code>, you can
-        control the size of the pool of selectors by using the
-        <strong>selectorPool.maxSelectors</strong> attribute.
-        Default is <code>true</code> or the value of the
-        <code>org.apache.tomcat.util.net.NioSelectorShared</code> system
-        property if present.</p>
-      </attribute>
-
       <attribute name="useInheritedChannel" required="false">
         <p>(bool)Defines if this connector should inherit an inetd/systemd network socket.
         Only one connector can inherit a network socket. This can option can be


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]