[4.1] b6645ff Call the "complete" function first, the read more in SES_RxStuff()

Poul-Henning Kamp phk at FreeBSD.org
Fri Sep 4 15:54:54 CEST 2015


commit b6645ff23f84633f4061aa6274318870239bc227
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Fri Aug 14 07:18:34 2015 +0000

    Call the "complete" function first, the read more in SES_RxStuff()
    
    This saves an entire state in the H1 FSM. (At the cost of not
    differentiating between read-ahead and full pipelining in VSC.)
    
    Inspired by patches from Lasse & Martin

diff --git a/bin/varnishd/cache/cache_session.c b/bin/varnishd/cache/cache_session.c
index 21f405f..69bf46d 100644
--- a/bin/varnishd/cache/cache_session.c
+++ b/bin/varnishd/cache/cache_session.c
@@ -232,6 +232,7 @@ SES_RxStuff(struct http_conn *htc, htc_complete_f *func, double t0,
 	double tmo;
 	double now;
 	enum htc_status_e hs;
+	int i;
 
 	CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
 
@@ -240,30 +241,12 @@ SES_RxStuff(struct http_conn *htc, htc_complete_f *func, double t0,
 	if (t1 != NULL)
 		assert(isnan(*t1));
 
-	now = t0;
 	while (1) {
-		if (!isnan(ti))
-			tmo = ti - t0;
-		else
-			tmo = tn - t0;
-		hs = SES_Rx(htc, tmo);
-		if (hs == HTC_S_EOF) {
-			WS_ReleaseP(htc->ws, htc->rxbuf_b);
-			return (HTC_S_CLOSE);
-		}
-		if (hs == HTC_S_OVERFLOW) {
-			WS_ReleaseP(htc->ws, htc->rxbuf_b);
-			return (HTC_S_OVERFLOW);
-		}
 		now = VTIM_real();
 		hs = func(htc);
-		if (hs == HTC_S_OVERFLOW) {
+		if (hs == HTC_S_OVERFLOW || hs == HTC_S_JUNK) {
 			WS_ReleaseP(htc->ws, htc->rxbuf_b);
-			return (HTC_S_OVERFLOW);
-		}
-		if (hs == HTC_S_JUNK) {
-			WS_ReleaseP(htc->ws, htc->rxbuf_b);
-			return (HTC_S_JUNK);
+			return (hs);
 		}
 		if (hs == HTC_S_COMPLETE) {
 			WS_ReleaseP(htc->ws, htc->rxbuf_e);
@@ -274,18 +257,37 @@ SES_RxStuff(struct http_conn *htc, htc_complete_f *func, double t0,
 				*t2 = now;
 			return (HTC_S_COMPLETE);
 		}
-		if (tn < now)
+		if (tn < now) {
+			/* XXX: WS_ReleaseP(htc->ws, htc->rxbuf_b); ? */
 			return (HTC_S_TIMEOUT);
+		}
 		if (hs == HTC_S_MORE) {
 			/* Working on it */
 			if (t1 != NULL && isnan(*t1))
 				*t1 = now;
 			tmo = tn - now;
-			continue;
+		} else if (hs != HTC_S_EMPTY)
+			WRONG("htc_status_e");
+
+		tmo = tn - t0;
+		if (!isnan(ti) && ti < tn)
+			tmo = ti - t0;
+		i = (htc->ws->r - htc->rxbuf_e) - 1;	/* space for NUL */
+		if (i <= 0) {
+			WS_ReleaseP(htc->ws, htc->rxbuf_b);
+			return (HTC_S_OVERFLOW);
+		}
+		i = VTCP_read(htc->fd, htc->rxbuf_e, i, tmo);
+		if (i == 0 || i == -1) {
+			WS_ReleaseP(htc->ws, htc->rxbuf_b);
+			return (HTC_S_EOF);
+		} else if (i > 0) {
+			htc->rxbuf_e += i;
+			*htc->rxbuf_e = '\0';
+		} else if (i == -2) {
+			if (hs == HTC_S_EMPTY && ti < now)
+				return (HTC_S_IDLE);
 		}
-		assert(hs == HTC_S_EMPTY);
-		if (ti < now)
-			return (HTC_S_IDLE);
 	}
 }
 
diff --git a/bin/varnishd/http1/cache_http1_fsm.c b/bin/varnishd/http1/cache_http1_fsm.c
index 6acf78c..7d7eef3 100644
--- a/bin/varnishd/http1/cache_http1_fsm.c
+++ b/bin/varnishd/http1/cache_http1_fsm.c
@@ -235,7 +235,16 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 			if (hs != HTC_S_COMPLETE)
 				WRONG("htc_status (nonbad)");
 
-			sp->sess_step = S_STP_H1WORKING;
+			i = http1_dissect(wrk, req);
+			req->acct.req_hdrbytes +=
+			    req->htc->rxbuf_e - req->htc->rxbuf_b;
+			if (i) {
+				SES_Close(req->sp, req->doclose);
+				sp->sess_step = S_STP_H1CLEANUP;
+				break;
+			}
+			req->req_step = R_STP_RECV;
+			sp->sess_step = S_STP_H1PROC;
 			break;
 		case S_STP_H1BUSY:
 			/*
@@ -252,18 +261,6 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 			}
 			sp->sess_step = S_STP_H1PROC;
 			break;
-		case S_STP_H1WORKING:
-			i = http1_dissect(wrk, req);
-			req->acct.req_hdrbytes +=
-			    req->htc->rxbuf_e - req->htc->rxbuf_b;
-			if (i) {
-				SES_Close(req->sp, req->doclose);
-				sp->sess_step = S_STP_H1CLEANUP;
-				break;
-			}
-			req->req_step = R_STP_RECV;
-			sp->sess_step = S_STP_H1PROC;
-			break;
 		case S_STP_H1PROC:
 			req->transport = &http1_transport;
 			if (CNT_Request(wrk, req) == REQ_FSM_DISEMBARK) {
@@ -277,18 +274,9 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 			if (Req_Cleanup(sp, wrk, req))
 				return;
 			SES_RxReInit(req->htc);
-			if (HTTP1_Complete(req->htc) == HTC_S_COMPLETE) {
-				WS_ReleaseP(req->htc->ws, req->htc->rxbuf_e);
-				AZ(req->vsl->wid);
-				req->t_first = req->t_req = sp->t_idle;
-				wrk->stats->sess_pipeline++;
-				sp->sess_step = S_STP_H1WORKING;
-			} else {
-				if (req->htc->rxbuf_e != req->htc->rxbuf_b)
-					wrk->stats->sess_readahead++;
-				sp->sess_step = S_STP_H1NEWREQ;
-
-			}
+			if (req->htc->rxbuf_e != req->htc->rxbuf_b)
+				wrk->stats->sess_readahead++;
+			sp->sess_step = S_STP_H1NEWREQ;
 			break;
 		default:
 			WRONG("Wrong H1 session state");
diff --git a/bin/varnishtest/tests/b00012.vtc b/bin/varnishtest/tests/b00012.vtc
index 81113b5..224bd1a 100644
--- a/bin/varnishtest/tests/b00012.vtc
+++ b/bin/varnishtest/tests/b00012.vtc
@@ -27,4 +27,4 @@ client c1 {
 	expect resp.http.x-varnish == "1005 1004"
 } -run
 
-varnish v1 -expect sess_pipeline == 2
+varnish v1 -expect sess_readahead == 2
diff --git a/include/tbl/steps.h b/include/tbl/steps.h
index a5e618a..2c805d4 100644
--- a/include/tbl/steps.h
+++ b/include/tbl/steps.h
@@ -36,7 +36,6 @@ SESS_STEP(h1newreq,	H1NEWREQ)
 SESS_STEP(h1proc,	H1PROC)
 SESS_STEP(h1busy,	H1BUSY)
 SESS_STEP(h1cleanup,	H1CLEANUP)
-SESS_STEP(h1working,	H1WORKING)
 SESS_STEP(h1_last,	H1_LAST)
 
 SESS_STEP(proxynewsess,	PROXYNEWSESS)
diff --git a/include/tbl/vsc_f_main.h b/include/tbl/vsc_f_main.h
index 3c6c8d0..94341b5 100644
--- a/include/tbl/vsc_f_main.h
+++ b/include/tbl/vsc_f_main.h
@@ -384,10 +384,6 @@ VSC_F(sess_closed_err,		uint64_t, 0, 'c', 'i', info,
 	"Total number of sessions closed with errors."
 	" See sc_* diag counters for detailed breakdown"
 )
-VSC_F(sess_pipeline,		uint64_t, 1, 'c', 'i', info,
-    "Session Pipeline",
-	""
-)
 VSC_F(sess_readahead,		uint64_t, 1, 'c', 'i', info,
     "Session Read Ahead",
 	""



More information about the varnish-commit mailing list