You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by vi...@apache.org on 2016/08/31 10:49:10 UTC
svn commit: r1758580 - in /tomcat/trunk/java/org/apache/tomcat/util/net:
AprEndpoint.java SocketWrapperBase.java
Author: violetagg
Date: Wed Aug 31 10:49:10 2016
New Revision: 1758580
URL: http://svn.apache.org/viewvc?rev=1758580&view=rev
Log:
When AprEndpoint.write(boolean, ByteBuffer) is invoked with a non direct ByteBuffer then copy that ByteBuffer to the socket write buffer before transferring the data to the socket.
Modified:
tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1758580&r1=1758579&r2=1758580&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Wed Aug 31 10:49:10 2016
@@ -2479,6 +2479,59 @@ public class AprEndpoint extends Abstrac
@Override
+ protected void writeByteBufferBlocking(ByteBuffer from) throws IOException {
+ if (from.isDirect()) {
+ super.writeByteBufferBlocking(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 boolean writeByteBufferNonBlocking(ByteBuffer from) throws IOException {
+ if (from.isDirect()) {
+ return super.writeByteBufferNonBlocking(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 true;
+ }
+ }
+
+ if (from.remaining() > 0) {
+ socketBufferHandler.configureWriteBufferForWrite();
+ transfer(from, writeBuffer);
+ }
+
+ return false;
+ }
+ }
+
+
+ @Override
protected void doWrite(boolean block, ByteBuffer from) throws IOException {
if (closed) {
throw new IOException(sm.getString("socket.apr.closed", getSocket()));
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java?rev=1758580&r1=1758579&r2=1758580&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java Wed Aug 31 10:49:10 2016
@@ -453,7 +453,7 @@ public abstract class SocketWrapperBase<
}
- private void writeByteBufferBlocking(ByteBuffer from) throws IOException {
+ protected void writeByteBufferBlocking(ByteBuffer from) throws IOException {
// The socket write buffer capacity is socket.appWriteBufSize
int limit = socketBufferHandler.getWriteBuffer().capacity();
int fromLimit = from.limit();
@@ -551,7 +551,7 @@ public abstract class SocketWrapperBase<
}
- private boolean writeByteBufferNonBlocking(ByteBuffer from) throws IOException {
+ protected boolean writeByteBufferNonBlocking(ByteBuffer from) throws IOException {
// The socket write buffer capacity is socket.appWriteBufSize
int limit = socketBufferHandler.getWriteBuffer().capacity();
int fromLimit = from.limit();
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1758580 - in /tomcat/trunk/java/org/apache/tomcat/util/net:
AprEndpoint.java SocketWrapperBase.java
Posted by Violeta Georgieva <mi...@gmail.com>.
2016-09-01 9:16 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>
> 2016-08-31 19:55 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
>
> > Do you mean the APR case or also NIO and NIO2? With the new API
> > CoyoteOutputStream.write(ByteBuffer) the application layer may provide
> > direct ByteBuffers so why should we copy them to the socket write buffer
> > and not use them directly?
> >
> > I'm talking about NIO+OpenSSL where it's done in our code, for example,
> but internally the JDK does it too.
> Even if the application can suddenly use direct buffers, that's not a good
> idea since GC isn't working and it's more costly (see ByteBufferUtils.
> cleanDirectBuffer). IMO the best model remains using a (direct) buffer
> where all writes go through in that case. I think it is disappointing too
> since it takes away a lot of my improvement ideas (NIO2 scatter/gather,
for
> starters).
>
> So I think your API change is probably good, but at the end it's going to
> be better to write to / read from the socket buffer in many cases rather
> than directly on the socket. It could be a good idea to always do it for
> consistency.
>
> More experimentation is needed.
Yes sure. I'll continue with the experiments. Any other feedback for the
changes that I introduced (working directly with ByteBuffers etc.) is most
welcome.
Thanks a lot,
Violeta
>
>
> Rémy
Re: svn commit: r1758580 - in /tomcat/trunk/java/org/apache/tomcat/util/net:
AprEndpoint.java SocketWrapperBase.java
Posted by Rémy Maucherat <re...@apache.org>.
2016-08-31 19:55 GMT+02:00 Violeta Georgieva <mi...@gmail.com>:
> Do you mean the APR case or also NIO and NIO2? With the new API
> CoyoteOutputStream.write(ByteBuffer) the application layer may provide
> direct ByteBuffers so why should we copy them to the socket write buffer
> and not use them directly?
>
> I'm talking about NIO+OpenSSL where it's done in our code, for example,
but internally the JDK does it too.
Even if the application can suddenly use direct buffers, that's not a good
idea since GC isn't working and it's more costly (see ByteBufferUtils.
cleanDirectBuffer). IMO the best model remains using a (direct) buffer
where all writes go through in that case. I think it is disappointing too
since it takes away a lot of my improvement ideas (NIO2 scatter/gather, for
starters).
So I think your API change is probably good, but at the end it's going to
be better to write to / read from the socket buffer in many cases rather
than directly on the socket. It could be a good idea to always do it for
consistency.
More experimentation is needed.
Rémy
Re: svn commit: r1758580 - in /tomcat/trunk/java/org/apache/tomcat/util/net:
AprEndpoint.java SocketWrapperBase.java
Posted by Violeta Georgieva <mi...@gmail.com>.
2016-08-31 14:03 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>
> 2016-08-31 12:49 GMT+02:00 <vi...@apache.org>:
>
> > Author: violetagg
> > Date: Wed Aug 31 10:49:10 2016
> > New Revision: 1758580
> >
> > URL: http://svn.apache.org/viewvc?rev=1758580&view=rev
> > Log:
> > When AprEndpoint.write(boolean, ByteBuffer) is invoked with a non direct
> > ByteBuffer then copy that ByteBuffer to the socket write buffer before
> > transferring the data to the socket.
> >
> > Ah ok, I forgot that. Well, ultimately, it's the same when using OpenSSL
> as well (the engine will do the wrapping for you). IMO it's unrealistic to
> write directly application layer byte buffers in all cases. If the socket
> buffer is direct, it should be copied there first IMO. The direct flag is
> probably the best hint of the right decision.
Do you mean the APR case or also NIO and NIO2? With the new API
CoyoteOutputStream.write(ByteBuffer) the application layer may provide
direct ByteBuffers so why should we copy them to the socket write buffer
and not use them directly?
>
> Rémy
Re: svn commit: r1758580 - in /tomcat/trunk/java/org/apache/tomcat/util/net:
AprEndpoint.java SocketWrapperBase.java
Posted by Rémy Maucherat <re...@apache.org>.
2016-08-31 12:49 GMT+02:00 <vi...@apache.org>:
> Author: violetagg
> Date: Wed Aug 31 10:49:10 2016
> New Revision: 1758580
>
> URL: http://svn.apache.org/viewvc?rev=1758580&view=rev
> Log:
> When AprEndpoint.write(boolean, ByteBuffer) is invoked with a non direct
> ByteBuffer then copy that ByteBuffer to the socket write buffer before
> transferring the data to the socket.
>
> Ah ok, I forgot that. Well, ultimately, it's the same when using OpenSSL
as well (the engine will do the wrapping for you). IMO it's unrealistic to
write directly application layer byte buffers in all cases. If the socket
buffer is direct, it should be copied there first IMO. The direct flag is
probably the best hint of the right decision.
Rémy
Re: svn commit: r1758580 - in /tomcat/trunk/java/org/apache/tomcat/util/net:
AprEndpoint.java SocketWrapperBase.java
Posted by Violeta Georgieva <mi...@gmail.com>.
2016-08-31 14:08 GMT+03:00 Rémy Maucherat <re...@apache.org>:
>
> 2016-08-31 12:49 GMT+02:00 <vi...@apache.org>:
>
> > Author: violetagg
> > Date: Wed Aug 31 10:49:10 2016
> > New Revision: 1758580
> >
> > URL: http://svn.apache.org/viewvc?rev=1758580&view=rev
> > Log:
> > When AprEndpoint.write(boolean, ByteBuffer) is invoked with a non direct
> > ByteBuffer then copy that ByteBuffer to the socket write buffer before
> > transferring the data to the socket.
> >
> > Hum, and is it the same thing on read as well ?
>
The new read methods with ByteBuffer are not used at the moment.
But yes it is the same.
> Rémy
Re: svn commit: r1758580 - in /tomcat/trunk/java/org/apache/tomcat/util/net:
AprEndpoint.java SocketWrapperBase.java
Posted by Rémy Maucherat <re...@apache.org>.
2016-08-31 12:49 GMT+02:00 <vi...@apache.org>:
> Author: violetagg
> Date: Wed Aug 31 10:49:10 2016
> New Revision: 1758580
>
> URL: http://svn.apache.org/viewvc?rev=1758580&view=rev
> Log:
> When AprEndpoint.write(boolean, ByteBuffer) is invoked with a non direct
> ByteBuffer then copy that ByteBuffer to the socket write buffer before
> transferring the data to the socket.
>
> Hum, and is it the same thing on read as well ?
Rémy