8193370: Provide more user friendly defaults for HTTP/2 client settings
Reviewed-by: chegar
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http2ClientImpl.java Wed Dec 13 07:51:57 2017 -0800
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http2ClientImpl.java Wed Dec 13 16:16:17 2017 +0000
@@ -36,6 +36,8 @@
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 @@
}
}
-// 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 @@
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(
- "jdk.httpclient.hpack.maxheadertablesize", 16 * K));
- frame.setParameter(ENABLE_PUSH, Utils.getIntegerNetProperty(
- "jdk.httpclient.enablepush", 1));
- frame.setParameter(MAX_CONCURRENT_STREAMS, Utils.getIntegerNetProperty(
- "jdk.httpclient.maxstreams", 16));
- frame.setParameter(INITIAL_WINDOW_SIZE, Utils.getIntegerNetProperty(
- "jdk.httpclient.windowsize", 64 * K - 1));
- frame.setParameter(MAX_FRAME_SIZE, Utils.getIntegerNetProperty(
- "jdk.httpclient.maxframesize", 16 * K));
+ // default defined for HTTP/2 is 4 K, we use 16 K.
+ frame.setParameter(HEADER_TABLE_SIZE, getParameter(
+ "jdk.httpclient.hpack.maxheadertablesize",
+ 0, Integer.MAX_VALUE, 16 * K));
+ // O: does not accept push streams. 1: accepts push streams.
+ frame.setParameter(ENABLE_PUSH, getParameter(
+ "jdk.httpclient.enablepush",
+ 0, 1, 1));
+ // HTTP/2 recommends to set the number of concurrent streams
+ // 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;
}
}
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http2Connection.java Wed Dec 13 07:51:57 2017 -0800
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http2Connection.java Wed Dec 13 16:16:17 2017 +0000
@@ -228,7 +228,7 @@
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 @@
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 @@
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 @@
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 @@
// 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);
- windowUpdater.sendWindowUpdate(len);
+ final int len = windowUpdater.initialWindowSize - initialWindowSize;
+ 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 @@
static final class ConnectionWindowUpdateSender extends WindowUpdateSender {
+ final int initialWindowSize;
public ConnectionWindowUpdateSender(Http2Connection connection,
int initialWindowSize) {
super(connection, initialWindowSize);
+ this.initialWindowSize = initialWindowSize;
}
@Override
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClientImpl.java Wed Dec 13 07:51:57 2017 -0800
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClientImpl.java Wed Dec 13 16:16:17 2017 +0000
@@ -1038,7 +1038,7 @@
// used for the connection window
int getReceiveBufferSize() {
return Utils.getIntegerNetProperty(
- "jdk.httpclient.connectionWindowSize", 256 * 1024
+ "jdk.httpclient.receiveBufferSize", 2 * 1024 * 1024
);
}
}
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java Wed Dec 13 07:51:57 2017 -0800
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java Wed Dec 13 16:16:17 2017 +0000
@@ -143,7 +143,9 @@
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 @@
}
}
+ 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; }
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/WindowUpdateSender.java Wed Dec 13 07:51:57 2017 -0800
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/WindowUpdateSender.java Wed Dec 13 16:16:17 2017 +0000
@@ -59,6 +59,8 @@
// 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();
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/frame/SettingsFrame.java Wed Dec 13 07:51:57 2017 -0800
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/frame/SettingsFrame.java Wed Dec 13 16:16:17 2017 +0000
@@ -100,6 +100,11 @@
this(0);
}
+ public SettingsFrame(SettingsFrame other) {
+ super(0, other.flags);
+ parameters = Arrays.copyOf(other.parameters, MAX_PARAM);
+ }
+
@Override
public int type() {
return TYPE;
--- a/test/jdk/java/net/httpclient/security/filePerms/httpclient.policy Wed Dec 13 07:51:57 2017 -0800
+++ b/test/jdk/java/net/httpclient/security/filePerms/httpclient.policy Wed Dec 13 16:16:17 2017 +0000
@@ -49,6 +49,7 @@
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";
--- a/test/jdk/java/net/httpclient/websocket/security/httpclient.policy Wed Dec 13 07:51:57 2017 -0800
+++ b/test/jdk/java/net/httpclient/websocket/security/httpclient.policy Wed Dec 13 16:16:17 2017 +0000
@@ -49,6 +49,7 @@
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";