From 3b65506c13f18535e2fb055fd790bf45efb13de5 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Fri, 22 Nov 2024 15:08:02 +0100 Subject: [PATCH] Use ByteBuffer support in ServletHttpHandlerAdapter As of Servlet 6.1, the `ServletInputStream` and `ServletOutputStream` offer read and write variants based on `ByteBuffer` instead of byte arrays. This can improve performance and avoid memory copy for I/O calls. This was already partially supported for some servers like Tomcat through specific adapters. This commit moves this support to the standard `ServletHttpHandlerAdapter` and makes it available for all Servlet 6.1+ containers. Closes gh-33748 --- .../reactive/JettyHttpHandlerAdapter.java | 83 ------------------- .../reactive/ServletServerHttpRequest.java | 42 ++++++---- .../reactive/ServletServerHttpResponse.java | 19 +++-- .../reactive/TomcatHttpHandlerAdapter.java | 64 +------------- .../reactive/bootstrap/JettyHttpServer.java | 7 +- 5 files changed, 40 insertions(+), 175 deletions(-) delete mode 100644 spring-web/src/main/java/org/springframework/http/server/reactive/JettyHttpHandlerAdapter.java diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/JettyHttpHandlerAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/JettyHttpHandlerAdapter.java deleted file mode 100644 index d45c72b2666..00000000000 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/JettyHttpHandlerAdapter.java +++ /dev/null @@ -1,83 +0,0 @@ -/* - * Copyright 2002-2024 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.http.server.reactive; - -import java.io.IOException; -import java.io.OutputStream; -import java.nio.ByteBuffer; - -import jakarta.servlet.AsyncContext; -import jakarta.servlet.http.HttpServletResponse; -import org.eclipse.jetty.ee10.servlet.HttpOutput; - -import org.springframework.core.io.buffer.DataBuffer; -import org.springframework.core.io.buffer.DataBufferFactory; - -/** - * {@link ServletHttpHandlerAdapter} extension that uses Jetty APIs for writing - * to the response with {@link ByteBuffer}. - * - * @author Violeta Georgieva - * @author Brian Clozel - * @author Juergen Hoeller - * @since 5.0 - * @see org.springframework.web.server.adapter.AbstractReactiveWebInitializer - */ -public class JettyHttpHandlerAdapter extends ServletHttpHandlerAdapter { - - public JettyHttpHandlerAdapter(HttpHandler httpHandler) { - super(httpHandler); - } - - - @Override - protected ServletServerHttpResponse createResponse(HttpServletResponse response, - AsyncContext context, ServletServerHttpRequest request) throws IOException { - - return new Jetty12ServerHttpResponse( - response, context, getDataBufferFactory(), getBufferSize(), request); - } - - - private static final class Jetty12ServerHttpResponse extends ServletServerHttpResponse { - - Jetty12ServerHttpResponse(HttpServletResponse response, AsyncContext asyncContext, - DataBufferFactory bufferFactory, int bufferSize, ServletServerHttpRequest request) - throws IOException { - - super(response, asyncContext, bufferFactory, bufferSize, request); - } - - @Override - protected int writeToOutputStream(DataBuffer dataBuffer) throws IOException { - OutputStream output = getOutputStream(); - if (output instanceof HttpOutput httpOutput) { - int len = 0; - try (DataBuffer.ByteBufferIterator iterator = dataBuffer.readableByteBuffers()) { - while (iterator.hasNext() && httpOutput.isReady()) { - ByteBuffer byteBuffer = iterator.next(); - len += byteBuffer.remaining(); - httpOutput.write(byteBuffer); - } - } - return len; - } - return super.writeToOutputStream(dataBuffer); - } - } - -} diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java index ed5c52b112e..ee2b9a5014b 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpRequest.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.net.URI; import java.net.URISyntaxException; +import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.security.cert.X509Certificate; import java.util.Enumeration; @@ -38,6 +39,7 @@ import reactor.core.publisher.Flux; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; +import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.http.HttpCookie; import org.springframework.http.HttpHeaders; @@ -58,6 +60,7 @@ import org.springframework.web.util.UriComponentsBuilder; * * @author Rossen Stoyanchev * @author Juergen Hoeller + * @author Brian Clozel * @since 5.0 */ class ServletServerHttpRequest extends AbstractServerHttpRequest { @@ -75,7 +78,7 @@ class ServletServerHttpRequest extends AbstractServerHttpRequest { private final DataBufferFactory bufferFactory; - private final byte[] buffer; + private final int bufferSize; private final AsyncListener asyncListener; @@ -99,7 +102,7 @@ class ServletServerHttpRequest extends AbstractServerHttpRequest { this.request = request; this.bufferFactory = bufferFactory; - this.buffer = new byte[bufferSize]; + this.bufferSize = bufferSize; this.asyncListener = new RequestAsyncListener(); @@ -275,20 +278,31 @@ class ServletServerHttpRequest extends AbstractServerHttpRequest { * or {@link #EOF_BUFFER} if the input stream returned -1. */ DataBuffer readFromInputStream() throws IOException { - int read = this.inputStream.read(this.buffer); - logBytesRead(read); - - if (read > 0) { - DataBuffer dataBuffer = this.bufferFactory.allocateBuffer(read); - dataBuffer.write(this.buffer, 0, read); - return dataBuffer; + DataBuffer dataBuffer = this.bufferFactory.allocateBuffer(this.bufferSize); + int read = -1; + try { + try (DataBuffer.ByteBufferIterator iterator = dataBuffer.writableByteBuffers()) { + Assert.state(iterator.hasNext(), "No ByteBuffer available"); + ByteBuffer byteBuffer = iterator.next(); + read = this.inputStream.read(byteBuffer); + } + logBytesRead(read); + if (read > 0) { + dataBuffer.writePosition(read); + return dataBuffer; + } + else if (read == -1) { + return EOF_BUFFER; + } + else { + return AbstractListenerReadPublisher.EMPTY_BUFFER; + } } - - if (read == -1) { - return EOF_BUFFER; + finally { + if (read <= 0) { + DataBufferUtils.release(dataBuffer); + } } - - return AbstractListenerReadPublisher.EMPTY_BUFFER; } protected final void logBytesRead(int read) { diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java index 4d8e96b27e0..ac145a04d45 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java @@ -17,7 +17,7 @@ package org.springframework.http.server.reactive; import java.io.IOException; -import java.io.InputStream; +import java.nio.ByteBuffer; import java.nio.charset.Charset; import jakarta.servlet.AsyncContext; @@ -45,6 +45,7 @@ import org.springframework.util.Assert; * * @author Rossen Stoyanchev * @author Juergen Hoeller + * @author Brian Clozel * @since 5.0 */ class ServletServerHttpResponse extends AbstractListenerServerHttpResponse { @@ -222,15 +223,15 @@ class ServletServerHttpResponse extends AbstractListenerServerHttpResponse { */ protected int writeToOutputStream(DataBuffer dataBuffer) throws IOException { ServletOutputStream outputStream = this.outputStream; - InputStream input = dataBuffer.asInputStream(); - int bytesWritten = 0; - byte[] buffer = new byte[this.bufferSize]; - int bytesRead; - while (outputStream.isReady() && (bytesRead = input.read(buffer)) != -1) { - outputStream.write(buffer, 0, bytesRead); - bytesWritten += bytesRead; + int len = 0; + try (DataBuffer.ByteBufferIterator iterator = dataBuffer.readableByteBuffers()) { + while (iterator.hasNext() && outputStream.isReady()) { + ByteBuffer byteBuffer = iterator.next(); + len += byteBuffer.remaining(); + outputStream.write(byteBuffer); + } } - return bytesWritten; + return len; } private void flush() throws IOException { diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java index d93b4b89002..20064837c71 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/TomcatHttpHandlerAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,16 +26,12 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletRequestWrapper; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpServletResponseWrapper; -import org.apache.catalina.connector.CoyoteInputStream; -import org.apache.catalina.connector.CoyoteOutputStream; import org.apache.catalina.connector.RequestFacade; import org.apache.catalina.connector.ResponseFacade; import org.apache.coyote.Request; import org.apache.coyote.Response; -import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; -import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.http.HttpHeaders; import org.springframework.util.Assert; import org.springframework.util.MultiValueMap; @@ -81,10 +77,6 @@ public class TomcatHttpHandlerAdapter extends ServletHttpHandlerAdapter { private static final Field COYOTE_REQUEST_FIELD; - private final int bufferSize; - - private final DataBufferFactory factory; - static { Field field = ReflectionUtils.findField(RequestFacade.class, "request"); Assert.state(field != null, "Incompatible Tomcat implementation"); @@ -97,8 +89,6 @@ public class TomcatHttpHandlerAdapter extends ServletHttpHandlerAdapter { throws IOException, URISyntaxException { super(createTomcatHttpHeaders(request), request, context, servletPath, factory, bufferSize); - this.factory = factory; - this.bufferSize = bufferSize; } private static MultiValueMap createTomcatHttpHeaders(HttpServletRequest request) { @@ -124,41 +114,6 @@ public class TomcatHttpHandlerAdapter extends ServletHttpHandlerAdapter { } } - @Override - protected DataBuffer readFromInputStream() throws IOException { - if (getInputStream() instanceof CoyoteInputStream coyoteInputStream) { - DataBuffer dataBuffer = this.factory.allocateBuffer(this.bufferSize); - int read = -1; - try { - try (DataBuffer.ByteBufferIterator iterator = dataBuffer.writableByteBuffers()) { - Assert.state(iterator.hasNext(), "No ByteBuffer available"); - ByteBuffer byteBuffer = iterator.next(); - read = coyoteInputStream.read(byteBuffer); - } - logBytesRead(read); - if (read > 0) { - dataBuffer.writePosition(read); - return dataBuffer; - } - else if (read == -1) { - return EOF_BUFFER; - } - else { - return AbstractListenerReadPublisher.EMPTY_BUFFER; - } - } - finally { - if (read <= 0) { - DataBufferUtils.release(dataBuffer); - } - } - } - else { - // It's possible InputStream can be wrapped, preventing use of CoyoteInputStream - return super.readFromInputStream(); - } - - } } @@ -208,23 +163,6 @@ public class TomcatHttpHandlerAdapter extends ServletHttpHandlerAdapter { adaptHeaders(true); } - @Override - protected int writeToOutputStream(DataBuffer dataBuffer) throws IOException { - if (getOutputStream() instanceof CoyoteOutputStream coyoteOutputStream) { - int len = 0; - try (DataBuffer.ByteBufferIterator iterator = dataBuffer.readableByteBuffers()) { - while (iterator.hasNext() && coyoteOutputStream.isReady()) { - ByteBuffer byteBuffer = iterator.next(); - len += byteBuffer.remaining(); - coyoteOutputStream.write(byteBuffer); - } - } - return len; - } - else { - return super.writeToOutputStream(dataBuffer); - } - } } } diff --git a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/http/server/reactive/bootstrap/JettyHttpServer.java b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/http/server/reactive/bootstrap/JettyHttpServer.java index 12878528ff1..eb1cc085991 100644 --- a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/http/server/reactive/bootstrap/JettyHttpServer.java +++ b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/http/server/reactive/bootstrap/JettyHttpServer.java @@ -22,7 +22,6 @@ import org.eclipse.jetty.ee10.websocket.server.config.JettyWebSocketServletConta import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; -import org.springframework.http.server.reactive.JettyHttpHandlerAdapter; import org.springframework.http.server.reactive.ServletHttpHandlerAdapter; /** @@ -41,7 +40,7 @@ public class JettyHttpServer extends AbstractHttpServer { this.jettyServer = new Server(); - ServletHttpHandlerAdapter servlet = createServletAdapter(); + ServletHttpHandlerAdapter servlet = new ServletHttpHandlerAdapter(resolveHttpHandler()); ServletHolder servletHolder = new ServletHolder(servlet); servletHolder.setAsyncSupported(true); @@ -56,10 +55,6 @@ public class JettyHttpServer extends AbstractHttpServer { this.jettyServer.setHandler(this.contextHandler); } - private ServletHttpHandlerAdapter createServletAdapter() { - return new JettyHttpHandlerAdapter(resolveHttpHandler()); - } - @Override protected void startInternal() throws Exception { this.jettyServer.start();