From: Willy Tarreau <w@1wt.eu>
Date: Thu, 28 Aug 2008 07:47:43 +0000 (+0200)
Subject: [OPTIM] stream_sock_read must check for null-reads more often
X-Git-Tag: v1.3.16-rc1~168
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2bea3a115525bfa172b90fce43833a25b95aea73;p=thirdparty%2Fhaproxy.git

[OPTIM] stream_sock_read must check for null-reads more often

With small HTTP messages, stream_sock_read() tends to wake the
task up for a message read without indicating that it may be
the last one. The reason is that level-triggered pollers generally
don't report HUP with data, but only afterwards, so stream_sock_read
has no chance to detect this condition and needs a respin.

So now we return on incomplete buffers only when the buffer is known
as a streamer, because here it generally makes sense. The net result
is that the number of calls in a single HTTP session has dropped
from 5 to 3, with one less wake up and several less calls to
stream_sock_data_update().
---

diff --git a/src/stream_sock.c b/src/stream_sock.c
index 3f4be6724d..0b935314a1 100644
--- a/src/stream_sock.c
+++ b/src/stream_sock.c
@@ -180,22 +180,34 @@ int stream_sock_read(int fd) {
 						//fputc('!', stderr);
 					}
 				}
+				/* unfortunately, on level-triggered events, POLL_HUP
+				 * is generally delivered AFTER the system buffer is
+				 * empty, so this one might never match.
+				 */
 				if (fdtab[fd].ev & FD_POLL_HUP)
 					goto out_shutdown_r;
-				break;
+
+				/* if a streamer has read few data, it may be because we
+				 * have exhausted system buffers. It's not worth trying
+				 * again.
+				 */
+				if (b->flags & BF_STREAMER)
+					break;
 			}
 
 			/* generally if we read something smaller than 1 or 2 MSS,
-			 * it means that it's not worth trying to read again. It may
-			 * also happen on headers, but the application then can stop
-			 * reading before we start polling.
+			 * it means that either we have exhausted the system's
+			 * buffers (streamer or question-response protocol) or that
+			 * the connection will be closed. Streamers are easily
+			 * detected so we return early. For other cases, it's still
+			 * better to perform a last read to be sure, because it may
+			 * save one complete poll/read/wakeup cycle in case of shutdown.
 			 */
-			if (ret < MIN_RET_FOR_READ_LOOP)
+			if (ret < MIN_RET_FOR_READ_LOOP && b->flags & BF_STREAMER)
 				break;
 
 			if (--read_poll <= 0)
 				break;
-
 		}
 		else if (ret == 0) {
 			/* connection closed */