8193370: Provide more user friendly defaults for HTTP/2 client settings

Reviewed-by: chegar

src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http2ClientImpl.java

@@ -36,6 +36,8 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CompletableFuture;
+
+import jdk.incubator.http.internal.common.Log;
 import jdk.incubator.http.internal.common.MinimalFuture;
 import jdk.incubator.http.internal.common.Utils;
 import jdk.incubator.http.internal.frame.SettingsFrame;
@@ -78,10 +80,6 @@ class Http2ClientImpl {
         }
     }
 
-//    boolean haveConnectionFor(URI uri, InetSocketAddress proxy) {
-//        return connections.containsKey(Http2Connection.keyFor(uri,proxy));
-//    }
-
     /**
      * If a https request then async waits until a connection is opened.
      * Returns null if the request is 'http' as a different (upgrade)
@@ -188,18 +186,64 @@ class Http2ClientImpl {
 
     private static final int K = 1024;
 
+    private static int getParameter(String property, int min, int max, int defaultValue) {
+        int value =  Utils.getIntegerNetProperty(property, defaultValue);
+        // use default value if misconfigured
+        if (value < min || value > max) {
+            Log.logError("Property value for {0}={1} not in [{2}..{3}]: " +
+                    "using default={4}", property, value, min, max, defaultValue);
+            value = defaultValue;
+        }
+        return value;
+    }
+
+    // used for the connection window, to have a connection window size
+    // bigger than the initial stream window size.
+    int getConnectionWindowSize(SettingsFrame clientSettings) {
+        // Maximum size is 2^31-1. Don't allow window size to be less
+        // than the stream window size. HTTP/2 specify a default of 64 * K -1,
+        // but we use 2^26 by default for better performance.
+        int streamWindow = clientSettings.getParameter(INITIAL_WINDOW_SIZE);
+
+        // The default is the max between the stream window size
+        // and the connection window size.
+        int defaultValue = Math.min(Integer.MAX_VALUE,
+                Math.max(streamWindow, K*K*32));
+
+        return getParameter(
+                "jdk.httpclient.connectionWindowSize",
+                streamWindow, Integer.MAX_VALUE, defaultValue);
+    }
+
     SettingsFrame getClientSettings() {
         SettingsFrame frame = new SettingsFrame();
-        frame.setParameter(HEADER_TABLE_SIZE, Utils.getIntegerNetProperty(
+        // default defined for HTTP/2 is 4 K, we use 16 K.
-                "jdk.httpclient.hpack.maxheadertablesize", 16 * K));
+        frame.setParameter(HEADER_TABLE_SIZE, getParameter(
-        frame.setParameter(ENABLE_PUSH, Utils.getIntegerNetProperty(
+                "jdk.httpclient.hpack.maxheadertablesize",
-            "jdk.httpclient.enablepush", 1));
+                0, Integer.MAX_VALUE, 16 * K));
-        frame.setParameter(MAX_CONCURRENT_STREAMS, Utils.getIntegerNetProperty(
+        // O: does not accept push streams. 1: accepts push streams.
-            "jdk.httpclient.maxstreams", 16));
+        frame.setParameter(ENABLE_PUSH, getParameter(
-        frame.setParameter(INITIAL_WINDOW_SIZE, Utils.getIntegerNetProperty(
+                "jdk.httpclient.enablepush",
-            "jdk.httpclient.windowsize", 64 * K - 1));
+                0, 1, 1));
-        frame.setParameter(MAX_FRAME_SIZE, Utils.getIntegerNetProperty(
+        // HTTP/2 recommends to set the number of concurrent streams
-            "jdk.httpclient.maxframesize", 16 * K));
+        // no lower than 100. We use 100. 0 means no stream would be
+        // accepted. That would render the client to be non functional,
+        // so we won't let 0 be configured for our Http2ClientImpl.
+        frame.setParameter(MAX_CONCURRENT_STREAMS, getParameter(
+                "jdk.httpclient.maxstreams",
+                1, Integer.MAX_VALUE, 100));
+        // Maximum size is 2^31-1. Don't allow window size to be less
+        // than the minimum frame size as this is likely to be a
+        // configuration error. HTTP/2 specify a default of 64 * K -1,
+        // but we use 16 M  for better performance.
+        frame.setParameter(INITIAL_WINDOW_SIZE, getParameter(
+                "jdk.httpclient.windowsize",
+                16 * K, Integer.MAX_VALUE, 16*K*K));
+        // HTTP/2 specify a minimum size of 16 K, a maximum size of 2^24-1,
+        // and a default of 16 K. We use 16 K as default.
+        frame.setParameter(MAX_FRAME_SIZE, getParameter(
+                "jdk.httpclient.maxframesize",
+                16 * K, 16 * K * K -1, 16 * K));
         return frame;
     }
 }

src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http2Connection.java

@@ -228,7 +228,7 @@ class Http2Connection  {
     private final WindowController windowController = new WindowController();
     private final FramesController framesController = new FramesController();
     private final Http2TubeSubscriber subscriber = new Http2TubeSubscriber();
-    final WindowUpdateSender windowUpdater;
+    final ConnectionWindowUpdateSender windowUpdater;
     private volatile Throwable cause;
     private volatile Supplier<ByteBuffer> initial;
 
@@ -247,7 +247,8 @@ class Http2Connection  {
         this.nextstreamid = nextstreamid;
         this.key = key;
         this.clientSettings = this.client2.getClientSettings();
-        this.framesDecoder = new FramesDecoder(this::processFrame, clientSettings.getParameter(SettingsFrame.MAX_FRAME_SIZE));
+        this.framesDecoder = new FramesDecoder(this::processFrame,
+                clientSettings.getParameter(SettingsFrame.MAX_FRAME_SIZE));
         // serverSettings will be updated by server
         this.serverSettings = SettingsFrame.getDefaultSettings();
         this.hpackOut = new Encoder(serverSettings.getParameter(HEADER_TABLE_SIZE));
@@ -255,7 +256,8 @@ class Http2Connection  {
         debugHpack.log(Level.DEBUG, () -> "For the record:" + super.toString());
         debugHpack.log(Level.DEBUG, "Decoder created: %s", hpackIn);
         debugHpack.log(Level.DEBUG, "Encoder created: %s", hpackOut);
-        this.windowUpdater = new ConnectionWindowUpdateSender(this, client().getReceiveBufferSize());
+        this.windowUpdater = new ConnectionWindowUpdateSender(this,
+                client2.getConnectionWindowSize(clientSettings));
     }
 
     /**
@@ -774,7 +776,8 @@ class Http2Connection  {
         Log.logTrace("{0}: start sending connection preface to {1}",
                      connection.channel().getLocalAddress(),
                      connection.address());
-        SettingsFrame sf = client2.getClientSettings();
+        SettingsFrame sf = new SettingsFrame(clientSettings);
+        int initialWindowSize = sf.getParameter(INITIAL_WINDOW_SIZE);
         ByteBuffer buf = framesEncoder.encodeConnectionPreface(PREFACE_BYTES, sf);
         Log.logFrames(sf, "OUT");
         // send preface bytes and SettingsFrame together
@@ -788,8 +791,10 @@ class Http2Connection  {
 
         // send a Window update for the receive buffer we are using
         // minus the initial 64 K specified in protocol
-        final int len = client2.client().getReceiveBufferSize() - (64 * 1024 - 1);
+        final int len = windowUpdater.initialWindowSize - initialWindowSize;
-        windowUpdater.sendWindowUpdate(len);
+        if (len > 0) {
+            windowUpdater.sendWindowUpdate(len);
+        }
         // there will be an ACK to the windows update - which should
         // cause any pending data stored before the preface was sent to be
         // flushed (see PrefaceController).
@@ -1202,9 +1207,11 @@ class Http2Connection  {
 
     static final class ConnectionWindowUpdateSender extends WindowUpdateSender {
 
+        final int initialWindowSize;
         public ConnectionWindowUpdateSender(Http2Connection connection,
                                             int initialWindowSize) {
             super(connection, initialWindowSize);
+            this.initialWindowSize = initialWindowSize;
         }
 
         @Override

src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClientImpl.java

@@ -1038,7 +1038,7 @@ class HttpClientImpl extends HttpClient {
     // used for the connection window
     int getReceiveBufferSize() {
         return Utils.getIntegerNetProperty(
-                "jdk.httpclient.connectionWindowSize", 256 * 1024
+                "jdk.httpclient.receiveBufferSize", 2 * 1024 * 1024
         );
     }
 }

src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java

@@ -143,7 +143,9 @@ class PlainHttpConnection extends HttpConnection {
             this.chan = SocketChannel.open();
             chan.configureBlocking(false);
             int bufsize = client.getReceiveBufferSize();
-            chan.setOption(StandardSocketOptions.SO_RCVBUF, bufsize);
+            if (!trySetReceiveBufferSize(bufsize)) {
+                trySetReceiveBufferSize(256*1024);
+            }
             chan.setOption(StandardSocketOptions.TCP_NODELAY, true);
             // wrap the connected channel in a Tube for async reading and writing
             tube = new SocketTube(client(), chan, Utils::getBuffer);
@@ -152,6 +154,18 @@ class PlainHttpConnection extends HttpConnection {
         }
     }
 
+    private boolean trySetReceiveBufferSize(int bufsize) {
+        try {
+            chan.setOption(StandardSocketOptions.SO_RCVBUF, bufsize);
+            return true;
+        } catch(IOException x) {
+            debug.log(Level.DEBUG,
+                    "Failed to set receive buffer size to %d on %s",
+                    bufsize, chan);
+        }
+        return false;
+    }
+
     @Override
     HttpPublisher publisher() { return writePublisher; }
 

src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/WindowUpdateSender.java

@@ -59,6 +59,8 @@ abstract class WindowUpdateSender {
         //   or
         // - remaining window size reached max frame size.
         limit = Math.min(v0, v1);
+        debug.log(Level.DEBUG, "maxFrameSize=%d, initWindowSize=%d, limit=%d",
+                maxFrameSize, initWindowSize, limit);
     }
 
     abstract int getStreamId();

src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/frame/SettingsFrame.java

@@ -100,6 +100,11 @@ public class SettingsFrame extends Http2Frame {
         this(0);
     }
 
+    public SettingsFrame(SettingsFrame other) {
+        super(0, other.flags);
+        parameters = Arrays.copyOf(other.parameters, MAX_PARAM);
+    }
+
     @Override
     public int type() {
         return TYPE;

test/jdk/java/net/httpclient/security/filePerms/httpclient.policy

@@ -49,6 +49,7 @@ grant codeBase "jrt:/jdk.incubator.httpclient" {
     permission java.util.PropertyPermission "jdk.httpclient.maxstreams","read";
     permission java.util.PropertyPermission "jdk.httpclient.redirects.retrylimit","read";
     permission java.util.PropertyPermission "jdk.httpclient.windowsize","read";
+    permission java.util.PropertyPermission "jdk.httpclient.receiveBufferSize","read";
     permission java.util.PropertyPermission "jdk.httpclient.bufsize","read";
     permission java.util.PropertyPermission "jdk.httpclient.internal.selector.timeout","read";
     permission java.util.PropertyPermission "jdk.internal.httpclient.debug","read";

test/jdk/java/net/httpclient/websocket/security/httpclient.policy

@@ -49,6 +49,7 @@ grant codeBase "jrt:/jdk.incubator.httpclient" {
     permission java.util.PropertyPermission "jdk.httpclient.maxstreams","read";
     permission java.util.PropertyPermission "jdk.httpclient.redirects.retrylimit","read";
     permission java.util.PropertyPermission "jdk.httpclient.windowsize","read";
+    permission java.util.PropertyPermission "jdk.httpclient.receiveBufferSize","read";
     permission java.util.PropertyPermission "jdk.httpclient.bufsize","read";
     permission java.util.PropertyPermission "jdk.httpclient.internal.selector.timeout","read";
     permission java.util.PropertyPermission "jdk.internal.httpclient.debug","read";