[tomcat] branch 9.0.x updated: Refactor to reduce pinning in HTTP/2 code when using virtual threads
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new dc8f0d1b3c Refactor to reduce pinning in HTTP/2 code when using
virtual threads
dc8f0d1b3c is described below
commit dc8f0d1b3c3d50caf84ee60b3a487928216ea432
Author: Mark Thomas
AuthorDate: Thu Jul 27 15:37:59 2023 +0100
Refactor to reduce pinning in HTTP/2 code when using virtual threads
---
java/org/apache/coyote/http2/AbstractStream.java | 84 ---
.../apache/coyote/http2/Http2UpgradeHandler.java | 241 +++--
java/org/apache/coyote/http2/RecycledStream.java | 1 -
java/org/apache/coyote/http2/Stream.java | 84 +++
.../coyote/http2/WindowAllocationManager.java | 27 ++-
5 files changed, 250 insertions(+), 187 deletions(-)
diff --git a/java/org/apache/coyote/http2/AbstractStream.java
b/java/org/apache/coyote/http2/AbstractStream.java
index f332b8c593..d6fb8d8280 100644
--- a/java/org/apache/coyote/http2/AbstractStream.java
+++ b/java/org/apache/coyote/http2/AbstractStream.java
@@ -16,6 +16,10 @@
*/
package org.apache.coyote.http2;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.res.StringManager;
@@ -33,6 +37,8 @@ abstract class AbstractStream {
private final String idAsString;
private long windowSize =
ConnectionSettingsBase.DEFAULT_INITIAL_WINDOW_SIZE;
+protected final Lock windowAllocationLock = new ReentrantLock();
+protected final Condition windowAllocationAvailable =
windowAllocationLock.newCondition();
private volatile int connectionAllocationRequested = 0;
private volatile int connectionAllocationMade = 0;
@@ -59,13 +65,23 @@ abstract class AbstractStream {
}
-final synchronized void setWindowSize(long windowSize) {
-this.windowSize = windowSize;
+final void setWindowSize(long windowSize) {
+windowAllocationLock.lock();
+try {
+this.windowSize = windowSize;
+} finally {
+windowAllocationLock.unlock();
+}
}
-final synchronized long getWindowSize() {
-return windowSize;
+final long getWindowSize() {
+windowAllocationLock.lock();
+try {
+return windowSize;
+} finally {
+windowAllocationLock.unlock();
+}
}
@@ -76,37 +92,47 @@ abstract class AbstractStream {
*
* @throws Http2Exception If the window size is now higher than the
maximum allowed
*/
-synchronized void incrementWindowSize(int increment) throws Http2Exception
{
-// No need for overflow protection here.
-// Increment can't be more than Integer.MAX_VALUE and once windowSize
-// goes beyond 2^31-1 an error is triggered.
-windowSize += increment;
-
-if (log.isDebugEnabled()) {
-log.debug(sm.getString("abstractStream.windowSizeInc",
getConnectionId(), getIdAsString(),
-Integer.toString(increment), Long.toString(windowSize)));
-}
+void incrementWindowSize(int increment) throws Http2Exception {
+windowAllocationLock.lock();
+try {
+// No need for overflow protection here.
+// Increment can't be more than Integer.MAX_VALUE and once
windowSize
+// goes beyond 2^31-1 an error is triggered.
+windowSize += increment;
+
+if (log.isDebugEnabled()) {
+log.debug(sm.getString("abstractStream.windowSizeInc",
getConnectionId(), getIdAsString(),
+Integer.toString(increment),
Long.toString(windowSize)));
+}
-if (windowSize > ConnectionSettingsBase.MAX_WINDOW_SIZE) {
-String msg = sm.getString("abstractStream.windowSizeTooBig",
getConnectionId(), identifier,
-Integer.toString(increment), Long.toString(windowSize));
-if (identifier.intValue() == 0) {
-throw new ConnectionException(msg,
Http2Error.FLOW_CONTROL_ERROR);
-} else {
-throw new StreamException(msg, Http2Error.FLOW_CONTROL_ERROR,
identifier.intValue());
+if (windowSize > ConnectionSettingsBase.MAX_WINDOW_SIZE) {
+String msg = sm.getString("abstractStream.windowSizeTooBig",
getConnectionId(), identifier,
+Integer.toString(increment),
Long.toString(windowSize));
+if (identifier.intValue() == 0) {
+throw new ConnectionException(msg,
Http2Error.FLOW_CONTROL_ERROR);
+} else {
+throw new StreamException(msg,
Http2Error.FLOW_CONTROL_ERROR, identi
[tomcat] branch 9.0.x updated: Refactor to reduce pinning in HTTP/2 code when using virtual threads
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new e74bda808a Refactor to reduce pinning in HTTP/2 code when using
virtual threads
e74bda808a is described below
commit e74bda808aa043b09790135f53d7de692cc3e1c3
Author: Mark Thomas
AuthorDate: Wed Jul 26 14:38:37 2023 +0100
Refactor to reduce pinning in HTTP/2 code when using virtual threads
---
java/org/apache/coyote/http2/Stream.java | 90 +---
1 file changed, 60 insertions(+), 30 deletions(-)
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index bce6c33559..915fda02ac 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -1153,6 +1153,7 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
class StandardStreamInputBuffer extends StreamInputBuffer {
+private final Lock readStateLock = new ReentrantLock();
/*
* Two buffers are required to avoid various multi-threading issues.
These issues arise from the fact that the
* Stream (or the Request/Response) used by the application is
processed in one thread but the connection is
@@ -1263,7 +1264,8 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
final boolean isReadyForRead() {
ensureBuffersExist();
-synchronized (this) {
+readStateLock.lock();
+try {
if (available() > 0) {
return true;
}
@@ -1273,21 +1275,33 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
}
return false;
+} finally {
+readStateLock.unlock();
}
}
@Override
-final synchronized boolean isRequestBodyFullyRead() {
-return (inBuffer == null || inBuffer.position() == 0) &&
isInputFinished();
+final boolean isRequestBodyFullyRead() {
+readStateLock.lock();
+try {
+return (inBuffer == null || inBuffer.position() == 0) &&
isInputFinished();
+} finally {
+readStateLock.unlock();
+}
}
@Override
-public final synchronized int available() {
-if (inBuffer == null) {
-return 0;
+public final int available() {
+readStateLock.lock();
+try {
+if (inBuffer == null) {
+return 0;
+}
+return inBuffer.position();
+} finally {
+readStateLock.unlock();
}
-return inBuffer.position();
}
@@ -1295,26 +1309,31 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
* Called after placing some data in the inBuffer.
*/
@Override
-final synchronized void onDataAvailable() throws IOException {
-if (closed) {
-swallowUnread();
-} else if (readInterest) {
-if (log.isDebugEnabled()) {
-log.debug(sm.getString("stream.inputBuffer.dispatch"));
-}
-readInterest = false;
-coyoteRequest.action(ActionCode.DISPATCH_READ, null);
-// Always need to dispatch since this thread is processing
-// the incoming connection and streams are processed on their
-// own.
-coyoteRequest.action(ActionCode.DISPATCH_EXECUTE, null);
-} else {
-if (log.isDebugEnabled()) {
-log.debug(sm.getString("stream.inputBuffer.signal"));
-}
-synchronized (inBuffer) {
-inBuffer.notifyAll();
+final void onDataAvailable() throws IOException {
+readStateLock.lock();
+try {
+if (closed) {
+swallowUnread();
+} else if (readInterest) {
+if (log.isDebugEnabled()) {
+log.debug(sm.getString("stream.inputBuffer.dispatch"));
+}
+readInterest = false;
+coyoteRequest.action(ActionCode.DISPATCH_READ, null);
+// Always need to dispatch since this thread is processing
+// the incoming connection and streams are processed on
their
+// own.
+coyoteRequest.action(ActionCode.DISPATCH_EXECUTE, null);
+} else {
+if (log.isDebugEnabled()) {
+log.debug(sm.getString("stream.inputBuffer.signal"));
+
[tomcat] branch 9.0.x updated: Refactor to reduce pinning in HTTP/2 code when using virtual threads
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 50f627546f Refactor to reduce pinning in HTTP/2 code when using
virtual threads
50f627546f is described below
commit 50f627546f085ae64aff4c3e4465d4cd797a685c
Author: Mark Thomas
AuthorDate: Wed Jul 26 13:23:39 2023 +0100
Refactor to reduce pinning in HTTP/2 code when using virtual threads
---
.../apache/coyote/http2/Http2AsyncUpgradeHandler.java| 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
index 1b86d0da05..d9f69464f6 100644
--- a/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2AsyncUpgradeHandler.java
@@ -26,6 +26,8 @@ import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
import javax.servlet.http.WebConnection;
@@ -44,9 +46,9 @@ public class Http2AsyncUpgradeHandler extends
Http2UpgradeHandler {
// Ensures headers are generated and then written for one thread at a time.
// Because of the compression used, headers need to be written to the
// network in the same order they are generated.
-private final Object headerWriteLock = new Object();
+private final Lock headerWriteLock = new ReentrantLock();
// Ensures thread triggers the stream reset is the first to send a RST
frame
-private final Object sendResetLock = new Object();
+private final Lock sendResetLock = new ReentrantLock();
private final AtomicReference error = new AtomicReference<>();
private final AtomicReference applicationIOE = new
AtomicReference<>();
@@ -148,7 +150,8 @@ public class Http2AsyncUpgradeHandler extends
Http2UpgradeHandler {
// may see out of order RST frames which may hard to follow if
// the client is unaware the RST frames may be received out of
// order.
-synchronized (sendResetLock) {
+sendResetLock.lock();
+try {
if (state != null) {
boolean active = state.isActive();
state.sendReset();
@@ -159,6 +162,8 @@ public class Http2AsyncUpgradeHandler extends
Http2UpgradeHandler {
socketWrapper.write(BlockingMode.SEMI_BLOCK,
protocol.getWriteTimeout(), TimeUnit.MILLISECONDS, null,
SocketWrapperBase.COMPLETE_WRITE, errorCompletion,
ByteBuffer.wrap(rstFrame));
+} finally {
+sendResetLock.unlock();
}
handleAsyncException();
}
@@ -191,7 +196,8 @@ public class Http2AsyncUpgradeHandler extends
Http2UpgradeHandler {
@Override
void writeHeaders(Stream stream, int pushedStreamId, MimeHeaders
mimeHeaders, boolean endOfStream, int payloadSize)
throws IOException {
-synchronized (headerWriteLock) {
+headerWriteLock.lock();
+try {
AsyncHeaderFrameBuffers headerFrameBuffers =
(AsyncHeaderFrameBuffers) doWriteHeaders(stream,
pushedStreamId, mimeHeaders, endOfStream, payloadSize);
if (headerFrameBuffers != null) {
@@ -200,6 +206,8 @@ public class Http2AsyncUpgradeHandler extends
Http2UpgradeHandler {
headerFrameBuffers.bufs.toArray(BYTEBUFFER_ARRAY));
handleAsyncException();
}
+} finally {
+headerWriteLock.unlock();
}
if (endOfStream) {
sentEndOfStream(stream);
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 9.0.x updated: Refactor to reduce pinning in HTTP/2 code when using virtual threads
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new acf7bc0d20 Refactor to reduce pinning in HTTP/2 code when using
virtual threads
acf7bc0d20 is described below
commit acf7bc0d20954f809cf2fb7a13afb6372d82f1f4
Author: Mark Thomas
AuthorDate: Wed Jul 26 12:36:15 2023 +0100
Refactor to reduce pinning in HTTP/2 code when using virtual threads
---
java/org/apache/coyote/http2/Stream.java | 244 +--
1 file changed, 136 insertions(+), 108 deletions(-)
diff --git a/java/org/apache/coyote/http2/Stream.java
b/java/org/apache/coyote/http2/Stream.java
index 4a770b54e4..bce6c33559 100644
--- a/java/org/apache/coyote/http2/Stream.java
+++ b/java/org/apache/coyote/http2/Stream.java
@@ -28,6 +28,8 @@ import java.util.HashSet;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Supplier;
import org.apache.coyote.ActionCode;
@@ -911,6 +913,7 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
class StreamOutputBuffer implements HttpOutputBuffer, WriteBuffer.Sink {
+private final Lock writeLock = new ReentrantLock();
private final ByteBuffer buffer = ByteBuffer.allocate(8 * 1024);
private final WriteBuffer writeBuffer = new WriteBuffer(32 * 1024);
// Flag that indicates that data was left over on a previous
@@ -929,125 +932,145 @@ class Stream extends AbstractNonZeroStream implements
HeaderEmitter {
*/
@Override
-public final synchronized int doWrite(ByteBuffer chunk) throws
IOException {
-if (closed) {
-throw new IOException(sm.getString("stream.closed",
getConnectionId(), getIdAsString()));
-}
-// chunk is always fully written
-int result = chunk.remaining();
-if (writeBuffer.isEmpty()) {
-int chunkLimit = chunk.limit();
-while (chunk.remaining() > 0) {
-int thisTime = Math.min(buffer.remaining(),
chunk.remaining());
-chunk.limit(chunk.position() + thisTime);
-buffer.put(chunk);
-chunk.limit(chunkLimit);
-if (chunk.remaining() > 0 && !buffer.hasRemaining()) {
-// Only flush if we have more data to write and the
buffer
-// is full
-if (flush(true, coyoteResponse.getWriteListener() ==
null)) {
-writeBuffer.add(chunk);
-dataLeft = true;
-break;
+public final int doWrite(ByteBuffer chunk) throws IOException {
+writeLock.lock();
+try {
+if (closed) {
+throw new IOException(sm.getString("stream.closed",
getConnectionId(), getIdAsString()));
+}
+// chunk is always fully written
+int result = chunk.remaining();
+if (writeBuffer.isEmpty()) {
+int chunkLimit = chunk.limit();
+while (chunk.remaining() > 0) {
+int thisTime = Math.min(buffer.remaining(),
chunk.remaining());
+chunk.limit(chunk.position() + thisTime);
+buffer.put(chunk);
+chunk.limit(chunkLimit);
+if (chunk.remaining() > 0 && !buffer.hasRemaining()) {
+// Only flush if we have more data to write and
the buffer
+// is full
+if (flush(true, coyoteResponse.getWriteListener()
== null)) {
+writeBuffer.add(chunk);
+dataLeft = true;
+break;
+}
}
}
+} else {
+writeBuffer.add(chunk);
}
-} else {
-writeBuffer.add(chunk);
+written += result;
+return result;
+} finally {
+writeLock.unlock();
}
-written += result;
-return result;
}
-final synchronized boolean flush(boolean block) throws IOException {
-/*
- * Need to ensure that there is exactly one call to flush even
when there is no data to write. Too few calls
- * (i.e. zero) and the end of stream message is not sent for a
completed asynchronous write. Too many calls
- * and the end of stream message is sent too soon and tra
[tomcat] branch 9.0.x updated: Refactor to reduce pinning in HTTP/2 code when using virtual threads
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 56ba62e2fc Refactor to reduce pinning in HTTP/2 code when using
virtual threads
56ba62e2fc is described below
commit 56ba62e2fc60c550a8b90a54452f78683a682ea0
Author: Mark Thomas
AuthorDate: Wed Jul 26 10:34:38 2023 +0100
Refactor to reduce pinning in HTTP/2 code when using virtual threads
---
java/org/apache/coyote/http2/StreamProcessor.java | 10 --
webapps/docs/changelog.xml| 4
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/http2/StreamProcessor.java
b/java/org/apache/coyote/http2/StreamProcessor.java
index 4b06ff9b71..8d8af36852 100644
--- a/java/org/apache/coyote/http2/StreamProcessor.java
+++ b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -22,6 +22,8 @@ import java.util.Enumeration;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
import javax.servlet.http.HttpServletResponse;
@@ -55,6 +57,7 @@ class StreamProcessor extends AbstractProcessor {
private static final Set H2_PSEUDO_HEADERS_REQUEST = new
HashSet<>();
+private final Lock processLock = new ReentrantLock();
private final Http2UpgradeHandler handler;
private final Stream stream;
private SendfileData sendfileData = null;
@@ -77,8 +80,9 @@ class StreamProcessor extends AbstractProcessor {
final void process(SocketEvent event) {
try {
-// FIXME: the regular processor syncs on socketWrapper, but here
this deadlocks
-synchronized (this) {
+// Note: The regular processor uses the socketWrapper lock, but
using that here triggers a deadlock
+processLock.lock();
+try {
// HTTP/2 equivalent of AbstractConnectionHandler#process()
without the
// socket <-> processor mapping
SocketState state = SocketState.CLOSED;
@@ -134,6 +138,8 @@ class StreamProcessor extends AbstractProcessor {
recycle();
}
}
+} finally {
+processLock.unlock();
}
} finally {
handler.executeQueuedStream();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3d7f7256a8..f8b8f3f95e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -143,6 +143,10 @@
certificateKeystoreFile attribute of an
SSLHostConfigCertificate instance. (markt)
+
+Refactor HTTP/2 implementation to reduce pinning when using virtual
+threads. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
