# HG changeset patch # User dfuchs # Date 1513181777 0 # Node ID a559b7cd1dea1c2ee8b3c3de5cb8b7c29b747858 # Parent daf3b49f4839ddd31ca396132aff302f634583c0 8193370: Provide more user friendly defaults for HTTP/2 client settings Reviewed-by: chegar diff -r daf3b49f4839 -r a559b7cd1dea src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http2ClientImpl.java --- 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; } } diff -r daf3b49f4839 -r a559b7cd1dea src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http2Connection.java --- 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 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 diff -r daf3b49f4839 -r a559b7cd1dea src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClientImpl.java --- 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 ); } } diff -r daf3b49f4839 -r a559b7cd1dea src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/PlainHttpConnection.java --- 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; } diff -r daf3b49f4839 -r a559b7cd1dea src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/WindowUpdateSender.java --- 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(); diff -r daf3b49f4839 -r a559b7cd1dea src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/frame/SettingsFrame.java --- 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; diff -r daf3b49f4839 -r a559b7cd1dea test/jdk/java/net/httpclient/security/filePerms/httpclient.policy --- 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"; diff -r daf3b49f4839 -r a559b7cd1dea test/jdk/java/net/httpclient/websocket/security/httpclient.policy --- 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";