You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2017/11/16 13:30:27 UTC
svn commit: r1815451 - in /tomcat/trunk:
java/org/apache/tomcat/util/net/NioBlockingSelector.java
java/org/apache/tomcat/util/net/NioEndpoint.java webapps/docs/changelog.xml
Author: markt
Date: Thu Nov 16 13:30:26 2017
New Revision: 1815451
URL: http://svn.apache.org/viewvc?rev=1815451&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61736
Improve performance of NIO connector when clients leave large time gaps between network packets.
Patch provided by Zilong Song.
This closes #81
Modified:
tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java
tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java?rev=1815451&r1=1815450&r2=1815451&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java Thu Nov 16 13:30:26 2017
@@ -263,10 +263,25 @@ public class NioBlockingSelector {
boolean result = false;
Runnable r = null;
result = (events.size() > 0);
- while ( (r = events.poll()) != null ) {
+
+ /* We only poll and run the runnable events when we start this
+ * method. Further events added to the queue later will be delayed
+ * to the next execution of this method.
+ *
+ * We do in this way, because running event from the events queue
+ * may lead the working thread to add more events to the queue (for
+ * example, the worker thread may add another RunnableAdd event when
+ * waken up by a previous RunnableAdd event who got an invalid
+ * SelectionKey). Trying to consume all the events in an increasing
+ * queue till it's empty, will make the loop hard to be terminated,
+ * which will kill a lot of time, and greatly affect performance of
+ * the poller loop.
+ */
+ for (int i = 0, size = events.size(); i < size && (r = events.poll()) != null; i++) {
r.run();
result = true;
}
+
return result;
}
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1815451&r1=1815450&r2=1815451&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Thu Nov 16 13:30:26 2017
@@ -595,7 +595,7 @@ public class NioEndpoint extends Abstrac
boolean result = false;
PollerEvent pe = null;
- while ( (pe = events.poll()) != null ) {
+ for (int i = 0, size = events.size(); i < size && (pe = events.poll()) != null; i++ ) {
result = true;
try {
pe.run();
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1815451&r1=1815450&r2=1815451&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Thu Nov 16 13:30:26 2017
@@ -111,6 +111,11 @@
InputStream.setReadListener with HTTP/2. (remm)
</fix>
<fix>
+ <bug>61736</bug>: Improve performance of NIO connector when clients
+ leave large time gaps between network packets. Patch provided by Zilong
+ Song. (markt)
+ </fix>
+ <fix>
<bug>61740</bug>: Correct an off-by-one error in the Hpack header index
validation that caused intermittent request failures when using HTTP/2.
(markt)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1815451 - in /tomcat/trunk:
java/org/apache/tomcat/util/net/NioBlockingSelector.java
java/org/apache/tomcat/util/net/NioEndpoint.java webapps/docs/changelog.xml
Posted by Mark Thomas <ma...@apache.org>.
On 16/11/17 14:05, Konstantin Kolinko wrote:
> 2017-11-16 16:30 GMT+03:00 <ma...@apache.org>:
>> Author: markt
>> Date: Thu Nov 16 13:30:26 2017
>> New Revision: 1815451
>>
>> URL: http://svn.apache.org/viewvc?rev=1815451&view=rev
>> Log:
>> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61736
>> Improve performance of NIO connector when clients leave large time gaps between network packets.
>> Patch provided by Zilong Song.
>> This closes #81
>>
>> Modified:
>> tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java
>> tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
>> tomcat/trunk/webapps/docs/changelog.xml
>>
>> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java?rev=1815451&r1=1815450&r2=1815451&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java (original)
>> +++ tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java Thu Nov 16 13:30:26 2017
>> @@ -263,10 +263,25 @@ public class NioBlockingSelector {
>> boolean result = false;
>> Runnable r = null;
>> result = (events.size() > 0);
>
> I think that the above line can be removed,
> or call size() once and save it in a local variable.
Agreed. I'll get that done.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1815451 - in /tomcat/trunk: java/org/apache/tomcat/util/net/NioBlockingSelector.java
java/org/apache/tomcat/util/net/NioEndpoint.java webapps/docs/changelog.xml
Posted by Konstantin Kolinko <kn...@gmail.com>.
2017-11-16 16:30 GMT+03:00 <ma...@apache.org>:
> Author: markt
> Date: Thu Nov 16 13:30:26 2017
> New Revision: 1815451
>
> URL: http://svn.apache.org/viewvc?rev=1815451&view=rev
> Log:
> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61736
> Improve performance of NIO connector when clients leave large time gaps between network packets.
> Patch provided by Zilong Song.
> This closes #81
>
> Modified:
> tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java
> tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
> tomcat/trunk/webapps/docs/changelog.xml
>
> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java?rev=1815451&r1=1815450&r2=1815451&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/net/NioBlockingSelector.java Thu Nov 16 13:30:26 2017
> @@ -263,10 +263,25 @@ public class NioBlockingSelector {
> boolean result = false;
> Runnable r = null;
> result = (events.size() > 0);
I think that the above line can be removed,
or call size() once and save it in a local variable.
org.apache.tomcat.util.collections.SynchronizedQueue.size() is a
"synchronized" method. It is better to avoid duplicate calls to it.
Further, the returned value is not used by the caller of this events() method.
> - while ( (r = events.poll()) != null ) {
> +
> + /* We only poll and run the runnable events when we start this
> + * method. Further events added to the queue later will be delayed
> + * to the next execution of this method.
> + *
> + * We do in this way, because running event from the events queue
> + * may lead the working thread to add more events to the queue (for
> + * example, the worker thread may add another RunnableAdd event when
> + * waken up by a previous RunnableAdd event who got an invalid
> + * SelectionKey). Trying to consume all the events in an increasing
> + * queue till it's empty, will make the loop hard to be terminated,
> + * which will kill a lot of time, and greatly affect performance of
> + * the poller loop.
> + */
> + for (int i = 0, size = events.size(); i < size && (r = events.poll()) != null; i++) {
> r.run();
> result = true;
> }
> +
> return result;
> }
>
>
> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1815451&r1=1815450&r2=1815451&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Thu Nov 16 13:30:26 2017
> @@ -595,7 +595,7 @@ public class NioEndpoint extends Abstrac
> boolean result = false;
>
> PollerEvent pe = null;
> - while ( (pe = events.poll()) != null ) {
> + for (int i = 0, size = events.size(); i < size && (pe = events.poll()) != null; i++ ) {
> result = true;
> try {
> pe.run();
>
> Modified: tomcat/trunk/webapps/docs/changelog.xml
> URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1815451&r1=1815450&r2=1815451&view=diff
> ==============================================================================
> --- tomcat/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/trunk/webapps/docs/changelog.xml Thu Nov 16 13:30:26 2017
> @@ -111,6 +111,11 @@
> InputStream.setReadListener with HTTP/2. (remm)
> </fix>
> <fix>
> + <bug>61736</bug>: Improve performance of NIO connector when clients
> + leave large time gaps between network packets. Patch provided by Zilong
> + Song. (markt)
> + </fix>
> + <fix>
> <bug>61740</bug>: Correct an off-by-one error in the Hpack header index
> validation that caused intermittent request failures when using HTTP/2.
> (markt)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org