From a87c86dc0d8611fc66b66f0a3d8441a7dee7ca1b Mon Sep 17 00:00:00 2001 From: nmittler Date: Fri, 10 Apr 2015 09:13:32 -0700 Subject: [PATCH] Change Http2Settings to use char keys. Motivation: Now that we have a CharObjectHashMap, we should change Http2Settings to use it. Modifications: Changed Http2Settings to extend CharObjectHashMap rather than IntObjectHashMap. Result: Http2Settings uses less memory to store keys. --- .../codec/http2/DefaultHttp2FrameReader.java | 2 +- .../codec/http2/DefaultHttp2FrameWriter.java | 8 +++---- .../codec/http2/Http2ClientUpgradeCodec.java | 5 +++-- .../handler/codec/http2/Http2CodecUtil.java | 12 +++++------ .../handler/codec/http2/Http2Settings.java | 21 ++++++++++--------- .../codec/http2/Http2SettingsTest.java | 12 +++++++++-- 6 files changed, 35 insertions(+), 25 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java index 94e1bce298..bff86df733 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java @@ -482,7 +482,7 @@ public class DefaultHttp2FrameReader implements Http2FrameReader, Http2FrameSize int numSettings = payloadLength / SETTING_ENTRY_LENGTH; Http2Settings settings = new Http2Settings(); for (int index = 0; index < numSettings; ++index) { - int id = payload.readUnsignedShort(); + char id = (char) payload.readUnsignedShort(); long value = payload.readUnsignedInt(); try { settings.put(id, value); diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java index ae9b7919ca..036537aa2d 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameWriter.java @@ -52,13 +52,14 @@ import static io.netty.handler.codec.http2.Http2FrameTypes.RST_STREAM; import static io.netty.handler.codec.http2.Http2FrameTypes.SETTINGS; import static io.netty.handler.codec.http2.Http2FrameTypes.WINDOW_UPDATE; import static io.netty.util.internal.ObjectUtil.checkNotNull; + import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPromise; import io.netty.handler.codec.http2.Http2CodecUtil.SimpleChannelPromiseAggregator; import io.netty.handler.codec.http2.Http2FrameWriter.Configuration; -import io.netty.util.collection.IntObjectMap; +import io.netty.util.collection.CharObjectMap; /** * A {@link Http2FrameWriter} that supports all frame types defined by the HTTP/2 specification. @@ -121,7 +122,6 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize public ChannelFuture writeData(ChannelHandlerContext ctx, int streamId, ByteBuf data, int padding, boolean endStream, ChannelPromise promise) { boolean releaseData = true; - ByteBuf buf = null; SimpleChannelPromiseAggregator promiseAggregator = new SimpleChannelPromiseAggregator(promise, ctx.channel(), ctx.executor()); try { @@ -133,7 +133,7 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize int payloadLength = data.readableBytes() + padding + flags.getPaddingPresenceFieldLength(); verifyPayloadLength(payloadLength); - buf = ctx.alloc().buffer(DATA_FRAME_HEADER_LENGTH); + ByteBuf buf = ctx.alloc().buffer(DATA_FRAME_HEADER_LENGTH); writeFrameHeaderInternal(buf, payloadLength, DATA, flags, streamId); writePaddingLength(buf, padding); ctx.write(buf, promiseAggregator.newPromise()); @@ -213,7 +213,7 @@ public class DefaultHttp2FrameWriter implements Http2FrameWriter, Http2FrameSize int payloadLength = SETTING_ENTRY_LENGTH * settings.size(); ByteBuf buf = ctx.alloc().buffer(FRAME_HEADER_LENGTH + settings.size() * SETTING_ENTRY_LENGTH); writeFrameHeaderInternal(buf, payloadLength, SETTINGS, new Http2Flags(), 0); - for (IntObjectMap.Entry entry : settings.entries()) { + for (CharObjectMap.Entry entry : settings.entries()) { writeUnsignedShort(entry.key(), buf); writeUnsignedInt(entry.value(), buf); } diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ClientUpgradeCodec.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ClientUpgradeCodec.java index 1d57bacaf1..9476e59161 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ClientUpgradeCodec.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ClientUpgradeCodec.java @@ -23,13 +23,14 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.writeUnsignedShort; import static io.netty.util.CharsetUtil.UTF_8; import static io.netty.util.ReferenceCountUtil.release; import static io.netty.util.internal.ObjectUtil.checkNotNull; + import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.base64.Base64; import io.netty.handler.codec.http.FullHttpResponse; import io.netty.handler.codec.http.HttpClientUpgradeHandler; import io.netty.handler.codec.http.HttpRequest; -import io.netty.util.collection.IntObjectMap; +import io.netty.util.collection.CharObjectHashMap; import java.util.Collection; import java.util.Collections; @@ -104,7 +105,7 @@ public class Http2ClientUpgradeCodec implements HttpClientUpgradeHandler.Upgrade // Serialize the payload of the SETTINGS frame. int payloadLength = SETTING_ENTRY_LENGTH * settings.size(); buf = ctx.alloc().buffer(payloadLength); - for (IntObjectMap.Entry entry : settings.entries()) { + for (CharObjectHashMap.Entry entry : settings.entries()) { writeUnsignedShort(entry.key(), buf); writeUnsignedInt(entry.value(), buf); } diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java index 88bdd859ba..19b27f5938 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java @@ -63,12 +63,12 @@ public final class Http2CodecUtil { public static final int WINDOW_UPDATE_FRAME_LENGTH = FRAME_HEADER_LENGTH + INT_FIELD_LENGTH; public static final int CONTINUATION_FRAME_HEADER_LENGTH = FRAME_HEADER_LENGTH + MAX_PADDING_LENGTH_LENGTH; - public static final int SETTINGS_HEADER_TABLE_SIZE = 1; - public static final int SETTINGS_ENABLE_PUSH = 2; - public static final int SETTINGS_MAX_CONCURRENT_STREAMS = 3; - public static final int SETTINGS_INITIAL_WINDOW_SIZE = 4; - public static final int SETTINGS_MAX_FRAME_SIZE = 5; - public static final int SETTINGS_MAX_HEADER_LIST_SIZE = 6; + public static final char SETTINGS_HEADER_TABLE_SIZE = 1; + public static final char SETTINGS_ENABLE_PUSH = 2; + public static final char SETTINGS_MAX_CONCURRENT_STREAMS = 3; + public static final char SETTINGS_INITIAL_WINDOW_SIZE = 4; + public static final char SETTINGS_MAX_FRAME_SIZE = 5; + public static final char SETTINGS_MAX_HEADER_LIST_SIZE = 6; public static final int NUM_STANDARD_SETTINGS = 6; public static final int MAX_HEADER_TABLE_SIZE = Integer.MAX_VALUE; // Size limited by HPACK library diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Settings.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Settings.java index c2fd28787d..dc3af932cb 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Settings.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Settings.java @@ -16,13 +16,13 @@ package io.netty.handler.codec.http2; import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_CONCURRENT_STREAMS; +import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_HEADER_LIST_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_HEADER_TABLE_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_INITIAL_WINDOW_SIZE; -import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_HEADER_LIST_SIZE; -import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_HEADER_TABLE_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_CONCURRENT_STREAMS; -import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_INITIAL_WINDOW_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_HEADER_LIST_SIZE; +import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_HEADER_TABLE_SIZE; +import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_INITIAL_WINDOW_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.NUM_STANDARD_SETTINGS; import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_ENABLE_PUSH; import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_HEADER_TABLE_SIZE; @@ -32,14 +32,15 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_MAX_FRAME_SIZ import static io.netty.handler.codec.http2.Http2CodecUtil.SETTINGS_MAX_HEADER_LIST_SIZE; import static io.netty.handler.codec.http2.Http2CodecUtil.isMaxFrameSizeValid; import static io.netty.util.internal.ObjectUtil.checkNotNull; -import io.netty.util.collection.IntObjectHashMap; + +import io.netty.util.collection.CharObjectHashMap; /** * Settings for one endpoint in an HTTP/2 connection. Each of the values are optional as defined in * the spec for the SETTINGS frame. Permits storage of arbitrary key/value pairs but provides helper * methods for standard settings. */ -public final class Http2Settings extends IntObjectHashMap { +public final class Http2Settings extends CharObjectHashMap { /** * Default capacity based on the number of standard settings from the HTTP/2 spec, adjusted so that adding all of * the standard settings will not cause the map capacity to change. @@ -65,7 +66,7 @@ public final class Http2Settings extends IntObjectHashMap { * @throws IllegalArgumentException if verification for a standard HTTP/2 setting fails. */ @Override - public Long put(int key, Long value) { + public Long put(char key, Long value) { verifyStandardSetting(key, value); return super.put(key, value); } @@ -184,11 +185,11 @@ public final class Http2Settings extends IntObjectHashMap { } /** - * A helper method that returns {@link Long#intValue()} on the return of {@link #get(int)}, if present. Note that - * if the range of the value exceeds {@link Integer#MAX_VALUE}, the {@link #get(int)} method should + * A helper method that returns {@link Long#intValue()} on the return of {@link #get(char)}, if present. Note that + * if the range of the value exceeds {@link Integer#MAX_VALUE}, the {@link #get(char)} method should * be used instead to avoid truncation of the value. */ - public Integer getIntValue(int key) { + public Integer getIntValue(char key) { Long value = get(key); if (value == null) { return null; @@ -238,7 +239,7 @@ public final class Http2Settings extends IntObjectHashMap { } @Override - protected String keyToString(int key) { + protected String keyToString(char key) { switch (key) { case SETTINGS_HEADER_TABLE_SIZE: return "HEADER_TABLE_SIZE"; diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2SettingsTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2SettingsTest.java index 06c25d7fa8..8166f5f8c2 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2SettingsTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2SettingsTest.java @@ -64,7 +64,15 @@ public class Http2SettingsTest { @Test public void nonStandardSettingsShouldBeSet() { - settings.put(0, 123L); - assertEquals(123L, (long) settings.get(0)); + char key = 0; + settings.put(key, 123L); + assertEquals(123L, (long) settings.get(key)); + } + + @Test + public void settingsShouldSupportUnsignedShort() { + char key = (char) (Short.MAX_VALUE + 1); + settings.put(key, 123L); + assertEquals(123L, (long) settings.get(key)); } }