Skip to content

Commit 5c1544e

Browse files
Fred1155olivergillespie
authored andcommitted
Remove Netty UnpooledByteBufAllocator workaround (#6968)
If using the JDK SSL provider with Netty, we currently switch away from Netty's default ByteBuffer allocator to the unpooled allocator, to work around a regression in Netty 4.1.43 - netty/netty#9768. This has been fixed in later versions, so we should revert back to using the default. This is essentially a revert of the relevant parts of a72f494. Reduces allocation rate on NettyHttpClientH1Benchmark by around 45% when using the JDK SSL provider. Co-authored-by: Oli Gillespie <ogillesp@amazon.com>
1 parent 8e8b673 commit 5c1544e

4 files changed

Lines changed: 6 additions & 27 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "Netty NIO HTTP Client",
4+
"contributor": "olivergillespie",
5+
"description": "Don't force Netty's unpooled ByteBuffer allocator when using the JDK SSL provider. The underlying Netty issue (netty/netty#9768) has been fixed in later versions."
6+
}

http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/AwaitCloseChannelPoolMap.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@ protected SimpleChannelPoolAwareChannelPool newPool(URI key) {
131131
ChannelPipelineInitializer pipelineInitializer = new ChannelPipelineInitializer(protocol,
132132
protocolNegotiation,
133133
sslContext,
134-
sslProvider,
135134
maxStreams,
136135
initialWindowSize,
137136
healthCheckPingPeriod,

http-clients/netty-nio-client/src/main/java/software/amazon/awssdk/http/nio/netty/internal/ChannelPipelineInitializer.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,9 @@
2424
import static software.amazon.awssdk.utils.NumericUtils.saturatedCast;
2525
import static software.amazon.awssdk.utils.StringUtils.lowerCase;
2626

27-
import io.netty.buffer.UnpooledByteBufAllocator;
2827
import io.netty.channel.Channel;
2928
import io.netty.channel.ChannelHandlerContext;
3029
import io.netty.channel.ChannelInitializer;
31-
import io.netty.channel.ChannelOption;
3230
import io.netty.channel.ChannelPipeline;
3331
import io.netty.channel.pool.AbstractChannelPoolHandler;
3432
import io.netty.channel.pool.ChannelPool;
@@ -44,7 +42,6 @@
4442
import io.netty.handler.ssl.ApplicationProtocolNegotiationHandler;
4543
import io.netty.handler.ssl.SslContext;
4644
import io.netty.handler.ssl.SslHandler;
47-
import io.netty.handler.ssl.SslProvider;
4845
import java.net.URI;
4946
import java.time.Duration;
5047
import java.util.concurrent.CompletableFuture;
@@ -64,7 +61,6 @@ public final class ChannelPipelineInitializer extends AbstractChannelPoolHandler
6461
private final Protocol protocol;
6562
private final ProtocolNegotiation protocolNegotiation;
6663
private final SslContext sslCtx;
67-
private final SslProvider sslProvider;
6864
private final long clientMaxStreams;
6965
private final int clientInitialWindowSize;
7066
private final Duration healthCheckPingPeriod;
@@ -75,7 +71,6 @@ public final class ChannelPipelineInitializer extends AbstractChannelPoolHandler
7571
public ChannelPipelineInitializer(Protocol protocol,
7672
ProtocolNegotiation protocolNegotiation,
7773
SslContext sslCtx,
78-
SslProvider sslProvider,
7974
long clientMaxStreams,
8075
int clientInitialWindowSize,
8176
Duration healthCheckPingPeriod,
@@ -85,7 +80,6 @@ public ChannelPipelineInitializer(Protocol protocol,
8580
this.protocol = protocol;
8681
this.protocolNegotiation = protocolNegotiation;
8782
this.sslCtx = sslCtx;
88-
this.sslProvider = sslProvider;
8983
this.clientMaxStreams = clientMaxStreams;
9084
this.clientInitialWindowSize = clientInitialWindowSize;
9185
this.healthCheckPingPeriod = healthCheckPingPeriod;
@@ -108,12 +102,6 @@ public void channelCreated(Channel ch) {
108102

109103
pipeline.addLast(sslHandler);
110104
pipeline.addLast(SslCloseCompletionEventHandler.getInstance());
111-
112-
// Use unpooled allocator to avoid increased heap memory usage from Netty 4.1.43.
113-
// See https://github.com/netty/netty/issues/9768
114-
if (sslProvider == SslProvider.JDK) {
115-
ch.config().setOption(ChannelOption.ALLOCATOR, UnpooledByteBufAllocator.DEFAULT);
116-
}
117105
}
118106

119107
configureProtocolHandlers(ch, pipeline, protocol, sslCtxPresent);

http-clients/netty-nio-client/src/test/java/software/amazon/awssdk/http/nio/netty/internal/ChannelPipelineInitializerTest.java

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,14 @@
1515

1616
package software.amazon.awssdk.http.nio.netty.internal;
1717

18-
import static org.hamcrest.CoreMatchers.is;
19-
import static org.hamcrest.MatcherAssert.assertThat;
2018
import static org.junit.jupiter.api.Assertions.assertNotNull;
2119
import static org.junit.jupiter.api.Assertions.assertNull;
2220
import static org.mockito.Mockito.mock;
2321
import static org.mockito.Mockito.when;
2422
import static software.amazon.awssdk.http.SdkHttpConfigurationOption.GLOBAL_HTTP_DEFAULTS;
2523

26-
import io.netty.buffer.UnpooledByteBufAllocator;
2724
import io.netty.channel.Channel;
2825
import io.netty.channel.ChannelHandlerContext;
29-
import io.netty.channel.ChannelOption;
3026
import io.netty.channel.embedded.EmbeddedChannel;
3127
import io.netty.channel.pool.ChannelPool;
3228
import io.netty.handler.codec.http2.Http2MultiplexHandler;
@@ -50,15 +46,6 @@ public class ChannelPipelineInitializerTest {
5046
private final URI TARGET_URI = URI.create("https://some-awesome-service-1234.amazonaws.com:8080");
5147
private static final SslProvider SSL_PROVIDER = SslProvider.JDK;
5248

53-
@Test
54-
public void channelConfigOptionCheck() {
55-
ChannelPipelineInitializer pipelineInitializer = createChannelPipelineInitializer(Protocol.HTTP1_1, ProtocolNegotiation.ASSUME_PROTOCOL);
56-
Channel channel = new EmbeddedChannel();
57-
pipelineInitializer.channelCreated(channel);
58-
59-
assertThat(channel.config().getOption(ChannelOption.ALLOCATOR), is(UnpooledByteBufAllocator.DEFAULT));
60-
}
61-
6249
@Test
6350
@EnabledIf("alpnSupported")
6451
public void h2AlpnEnabled_shouldUseAlpn() {
@@ -126,7 +113,6 @@ private ChannelPipelineInitializer createChannelPipelineInitializer(Protocol pro
126113
return new ChannelPipelineInitializer(protocol,
127114
protocolNegotiation,
128115
sslContextProvider.sslContext(),
129-
SSL_PROVIDER,
130116
100,
131117
1024,
132118
Duration.ZERO,

0 commit comments

Comments
 (0)