This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 7d2365d Simplify regular endpoint writes by removing
write(Non)BlockingDirect
7d2365d is described below
commit 7d2365d4e302717140946e019b63f05598b4a080
Author: remm <r...@apache.org>
AuthorDate: Wed Nov 27 10:30:15 2019 +0100
Simplify regular endpoint writes by removing write(Non)BlockingDirect
Port patch from trunk/9.0.
---
java/org/apache/tomcat/util/net/AprEndpoint.java | 51 -----------
.../apache/tomcat/util/net/SocketWrapperBase.java | 100 ++++-----------------
webapps/docs/changelog.xml | 5 ++
3 files changed, 22 insertions(+), 134 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java
b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 927dd01..64f0257 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -2330,57 +2330,6 @@ public class AprEndpoint extends AbstractEndpoint<Long>
implements SNICallBack {
@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 {
if (closed) {
throw new IOException(sm.getString("socket.apr.closed",
getSocket()));
diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
index dd5e741..545d333 100644
--- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java
+++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java
@@ -403,8 +403,6 @@ public abstract class SocketWrapperBase<E> {
* - To enable a marginally more efficient implemented for blocking
* writes which do not require the additional checks related to the
* use of the non-blocking write buffer
- * TODO: Explore re-factoring options to remove the split into
- * separate methods
*/
if (block) {
writeBlocking(buf, off, len);
@@ -452,8 +450,6 @@ public abstract class SocketWrapperBase<E> {
* - To enable a marginally more efficient implemented for blocking
* writes which do not require the additional checks related to the
* use of the non-blocking write buffer
- * TODO: Explore re-factoring options to remove the split into
- * separate methods
*/
if (block) {
writeBlocking(from);
@@ -481,7 +477,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);
@@ -505,46 +501,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());
}
@@ -614,11 +574,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);
}
@@ -634,44 +595,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 8452d2e..efc034a 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -72,6 +72,11 @@
<add>
Add async API to the NIO and APR connector. (remm)
</add>
+ <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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
