Skip to content

Commit

Permalink
Netty: revert performance improvement around creation of netty contex…
Browse files Browse the repository at this point in the history
…t instance

-  it doesn't work on linux
  • Loading branch information
jknack committed Jul 2, 2023
1 parent 3458a09 commit 651b140
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 193 deletions.
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

# Usage:
# - docker build -t jooby .
# - docker run -it jooby -v "$HOME/.m2":/root/.m2
# - /build # mvn clean package
# - docker run -v "$HOME/.m2":/root/.m2 -it jooby
# - /build # mvn clean package -P '!git-hooks'

FROM maven:3-eclipse-temurin-17 as build

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public class NettyContext implements DefaultContext, ChannelFutureListener {
private String scheme;
private int port;

public void init(
public NettyContext(
ChannelHandlerContext ctx,
HttpRequest req,
Router router,
Expand All @@ -157,32 +157,9 @@ public void init(
this.router = router;
this.bufferSize = bufferSize;
this.method = req.method().name().toUpperCase();
/** Clear everything else. */
this.attributes.clear();
this.setHeaders.clear();
this.route = null;
this.decoder = null;
this.webSocket = null;
this.listeners = null;
this.cookies = null;
this.responseCookies = null;
this.needsFlush = false;
this.host = null;
this.port = 0;
this.contentLength = -1;
this.scheme = null;
this.responseType = null;
this.responseStarted = false;
this.headers = null;
this.status = HttpResponseStatus.OK;
this.query = null;
this.formdata = null;
this.files = null;
this.pathMap = Collections.EMPTY_MAP;
this.resetHeadersOnError = null;
if (http2) {
// Save streamId for HTTP/2
this.streamId = req.headers().get(STREAM_ID);
this.streamId = header(STREAM_ID).valueOrNull();
ifStreamId(this.streamId);
} else {
this.streamId = null;
Expand Down Expand Up @@ -352,7 +329,7 @@ public List<Certificate> getClientCertificates() {
throw SneakyThrows.propagate(x);
}
}
return new ArrayList<Certificate>();
return Collections.emptyList();
}

@NonNull @Override
Expand Down Expand Up @@ -451,7 +428,7 @@ public Context upgrade(WebSocket.Initializer handler) {
handler.init(Context.readOnly(this), webSocket);
FullHttpRequest webSocketRequest =
new DefaultFullHttpRequest(
req.protocolVersion(),
HTTP_1_1,
req.method(),
req.uri(),
Unpooled.EMPTY_BUFFER,
Expand Down Expand Up @@ -596,7 +573,7 @@ public PrintWriter responseWriter(MediaType type, Charset charset) {
@NonNull @Override
public Sender responseSender() {
prepareChunked();
ctx.write(new DefaultHttpResponse(req.protocolVersion(), status, setHeaders));
ctx.write(new DefaultHttpResponse(HTTP_1_1, status, setHeaders));
return new NettySender(this, ctx);
}

Expand Down Expand Up @@ -638,7 +615,7 @@ public final Context send(ByteBuffer data) {
private Context send(@NonNull ByteBuf data) {
try {
responseStarted = true;
setHeaders.set(CONTENT_LENGTH, Long.toString(data.readableBytes()));
setHeaders.set(CONTENT_LENGTH, Integer.toString(data.readableBytes()));
DefaultFullHttpResponse response =
new DefaultFullHttpResponse(HTTP_1_1, status, data, setHeaders, NO_TRAILING);
if (ctx.channel().eventLoop().inEventLoop()) {
Expand Down Expand Up @@ -903,7 +880,7 @@ void destroy(Throwable cause) {
private NettyOutputStream newOutputStream() {
prepareChunked();
return new NettyOutputStream(
this, ctx, bufferSize, new DefaultHttpResponse(req.protocolVersion(), status, setHeaders));
this, ctx, bufferSize, new DefaultHttpResponse(HTTP_1_1, status, setHeaders));
}

private FileUpload register(FileUpload upload) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import io.jooby.SneakyThrows;
import io.jooby.StatusCode;
import io.jooby.WebSocketCloseStatus;
import io.netty.channel.ChannelHandler;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.handler.codec.http.HttpContent;
Expand All @@ -43,10 +42,8 @@
import io.netty.handler.codec.http.websocketx.WebSocketFrame;
import io.netty.handler.timeout.IdleStateEvent;
import io.netty.util.AsciiString;
import io.netty.util.Attribute;
import io.netty.util.AttributeKey;

@ChannelHandler.Sharable
public class NettyHandler extends ChannelInboundHandlerAdapter {
private static final AtomicReference<String> cachedDateString = new AtomicReference<>();

Expand All @@ -65,6 +62,8 @@ public class NettyHandler extends ChannelInboundHandlerAdapter {
private long chunkSize;
private boolean http2;

private NettyContext context;

public NettyHandler(
ScheduledExecutorService scheduler,
Router router,
Expand All @@ -82,16 +81,12 @@ public NettyHandler(
this.http2 = http2;
}

public boolean isHttp2() {
return http2;
}

@Override
public void channelRead(ChannelHandlerContext ctx, Object msg) {
if (isHttpRequest(msg)) {
var req = (HttpRequest) msg;
var context = getOrCreateContext(ctx);
context.init(ctx, req, router, pathOnly(req.uri()), bufferSize, http2);

context = new NettyContext(ctx, req, router, pathOnly(req.uri()), bufferSize, http2);

if (defaultHeaders) {
context.setHeaders.set(HttpHeaderNames.DATE, date(router.getLog(), scheduler));
Expand All @@ -114,7 +109,6 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
} else if (isHttpContent(msg)) {
var chunk = (HttpContent) msg;
try {
var context = getContextOrNull(ctx);
// when decoder == null, chunk is always a LastHttpContent.EMPTY, ignore it
if (context.decoder != null) {
chunkSize += chunk.content().readableBytes();
Expand All @@ -136,7 +130,6 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
release(chunk);
}
} else if (msg instanceof WebSocketFrame) {
var context = getContextOrNull(ctx);
if (context.webSocket != null) {
context.webSocket.handleFrame((WebSocketFrame) msg);
}
Expand All @@ -149,23 +142,8 @@ private void release(HttpContent ref) {
}
}

private NettyContext getOrCreateContext(ChannelHandlerContext ctx) {
Attribute<NettyContext> attr = ctx.channel().attr(CONTEXT);
var context = attr.get();
if (context == null) {
context = new NettyContext();
attr.set(context);
}
return context;
}

private NettyContext getContextOrNull(ChannelHandlerContext ctx) {
return ctx.channel().attr(CONTEXT).get();
}

@Override
public void channelReadComplete(ChannelHandlerContext ctx) {
var context = getContextOrNull(ctx);
if (context != null) {
context.flush();
}
Expand All @@ -184,7 +162,6 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) {
@Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
try {
var context = getContextOrNull(ctx);
Logger log = router.getLog();
if (Server.connectionLost(cause)) {
if (log.isDebugEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,29 @@

public class NettyPipeline extends ChannelInitializer<SocketChannel> {
private static final String H2_HANDSHAKE = "h2-handshake";

private Integer compressionLevel;
private int bufferSize;
private long maxRequestSize;
private SslContext sslContext;
private boolean is100ContinueExpected;
private NettyHandler handler;
private boolean http2;
private Supplier<NettyHandler> handlerFactory;

public NettyPipeline(
NettyHandler handler,
Supplier<NettyHandler> handlerFactory,
SslContext sslContext,
Integer compressionLevel,
int bufferSize,
long maxRequestSize,
boolean http2,
boolean is100ContinueExpected) {
this.sslContext = sslContext;
this.compressionLevel = compressionLevel;
this.bufferSize = bufferSize;
this.maxRequestSize = maxRequestSize;
this.is100ContinueExpected = is100ContinueExpected;
this.handler = handler;
this.http2 = http2;
this.handlerFactory = handlerFactory;
}

@Override
Expand All @@ -53,7 +55,7 @@ public void initChannel(SocketChannel ch) {
if (sslContext != null) {
p.addLast("ssl", sslContext.newHandler(ch.alloc()));
}
if (handler.isHttp2()) {
if (http2) {
Http2Settings settings = new Http2Settings(maxRequestSize, sslContext != null);
Http2Extension extension =
new Http2Extension(
Expand All @@ -67,7 +69,7 @@ public void initChannel(SocketChannel ch) {

setupCompression(p);

p.addLast("handler", handler);
p.addLast("handler", handlerFactory.get());
} else {
http11(p);
}
Expand Down Expand Up @@ -113,7 +115,7 @@ private void http11(ChannelPipeline p) {
p.addLast("codec", codec);
setupExpectContinue(p);
setupCompression(p);
p.addLast("handler", handler);
p.addLast("handler", handlerFactory.get());
}

HttpServerCodec createServerCodec() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,13 @@ private ClientAuth toClientAuth(SslOptions.ClientAuth clientAuth) {
private NettyPipeline newPipeline(HttpDataFactory factory, SslContext sslContext, boolean http2) {
var executor = acceptorloop.next();
var router = applications.get(0);
var handler = createHandler(executor, router, options, factory, http2);
return new NettyPipeline(
handler,
() -> createHandler(executor, router, options, factory, http2),
sslContext,
options.getCompressionLevel(),
options.getBufferSize(),
options.getMaxRequestSize(),
http2,
options.isExpectContinue() == Boolean.TRUE);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.nio.channels.ReadableByteChannel;
import java.nio.charset.Charset;
import java.security.cert.Certificate;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -242,7 +241,7 @@ public List<Certificate> getClientCertificates() {
throw SneakyThrows.propagate(x);
}
}
return new ArrayList<Certificate>();
return Collections.emptyList();
}

@NonNull @Override
Expand Down
Loading

0 comments on commit 651b140

Please sign in to comment.