Apply some micro-optimization discovered by JProfiler #775

Author: jknack

coverage-report/pom.xml

@@ -5,7 +5,7 @@
   <parent>
     <groupId>org.jooby</groupId>
     <artifactId>jooby-project</artifactId>
-    <version>1.1.1</version>
+    <version>1.1.2-SNAPSHOT</version>
   </parent>
 
   <modelVersion>4.0.0</modelVersion>
@@ -229,6 +229,7 @@
             <groupId>org.apache.maven.plugins</groupId>
             <artifactId>maven-surefire-plugin</artifactId>
             <configuration>
+              <skip>false</skip>
               <includes>
                 <include>**/*Test.java</include>
                 <include>**/*Feature.java</include>
@@ -288,11 +289,7 @@
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-surefire-plugin</artifactId>
         <configuration>
-          <includes>
-            <include>**/*Test.java</include>
-            <include>**/*Feature.java</include>
-            <include>**/*Issue*.java</include>
-          </includes>
+          <skip>true</skip>
         </configuration>
       </plugin>
 

coverage-report/src/test/java/org/jooby/ContentNegotiationFeature.java

@@ -42,19 +42,18 @@ public String json() {
 
     renderer(BodyConverters.toJson);
 
-    get("/any", req ->
-        Results
-            .when("text/html", () -> Results.html("test").put("this", "body"))
-            .when("*/*", () -> "body"));
+    get("/any", req -> Results
+        .when("text/html", () -> Results.html("test").put("this", "body"))
+        .when("*/*", () -> "body"));
 
-    get("/status", req ->
-        Results
-            .when("*", () -> Status.NOT_ACCEPTABLE));
+    get("/status", req -> Results
+        .when("*", () -> {
+          throw new Err(Status.NOT_ACCEPTABLE);
+        }));
 
-    get("/like", req ->
-        Results
-            .when("text/html", () -> Results.html("test").put("this", "body"))
-            .when("application/json", () -> "body"));
+    get("/like", req -> Results
+        .when("text/html", () -> Results.html("test").put("this", "body"))
+        .when("application/json", () -> "body"));
 
     get("/html", (req, resp) -> resp.send(Results.html("test").put("this", "body")))
         .produces(MediaType.html);
@@ -205,10 +204,10 @@ public void fallback() throws Exception {
 
   @Test
   public void like() throws Exception {
-//    request()
-//        .get("/like")
-//        .header("Accept", "application/*+json")
-//        .expect("{\"body\": \"body\"}");
+    // request()
+    // .get("/like")
+    // .header("Accept", "application/*+json")
+    // .expect("{\"body\": \"body\"}");
 
     request()
         .get("/like")

coverage-report/src/test/java/org/jooby/RouteReferenceFeature.java

@@ -17,14 +17,18 @@ public class RouteReferenceFeature extends ServerFeature {
       assertEquals("GET", route.method());
       assertEquals("/", route.path());
       assertEquals("/", route.pattern());
+      assertEquals("bar", route.attr("foo"));
       assertEquals(
-          "| Method | Path | Source                             | Name       | Pattern | Consumes | Produces |\n" +
-          "|--------|------|------------------------------------|------------|---------|----------|----------|\n" +
-          "| GET    | /    | org.jooby.RouteReferenceFeature:13 | /anonymous | /       | [*/*]    | [*/*]    |" +
-          "",
+          "| Method | Path | Source                             | Name       | Pattern | Consumes | Produces |\n"
+              +
+              "|--------|------|------------------------------------|------------|---------|----------|----------|\n"
+              +
+              "| GET    | /    | org.jooby.RouteReferenceFeature:13 | /anonymous | /       | [*/*]    | [*/*]    |"
+              +
+              "",
           req.toString());
       rsp.send("done");
-    });
+    }).attr("foo", "bar");
 
     get("/:var", (req, rsp) -> {
       Route route = req.route();

coverage.sh

@@ -1,5 +1,4 @@
 #!/bin/bash
 ALPN_VERSION="`groovy -e 'String v = (System.getProperty("java.vendor").contains("IBM") ? {def m = (["java", "-version"].execute().err.text =~ /(?s)java version "([0-9]*.[0-9]*.[0-9]*)".*Oracle jdk[0-9]u([0-9]*)-.*/); "${m[0][1]}_${m[0][2]}"}.call() : System.getProperty("java.version")); ((new URL("http://www.eclipse.org/jetty/documentation/9.4.x/alpn-chapter.html")).text =~ /ALPN vs. OpenJDK versions(.*)<\/table>/).each { m, s -> ( s =~ /([0-9].[0-9].[0-9]*u[0-9]*)[^0-9]*([0-9].[0-9]*.[0-9]*.v2[0-9]*)/).each { _, jdkv, alpnv -> if (v.equals(jdkv.replace("u","_"))){ println alpnv }}}'`"
 echo "ALPN $ALPN_VERSION"
-cd coverage-report
 mvn -Dlogback.configurationFile=logback-travis.xml -Dalpn-boot-version=$ALPN_VERSION -DdryRun=true clean package coveralls:report -P coverage

jooby-netty/src/main/java/org/jooby/internal/netty/NettyHandler.java

@@ -19,15 +19,12 @@
 package org.jooby.internal.netty;
 
 import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
-import static java.util.Objects.requireNonNull;
 
 import org.jooby.internal.ConnectionResetByPeer;
 import org.jooby.spi.HttpHandler;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.typesafe.config.Config;
-
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.SimpleChannelInboundHandler;
 import io.netty.handler.codec.http.DefaultFullHttpResponse;
@@ -61,14 +58,12 @@ public class NettyHandler extends SimpleChannelInboundHandler<Object> {
 
   private int bufferSize;
 
-  public NettyHandler(final HttpHandler handler, final Config config) {
-    this.handler = requireNonNull(handler, "Application handler is required.");
-    this.tmpdir = config.getString("application.tmpdir");
-    this.bufferSize = config.getBytes("server.http.ResponseBufferSize").intValue();
-    this.wsMaxMessageSize = Math
-        .max(
-            config.getBytes("server.ws.MaxTextMessageSize").intValue(),
-            config.getBytes("server.ws.MaxBinaryMessageSize").intValue());
+  public NettyHandler(final HttpHandler handler, final String tmpdir, final int bufferSize,
+      final int wsBufferSize) {
+    this.handler = handler;
+    this.tmpdir = tmpdir;
+    this.bufferSize = bufferSize;
+    this.wsMaxMessageSize = wsBufferSize;
   }
 
   @Override

jooby-netty/src/main/java/org/jooby/internal/netty/NettyPipeline.java

@@ -74,6 +74,12 @@ public class NettyPipeline extends ChannelInitializer<SocketChannel> {
 
   private boolean supportH2;
 
+  private String tmpdir;
+
+  private int bufferSize;
+
+  private int wsMaxMessageSize;
+
   public NettyPipeline(final EventExecutorGroup executor, final HttpHandler handler,
       final Config conf, final SslContext sslCtx) {
     this.executor = executor;
@@ -86,6 +92,12 @@ public NettyPipeline(final EventExecutorGroup executor, final HttpHandler handle
     maxContentLength = conf.getBytes("netty.http.MaxContentLength").intValue();
     idleTimeOut = conf.getDuration("netty.http.IdleTimeout", TimeUnit.MILLISECONDS);
     supportH2 = conf.getBoolean("server.http2.enabled");
+    this.tmpdir = config.getString("application.tmpdir");
+    this.bufferSize = config.getBytes("server.http.ResponseBufferSize").intValue();
+    this.wsMaxMessageSize = Math
+        .max(
+            config.getBytes("server.ws.MaxTextMessageSize").intValue(),
+            config.getBytes("server.ws.MaxBinaryMessageSize").intValue());
     this.sslCtx = sslCtx;
   }
 
@@ -143,7 +155,7 @@ private void aggregator(final ChannelPipeline p) {
   }
 
   private void jooby(final ChannelPipeline p) {
-    p.addLast(executor, "jooby", new NettyHandler(handler, config));
+    p.addLast(executor, "jooby", new NettyHandler(handler, tmpdir, bufferSize, wsMaxMessageSize));
   }
 
   private Http2ConnectionHandler newHttp2ConnectionHandler(final ChannelPipeline p) {

jooby-netty/src/main/java/org/jooby/internal/netty/NettyResponse.java

@@ -37,6 +37,7 @@
 import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelPipeline;
+import io.netty.channel.ChannelPromise;
 import io.netty.channel.DefaultFileRegion;
 import io.netty.handler.codec.http.DefaultFullHttpResponse;
 import io.netty.handler.codec.http.DefaultHttpHeaders;
@@ -131,12 +132,20 @@ public void send(final InputStream stream) throws Exception {
     } else {
       DefaultHttpResponse rsp = new DefaultHttpResponse(HttpVersion.HTTP_1_1, status);
 
-      if (!headers.contains(HttpHeaderNames.CONTENT_LENGTH)) {
+      boolean lenSet = headers.contains(HttpHeaderNames.CONTENT_LENGTH);
+      final boolean keepAlive;
+      final ChannelPromise promise;
+      if (!lenSet) {
         headers.set(HttpHeaderNames.TRANSFER_ENCODING, HttpHeaderValues.CHUNKED);
+        keepAlive = false;
+        promise = ctx.newPromise();
+      } else if (this.keepAlive) {
+        headers.set(HttpHeaderNames.CONNECTION, HttpHeaderValues.KEEP_ALIVE);
+        keepAlive = this.keepAlive;
+        promise = ctx.voidPromise();
       } else {
-        if (keepAlive) {
-          headers.set(HttpHeaderNames.CONNECTION, HttpHeaderValues.KEEP_ALIVE);
-        }
+        keepAlive = false;
+        promise = ctx.newPromise();
       }
 
       // dump headers
@@ -155,7 +164,10 @@ public void send(final InputStream stream) throws Exception {
         ctx.write(buffer);
         // send tail
         ctx.write(new ChunkedStream(stream, bufferSize));
-        keepAlive(ctx.writeAndFlush(LastHttpContent.EMPTY_LAST_CONTENT));
+        ChannelFuture future = ctx.writeAndFlush(LastHttpContent.EMPTY_LAST_CONTENT, promise);
+        if (!keepAlive) {
+          future.addListener(CLOSE);
+        }
       });
     }
 
@@ -171,10 +183,8 @@ public void send(final FileChannel channel) throws Exception {
   public void send(final FileChannel channel, final long offset, final long count)
       throws Exception {
     DefaultHttpResponse rsp = new DefaultHttpResponse(HttpVersion.HTTP_1_1, status);
-    if (!headers.contains(HttpHeaderNames.CONTENT_LENGTH)) {
-      headers.remove(HttpHeaderNames.TRANSFER_ENCODING);
-      headers.set(HttpHeaderNames.CONTENT_LENGTH, count);
-    }
+    headers.remove(HttpHeaderNames.TRANSFER_ENCODING);
+    headers.set(HttpHeaderNames.CONTENT_LENGTH, count);
 
     if (keepAlive) {
       headers.set(HttpHeaderNames.CONNECTION, HttpHeaderValues.KEEP_ALIVE);
@@ -198,17 +208,25 @@ public void send(final FileChannel channel, final long offset, final long count)
 
       ctx.channel().eventLoop().execute(() -> {
         // send headers
-        ctx.write(rsp);
+        ctx.write(rsp, ctx.voidPromise());
         // chunked file
-        keepAlive(ctx.writeAndFlush(chunkedInput));
+        if (keepAlive) {
+          ctx.writeAndFlush(chunkedInput, ctx.voidPromise());
+        } else {
+          ctx.writeAndFlush(chunkedInput).addListener(CLOSE);
+        }
       });
     } else {
       ctx.channel().eventLoop().execute(() -> {
         // send headers
-        ctx.write(rsp);
+        ctx.write(rsp, ctx.voidPromise());
         // file region
-        ctx.write(new DefaultFileRegion(channel, offset, count));
-        keepAlive(ctx.writeAndFlush(LastHttpContent.EMPTY_LAST_CONTENT));
+        ctx.write(new DefaultFileRegion(channel, offset, count), ctx.voidPromise());
+        if (keepAlive) {
+          ctx.writeAndFlush(LastHttpContent.EMPTY_LAST_CONTENT, ctx.voidPromise());
+        } else {
+          ctx.writeAndFlush(LastHttpContent.EMPTY_LAST_CONTENT).addListener(CLOSE);
+        }
       });
     }
 
@@ -219,39 +237,33 @@ public void send(final FileChannel channel, final long offset, final long count)
   private void send(final ByteBuf buffer) throws Exception {
     DefaultFullHttpResponse rsp = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, status, buffer);
 
-    if (!headers.contains(HttpHeaderNames.CONTENT_LENGTH)) {
-      headers.remove(HttpHeaderNames.TRANSFER_ENCODING)
-          .set(HttpHeaderNames.CONTENT_LENGTH, buffer.readableBytes());
-    }
+    headers.remove(HttpHeaderNames.TRANSFER_ENCODING)
+        .set(HttpHeaderNames.CONTENT_LENGTH, buffer.readableBytes());
 
+    ChannelPromise promise;
     if (keepAlive) {
+      promise = ctx.voidPromise();
       headers.set(HttpHeaderNames.CONNECTION, HttpHeaderValues.KEEP_ALIVE);
+    } else {
+      promise = ctx.newPromise();
     }
 
     // dump headers
     rsp.headers().set(headers);
 
     Attribute<Boolean> async = ctx.channel().attr(NettyRequest.ASYNC);
-    boolean isAsync = async != null && async.get() == Boolean.TRUE;
-    if (isAsync) {
-      // we need flush, from async
-      keepAlive(ctx.writeAndFlush(rsp));
+    boolean flush = async != null && async.get() == Boolean.TRUE;
+    final ChannelFuture future;
+    if (flush) {
+      future = ctx.writeAndFlush(rsp, promise);
     } else {
-      keepAlive(ctx.write(rsp));
+      future = ctx.write(rsp, promise);
     }
-
-    committed = true;
-  }
-
-  private void keepAlive(final ChannelFuture future) {
-    if (headers.contains(HttpHeaderNames.CONTENT_LENGTH)) {
-      if (!keepAlive) {
-        future.addListener(CLOSE);
-      }
-    } else {
-      // content len is not set, just close the connection regardless keep alive or not.
+    if (!keepAlive) {
       future.addListener(CLOSE);
     }
+
+    committed = true;
   }
 
   @Override
@@ -282,11 +294,16 @@ public void end() {
       }
       if (!committed) {
         DefaultHttpResponse rsp = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, status);
+        headers.set(HttpHeaderNames.CONTENT_LENGTH, 0);
         // dump headers
         rsp.headers().set(headers);
-        keepAlive(ctx.write(rsp));
+        if (keepAlive) {
+          ctx.write(rsp, ctx.voidPromise());
+        } else {
+          ctx.write(rsp).addListener(CLOSE);
+        }
+        committed = true;
       }
-      committed = true;
       ctx = null;
     }
   }

jooby-netty/src/main/java/org/jooby/internal/netty/NettyServer.java

@@ -56,6 +56,8 @@
 import io.netty.handler.logging.LogLevel;
 import io.netty.handler.logging.LoggingHandler;
 import io.netty.handler.ssl.SslContext;
+import io.netty.util.ResourceLeakDetector;
+import io.netty.util.ResourceLeakDetector.Level;
 import io.netty.util.concurrent.DefaultEventExecutorGroup;
 import io.netty.util.concurrent.DefaultThreadFactory;
 import io.netty.util.concurrent.EventExecutorGroup;
@@ -81,6 +83,7 @@ public class NettyServer implements Server {
   public NettyServer(final HttpHandler dispatcher, final Config config) {
     this.dispatcher = dispatcher;
     this.conf = config;
+    ResourceLeakDetector.setLevel(Level.DISABLED);
   }
 
   @Override
@@ -143,8 +146,7 @@ public void join() throws InterruptedException {
   }
 
   @Override
-  public Optional<Executor> executor()
-  {
+  public Optional<Executor> executor() {
     return Optional.ofNullable(executor);
   }
 

jooby-netty/src/test/java/org/jooby/internal/netty/Issue581.java

@@ -59,7 +59,11 @@ public class Issue581 {
       .withValue("netty.http.IdleTimeout", ConfigValueFactory.fromAnyRef("30s"))
       .withValue("netty.options.CONNECT_TIMEOUT_MILLIS", ConfigValueFactory.fromAnyRef(1000))
       .withValue("application.port", ConfigValueFactory.fromAnyRef(6789))
-      .withValue("application.host", ConfigValueFactory.fromAnyRef("0.0.0.0"));
+      .withValue("application.host", ConfigValueFactory.fromAnyRef("0.0.0.0"))
+      .withValue("server.http.ResponseBufferSize", ConfigValueFactory.fromAnyRef("16k"))
+      .withValue("server.ws.MaxTextMessageSize", ConfigValueFactory.fromAnyRef("16k"))
+      .withValue("server.ws.MaxBinaryMessageSize", ConfigValueFactory.fromAnyRef("16k"))
+      .withValue("application.tmpdir", ConfigValueFactory.fromAnyRef("target"));
 
   @SuppressWarnings({"rawtypes", "unchecked" })
   private MockUnit.Block parentEventLoop = unit -> {