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 2015/01/15 15:28:14 UTC
svn commit: r1652108 -
/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
Author: remm
Date: Thu Jan 15 14:28:13 2015
New Revision: 1652108
URL: http://svn.apache.org/r1652108
Log:
Fix use of the semaphore (it seems equivalent to not using it). This fixes the ab corruption I was experiencing.
Modified:
tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1652108&r1=1652107&r2=1652108&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Thu Jan 15 14:28:13 2015
@@ -744,11 +744,13 @@ public class Nio2Endpoint extends Abstra
failed(new ClosedChannelException(), attachment);
return;
}
+ readPending.release();
getEndpoint().processSocket(attachment, SocketStatus.OPEN_READ, true);
}
@Override
public void failed(Throwable exc, SocketWrapperBase<Nio2Channel> attachment) {
+ readPending.release();
getEndpoint().processSocket(attachment, SocketStatus.DISCONNECT, true);
}
};
@@ -1310,9 +1312,6 @@ public class Nio2Endpoint extends Abstra
getSocket().getBufHandler().configureReadBufferForWrite();
getSocket().read(getSocket().getBufHandler().getReadBuffer(),
getTimeout(), TimeUnit.MILLISECONDS, this, awaitBytesHandler);
- // TODO Figure out why moving this to the awaitBytesHandler
- // causes test failures.
- readPending.release();
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1652108 - /tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
Posted by Mark Thomas <ma...@apache.org>.
On 15/01/2015 18:24, Rémy Maucherat wrote:
> 2015-01-15 17:03 GMT+01:00 Mark Thomas <ma...@apache.org>:
>
>> On 15/01/2015 14:28, remm@apache.org wrote:
>>> Author: remm
>>> Date: Thu Jan 15 14:28:13 2015
>>> New Revision: 1652108
>>>
>>> URL: http://svn.apache.org/r1652108
>>> Log:
>>> Fix use of the semaphore (it seems equivalent to not using it). This
>> fixes the ab corruption I was experiencing.
>>
>> Thanks. That fixes the performance issue I was seeing (which was caused
>> by an exception during socket close so the lower maxKeepAliveRequests
>> the more obvious the problem became). It certainly drives home the
>> message that you never, ever want exceptions routinely occurring on the
>> critical path.
>>
> For the ab results, I am now at (8 to trunk before latest refactoring to
> trunk):
> NIO1: 50k -> 60k -> 57k
> APR: 67k -> 67k -> 62k
> NIO2: 65k -> 65k -> 60k
>
> So I see a small difference with this latest refactoring.
Looks like I still have some work to do. I'll keep chipping away with
the performance improvements along side my 8.0.x next work.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1652108 - /tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
Posted by Rémy Maucherat <re...@apache.org>.
2015-01-15 17:03 GMT+01:00 Mark Thomas <ma...@apache.org>:
> On 15/01/2015 14:28, remm@apache.org wrote:
> > Author: remm
> > Date: Thu Jan 15 14:28:13 2015
> > New Revision: 1652108
> >
> > URL: http://svn.apache.org/r1652108
> > Log:
> > Fix use of the semaphore (it seems equivalent to not using it). This
> fixes the ab corruption I was experiencing.
>
> Thanks. That fixes the performance issue I was seeing (which was caused
> by an exception during socket close so the lower maxKeepAliveRequests
> the more obvious the problem became). It certainly drives home the
> message that you never, ever want exceptions routinely occurring on the
> critical path.
>
> For the ab results, I am now at (8 to trunk before latest refactoring to
trunk):
NIO1: 50k -> 60k -> 57k
APR: 67k -> 67k -> 62k
NIO2: 65k -> 65k -> 60k
So I see a small difference with this latest refactoring.
Rémy
Re: svn commit: r1652108 - /tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
Posted by Mark Thomas <ma...@apache.org>.
On 15/01/2015 14:28, remm@apache.org wrote:
> Author: remm
> Date: Thu Jan 15 14:28:13 2015
> New Revision: 1652108
>
> URL: http://svn.apache.org/r1652108
> Log:
> Fix use of the semaphore (it seems equivalent to not using it). This fixes the ab corruption I was experiencing.
Thanks. That fixes the performance issue I was seeing (which was caused
by an exception during socket close so the lower maxKeepAliveRequests
the more obvious the problem became). It certainly drives home the
message that you never, ever want exceptions routinely occurring on the
critical path.
I'll run the unit tests locally as well as let the CI system do its
thing to see if I still get the unit test failures I got when I tried to
the patch you just applied. My best guess right now is the patch below
exposed another error that, if I am lucky, I have already fixed.
Mark
> Modified:
> tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
>
> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1652108&r1=1652107&r2=1652108&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Thu Jan 15 14:28:13 2015
> @@ -744,11 +744,13 @@ public class Nio2Endpoint extends Abstra
> failed(new ClosedChannelException(), attachment);
> return;
> }
> + readPending.release();
> getEndpoint().processSocket(attachment, SocketStatus.OPEN_READ, true);
> }
>
> @Override
> public void failed(Throwable exc, SocketWrapperBase<Nio2Channel> attachment) {
> + readPending.release();
> getEndpoint().processSocket(attachment, SocketStatus.DISCONNECT, true);
> }
> };
> @@ -1310,9 +1312,6 @@ public class Nio2Endpoint extends Abstra
> getSocket().getBufHandler().configureReadBufferForWrite();
> getSocket().read(getSocket().getBufHandler().getReadBuffer(),
> getTimeout(), TimeUnit.MILLISECONDS, this, awaitBytesHandler);
> - // TODO Figure out why moving this to the awaitBytesHandler
> - // causes test failures.
> - readPending.release();
> }
> }
> }
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org