You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2019/11/25 16:44:20 UTC
[tomcat] branch master updated: Simplify regular endpoint writes by
removing write(Non)BlockingDirect
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 33eccc7 Simplify regular endpoint writes by removing write(Non)BlockingDirect
33eccc7 is described below
commit 33eccc77048bd15131f8f9f5d95720399719952a
Author: remm <re...@apache.org>
AuthorDate: Mon Nov 25 17:44:00 2019 +0100
Simplify regular endpoint writes by removing write(Non)BlockingDirect
The performance difference is null on what I exercised (static files).
However, this removes code and makes buffering behavior consistent,
avoiding a possible issue mentioned in the fixme (small writes causing a
performance degradation).
Following this I don't find any additional interesting cleanup. After
testing to verify things are not broken, I will work on backporting to
8.5 piece by piece.
---
java/org/apache/tomcat/util/net/AprEndpoint.java | 51 ------------
.../apache/tomcat/util/net/SocketWrapperBase.java | 96 ++++------------------
webapps/docs/changelog.xml | 5 ++
3 files changed, 22 insertions(+), 130 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 10322da..ebfd51b 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -2250,57 +2250,6 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB
@Override
- protected void writeBlockingDirect(ByteBuffer from) throws IOException {
- if (from.isDirect()) {
- super.writeBlockingDirect(from);
- } else {
- // The socket write buffer capacity is socket.appWriteBufSize
- ByteBuffer writeBuffer = socketBufferHandler.getWriteBuffer();
- int limit = writeBuffer.capacity();
- while (from.remaining() >= limit) {
- socketBufferHandler.configureWriteBufferForWrite();
- transfer(from, writeBuffer);
- doWrite(true);
- }
-
- if (from.remaining() > 0) {
- socketBufferHandler.configureWriteBufferForWrite();
- transfer(from, writeBuffer);
- }
- }
- }
-
-
- @Override
- protected void writeNonBlockingDirect(ByteBuffer from) throws IOException {
- if (from.isDirect()) {
- super.writeNonBlockingDirect(from);
- } else {
- // The socket write buffer capacity is socket.appWriteBufSize
- ByteBuffer writeBuffer = socketBufferHandler.getWriteBuffer();
- int limit = writeBuffer.capacity();
- while (from.remaining() >= limit) {
- socketBufferHandler.configureWriteBufferForWrite();
- transfer(from, writeBuffer);
- int newPosition = writeBuffer.position() + limit;
- doWrite(false);
- if (writeBuffer.position() != newPosition) {
- // Didn't write the whole amount of data in the last
- // non-blocking write.
- // Exit the loop.
- return;
- }
- }
-
- if (from.remaining() > 0) {
- socketBufferHandler.configureWriteBufferForWrite();
- transfer(from, writeBuffer);
- }
- }
- }
-
-
- @Override
protected void doWrite(boolean block, ByteBuffer from) throws IOException {
Lock readLock = getBlockingStatusReadLock();
WriteLock writeLock = getBlockingStatusWriteLock();
diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
index e81eb26..777276d 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -531,7 +531,7 @@ public abstract class SocketWrapperBase<E> {
protected void writeBlocking(byte[] buf, int off, int len) throws IOException {
socketBufferHandler.configureWriteBufferForWrite();
int thisTime = transfer(buf, off, len, socketBufferHandler.getWriteBuffer());
- while (socketBufferHandler.getWriteBuffer().remaining() == 0) {
+ while (!socketBufferHandler.getWriteBuffer().hasRemaining()) {
len = len - thisTime;
off = off + thisTime;
doWrite(true);
@@ -555,46 +555,10 @@ public abstract class SocketWrapperBase<E> {
* @throws IOException If an IO error occurs during the write
*/
protected void writeBlocking(ByteBuffer from) throws IOException {
- if (socketBufferHandler.isWriteBufferEmpty()) {
- // Socket write buffer is empty. Write the provided buffer directly
- // to the network.
- // TODO Shouldn't smaller writes be buffered anyway?
- writeBlockingDirect(from);
- } else {
- // Socket write buffer has some data.
- socketBufferHandler.configureWriteBufferForWrite();
- // Put as much data as possible into the write buffer
- transfer(from, socketBufferHandler.getWriteBuffer());
- // If the buffer is now full, write it to the network and then write
- // the remaining data directly to the network.
- if (!socketBufferHandler.isWriteBufferWritable()) {
- doWrite(true);
- writeBlockingDirect(from);
- }
- }
- }
-
-
- /**
- * Writes directly to the network, bypassing the socket write buffer.
- *
- * @param from The ByteBuffer containing the data to be written
- *
- * @throws IOException If an IO error occurs during the write
- */
- protected void writeBlockingDirect(ByteBuffer from) throws IOException {
- // The socket write buffer capacity is socket.appWriteBufSize
- // TODO This only matters when using TLS. For non-TLS connections it
- // should be possible to write the ByteBuffer in a single write
- int limit = socketBufferHandler.getWriteBuffer().capacity();
- int fromLimit = from.limit();
- while (from.remaining() >= limit) {
- from.limit(from.position() + limit);
- doWrite(true, from);
- from.limit(fromLimit);
- }
-
- if (from.remaining() > 0) {
+ socketBufferHandler.configureWriteBufferForWrite();
+ transfer(from, socketBufferHandler.getWriteBuffer());
+ while (from.hasRemaining()) {
+ doWrite(true);
socketBufferHandler.configureWriteBufferForWrite();
transfer(from, socketBufferHandler.getWriteBuffer());
}
@@ -664,11 +628,12 @@ public abstract class SocketWrapperBase<E> {
protected void writeNonBlocking(ByteBuffer from)
throws IOException {
- if (nonBlockingWriteBuffer.isEmpty() && socketBufferHandler.isWriteBufferWritable()) {
+ if (from.hasRemaining() && nonBlockingWriteBuffer.isEmpty()
+ && socketBufferHandler.isWriteBufferWritable()) {
writeNonBlockingInternal(from);
}
- if (from.remaining() > 0) {
+ if (from.hasRemaining()) {
// Remaining data must be buffered
nonBlockingWriteBuffer.add(from);
}
@@ -684,44 +649,17 @@ public abstract class SocketWrapperBase<E> {
* @throws IOException If an IO error occurs during the write
*/
protected void writeNonBlockingInternal(ByteBuffer from) throws IOException {
- if (socketBufferHandler.isWriteBufferEmpty()) {
- writeNonBlockingDirect(from);
- } else {
- socketBufferHandler.configureWriteBufferForWrite();
- transfer(from, socketBufferHandler.getWriteBuffer());
- if (!socketBufferHandler.isWriteBufferWritable()) {
- doWrite(false);
- if (socketBufferHandler.isWriteBufferWritable()) {
- writeNonBlockingDirect(from);
- }
- }
- }
- }
-
-
- protected void writeNonBlockingDirect(ByteBuffer from) throws IOException {
- // The socket write buffer capacity is socket.appWriteBufSize
- // TODO This only matters when using TLS. For non-TLS connections it
- // should be possible to write the ByteBuffer in a single write
- int limit = socketBufferHandler.getWriteBuffer().capacity();
- int fromLimit = from.limit();
- while (from.remaining() >= limit) {
- int newLimit = from.position() + limit;
- from.limit(newLimit);
- doWrite(false, from);
- from.limit(fromLimit);
- if (from.position() != newLimit) {
- // Didn't write the whole amount of data in the last
- // non-blocking write.
- // Exit the loop.
- return;
+ socketBufferHandler.configureWriteBufferForWrite();
+ transfer(from, socketBufferHandler.getWriteBuffer());
+ while (from.hasRemaining() && !socketBufferHandler.isWriteBufferWritable()) {
+ doWrite(false);
+ if (socketBufferHandler.isWriteBufferWritable()) {
+ socketBufferHandler.configureWriteBufferForWrite();
+ transfer(from, socketBufferHandler.getWriteBuffer());
+ } else {
+ break;
}
}
-
- if (from.remaining() > 0) {
- socketBufferHandler.configureWriteBufferForWrite();
- transfer(from, socketBufferHandler.getWriteBuffer());
- }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 486fe95..38bdc88 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -59,6 +59,11 @@
<bug>63949</bug>: Fix non blocking write problems with NIO due to the
need for a write loop. (remm)
</fix>
+ <fix>
+ Simplify regular endpoint writes by removing write(Non)BlockingDirect.
+ All regular writes will now be buffered for a more predictable
+ behavior. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org