simplify + add extra test

Signed-off-by: Ludovic Orban <[email protected]>

Author: lorban

jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/HttpOutput.java

@@ -802,17 +802,6 @@ private void checkWritable() throws EofException
 
     @Override
     public void write(byte[] b, int off, int len) throws IOException
-    {
-        write(b, off, len, null);
-    }
-
-    @FunctionalInterface
-    private interface IORunnable
-    {
-        void run() throws IOException;
-    }
-
-    private void write(byte[] b, int off, int len, IORunnable onComplete) throws IOException
     {
         if (LOG.isDebugEnabled())
             LOG.debug("write(array {})", BufferUtil.toDetailString(ByteBuffer.wrap(b, off, len)));
@@ -878,8 +867,6 @@ private void write(byte[] b, int off, int len, IORunnable onComplete) throws IOE
                     if (LOG.isDebugEnabled())
                         LOG.debug("write(array) {} aggregated !flush {}",
                             lockedStateString(), _aggregate);
-                    if (onComplete != null)
-                        onComplete.run();
                     return;
                 }
 
@@ -895,27 +882,8 @@ private void write(byte[] b, int off, int len, IORunnable onComplete) throws IOE
 
         if (async)
         {
-            IORunnable finalOnComplete = onComplete;
             // Do the asynchronous writing from the callback
-            new AsyncWrite(b, off, len, last)
-            {
-                @Override
-                protected void onCompleted(Throwable causeOrNull)
-                {
-                    if (finalOnComplete != null)
-                    {
-                        try
-                        {
-                            finalOnComplete.run();
-                        }
-                        catch (Throwable x)
-                        {
-                            causeOrNull = ExceptionUtil.combine(causeOrNull, x);
-                        }
-                    }
-                    super.onCompleted(causeOrNull);
-                }
-            }.iterate();
+            new AsyncWrite(b, off, len, last).iterate();
             return;
         }
 
@@ -936,12 +904,6 @@ protected void onCompleted(Throwable causeOrNull)
                 {
                     BufferUtil.append(byteBuffer, b, off, len);
                     onWriteComplete(false, null);
-                    if (onComplete != null)
-                    {
-                        IORunnable runOnComplete = onComplete;
-                        onComplete = null;
-                        runOnComplete.run();
-                    }
                     return;
                 }
             }
@@ -971,18 +933,10 @@ else if (last && !complete)
             }
 
             onWriteComplete(last, null);
-            if (onComplete != null)
-            {
-                IORunnable runOnComplete = onComplete;
-                onComplete = null;
-                runOnComplete.run();
-            }
         }
         catch (Throwable t)
         {
             onWriteComplete(last, t);
-            if (onComplete != null)
-                onComplete.run();
             throw t;
         }
     }
@@ -1159,12 +1113,46 @@ private void print(String s, boolean eoln) throws IOException
             throw new IOException("Closed");
 
         s = String.valueOf(s);
-
         String charset = _servletChannel.getServletContextResponse().getCharacterEncoding(false);
-
+        // String.getBytes() is much faster than a CharsetEncoder.encode() loop that would also need to estimate the
+        // destination buffer size, so this compensates for the rare extra copy that needs to be done when the aggregation
+        // buffer is full.
         byte[] bytes = s.getBytes(charset);
-        IORunnable onComplete = eoln ? () -> write(IO.CRLF_BYTES) : null;
-        write(bytes, 0, bytes.length, onComplete);
+        if (!eoln)
+        {
+            write(bytes);
+        }
+        else
+        {
+            int len = bytes.length + IO.CRLF_BYTES.length;
+
+            // If there is enough room left in the aggregation buffer, just fill it otherwise copy into a bigger byte array.
+            boolean canAggregate = false;
+            try (AutoLock ignored = _channelState.lock())
+            {
+                if (len <= _commitSize)
+                {
+                    lockedAcquireBuffer();
+                    if (len <= maximizeAggregateSpace())
+                    {
+                        BufferUtil.fill(_aggregate.getByteBuffer(), bytes, 0, bytes.length);
+                        canAggregate = true;
+                    }
+                }
+            }
+
+            if (canAggregate)
+            {
+                write(IO.CRLF_BYTES);
+            }
+            else
+            {
+                byte[] bytesWithCrLf = new byte[len];
+                System.arraycopy(bytes, 0, bytesWithCrLf, 0, bytes.length);
+                System.arraycopy(IO.CRLF_BYTES, 0, bytesWithCrLf, bytes.length, IO.CRLF_BYTES.length);
+                write(bytesWithCrLf);
+            }
+        }
     }
 
     /**

jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/HttpOutputTest.java

@@ -1058,6 +1058,41 @@ protected void service(HttpServletRequest request, HttpServletResponse response)
         assertThat(response.replace("\r\n", System.lineSeparator()), containsString(expected));
     }
 
+    @Test
+    public void testPrintlnSmallBuffer() throws Exception
+    {
+        ByteArrayOutputStream bout = new ByteArrayOutputStream();
+        PrintWriter exp = new PrintWriter(bout, true, StandardCharsets.UTF_8);
+        HttpServlet servlet = new HttpServlet()
+        {
+            @Override
+            protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException
+            {
+                response.setCharacterEncoding("UTF-8");
+                HttpOutput out = (HttpOutput)response.getOutputStream();
+
+                exp.println("a".repeat(8));
+                out.println("a".repeat(8));
+                exp.println("a".repeat(9));
+                out.println("a".repeat(9));
+                exp.println("a".repeat(10));
+                out.println("a".repeat(10));
+                exp.println("a".repeat(11));
+                out.println("a".repeat(11));
+            }
+        };
+        _servletContextHandler.addServlet(servlet, "/test");
+        ((HttpConnectionFactory)_connector.getDefaultConnectionFactory()).getHttpConfiguration().setOutputBufferSize(10);
+        ((HttpConnectionFactory)_connector.getDefaultConnectionFactory()).getHttpConfiguration().setOutputAggregationSize(10);
+        _server.start();
+        ByteBuffer responseBuf = _connector.getResponse(ByteBuffer.wrap("GET /test HTTP/1.0\nHost: localhost:80\n\n".getBytes(StandardCharsets.UTF_8)));
+        String response = BufferUtil.toString(responseBuf, StandardCharsets.UTF_8);
+        assertThat(response, containsString("HTTP/1.1 200 OK"));
+        String expected = bout.toString(StandardCharsets.UTF_8);
+        assertThat(expected, not(emptyString()));
+        assertThat(response.replace("\r\n", System.lineSeparator()), containsString(expected));
+    }
+
     @Test
     public void testReset() throws Exception
     {