[master] c0442f9 Only allocate sp->req where we need it.

Poul-Henning Kamp phk at varnish-cache.org
Fri Dec 23 09:56:29 CET 2011


commit c0442f9041083486532712589bc7584ef9f9c53b
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Fri Dec 23 08:55:52 2011 +0000

    Only allocate sp->req where we need it.
    
    It's lifetime is now controlled by the timeout_linger parameter, so that
    the waiters won't have to think about it at all.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index b674bd7..3c37176 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -922,6 +922,8 @@ struct sesspool *SES_NewPool(struct pool *pp, unsigned pool_no);
 void SES_DeletePool(struct sesspool *sp, struct worker *wrk);
 int SES_Schedule(struct sess *sp);
 void SES_Handle(struct sess *sp, double now);
+void SES_GetReq(struct sess *sp);
+void SES_ReleaseReq(struct sess *sp);
 
 /* cache_shmlog.c */
 extern struct VSC_C_main *VSC_C_main;
diff --git a/bin/varnishd/cache/cache_center.c b/bin/varnishd/cache/cache_center.c
index 6587bf2..a119360 100644
--- a/bin/varnishd/cache/cache_center.c
+++ b/bin/varnishd/cache/cache_center.c
@@ -37,7 +37,7 @@
  * a dot(1) graph in the source code comments.  So to see the big picture,
  * extract the DOT lines and run though dot(1), for instance with the
  * command:
- *	sed -n '/^DOT/s///p' cache_center.c | dot -Tps > /tmp/_.ps
+ *	sed -n '/^DOT/s///p' cache/cache_center.c | dot -Tps > /tmp/_.ps
  */
 
 /*
@@ -91,7 +91,8 @@ DOT	]
 DOT	herding [shape=hexagon]
 DOT	wait -> start [label="got req"]
 DOT	wait -> "SES_Delete()" [label="errors"]
-DOT	wait -> herding [label="timeout"]
+DOT	wait -> herding [label="timeout_linger"]
+DOT	herding -> wait [label="fd read_ready"]
 DOT }
  */
 
@@ -106,6 +107,9 @@ cnt_wait(struct sess *sp)
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 	wrk = sp->wrk;
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
+	if (sp->req == NULL)
+		SES_GetReq(sp);
+
 	AZ(sp->vcl);
 	AZ(wrk->obj);
 	AZ(sp->esi_level);
@@ -150,6 +154,7 @@ cnt_wait(struct sess *sp)
 				sp->t_req = NAN;
 				wrk->stats.sess_herd++;
 				SES_Charge(sp);
+				SES_ReleaseReq(sp);
 				WAIT_Enter(sp);
 				return (1);
 			}
@@ -350,10 +355,10 @@ cnt_deliver(struct sess *sp)
  *
 DOT	DONE [
 DOT		shape=hexagon
-DOT		label="Request completed"
+DOT		label="cnt_done:\nRequest completed"
 DOT	]
 DOT	ESI_RESP [ shape=hexagon ]
-DOT	DONE -> start
+DOT	DONE -> start [label="full pipeline"]
 DOT	DONE -> wait
 DOT	DONE -> ESI_RESP
  */
@@ -446,21 +451,11 @@ cnt_done(struct sess *sp)
 		sp->step = STP_START;
 		return (0);
 	}
-	if (Tlen(sp->htc->rxbuf)) {
+	if (Tlen(sp->htc->rxbuf))
 		wrk->stats.sess_readahead++;
-		sp->step = STP_WAIT;
-		sp->t_req = sp->t_idle;
-		return (0);
-	}
-	if (cache_param->timeout_linger > 0.) {
-		wrk->stats.sess_linger++;
-		sp->step = STP_WAIT;
-		sp->t_req = sp->t_idle;		// XXX: not quite correct
-		return (0);
-	}
-	wrk->stats.sess_herd++;
-	WAIT_Enter(sp);
-	return (1);
+	sp->step = STP_WAIT;
+	sp->t_req = sp->t_idle;
+	return (0);
 }
 
 /*--------------------------------------------------------------------
@@ -1023,7 +1018,7 @@ cnt_streambody(struct sess *sp)
 DOT subgraph xcluster_first {
 DOT	first [
 DOT		shape=box
-DOT		label="first\nConfigure data structures"
+DOT		label="cnt_first:\nSockaddr's"
 DOT	]
 DOT }
 DOT first -> wait
@@ -1567,11 +1562,11 @@ cnt_recv(struct sess *sp)
 
 /*--------------------------------------------------------------------
  * START
- * Handle a request, wherever it came from recv/restart.
+ * Handle a request.
  *
 DOT start [
 DOT	shape=box
-DOT	label="Dissect request\nHandle expect"
+DOT	label="cnt_start:\nDissect request\nHandle expect"
 DOT ]
 DOT start -> recv [style=bold,color=green]
 DOT start -> DONE [label=errors]
@@ -1588,6 +1583,7 @@ cnt_start(struct sess *sp)
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 	wrk = sp->wrk;
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
+	CHECK_OBJ_NOTNULL(sp->req, REQ_MAGIC);
 	AZ(sp->restarts);
 	AZ(wrk->obj);
 	AZ(sp->vcl);
@@ -1684,8 +1680,10 @@ CNT_Session(struct sess *sp)
 	struct worker *wrk;
 
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
+#if 0
 	CHECK_OBJ_NOTNULL(sp->req, REQ_MAGIC);
 	MPL_AssertSane(sp->req);
+#endif
 	wrk = sp->wrk;
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 
@@ -1723,8 +1721,10 @@ CNT_Session(struct sess *sp)
 	 */
 	for (done = 0; !done; ) {
 		assert(sp->wrk == wrk);
+#if 0
 		CHECK_OBJ_NOTNULL(sp->req, REQ_MAGIC);
 		MPL_AssertSane(sp->req);
+#endif
 		/*
 		 * This is a good place to be paranoid about the various
 		 * pointers still pointing to the things we expect.
diff --git a/bin/varnishd/cache/cache_session.c b/bin/varnishd/cache/cache_session.c
index 1292e5e..530c96e 100644
--- a/bin/varnishd/cache/cache_session.c
+++ b/bin/varnishd/cache/cache_session.c
@@ -218,9 +218,6 @@ SES_New(struct worker *wrk, struct sesspool *pp)
 	if (sm == NULL)
 		return (NULL);
 	sp = &sm->sess;
-	sp->req = MPL_Get(pp->mpl_req, NULL);
-	AN(sp->req);
-	sp->req->magic = REQ_MAGIC;
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 	return (sp);
 }
@@ -245,24 +242,33 @@ SES_Alloc(void)
 }
 
 /*--------------------------------------------------------------------
- * Schedule a session back on a work-thread from its pool
  */
 
-int
-SES_Schedule(struct sess *sp)
+static struct sesspool *
+ses_getpool(const struct sess *sp)
 {
 	struct sessmem *sm;
 	struct sesspool *pp;
 
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
-
-	AZ(sp->wrk);
-
 	sm = sp->mem;
 	CHECK_OBJ_NOTNULL(sm, SESSMEM_MAGIC);
-
 	pp = sm->pool;
 	CHECK_OBJ_NOTNULL(pp, SESSPOOL_MAGIC);
+	return (pp);
+}
+
+/*--------------------------------------------------------------------
+ * Schedule a session back on a work-thread from its pool
+ */
+
+int
+SES_Schedule(struct sess *sp)
+{
+	struct sesspool *pp;
+
+	pp = ses_getpool(sp);
+	AZ(sp->wrk);
 
 	AN(pp->pool);
 
@@ -330,14 +336,10 @@ SES_Delete(struct sess *sp, const char *reason, double now)
 	struct worker *wrk;
 	struct sesspool *pp;
 
+	pp = ses_getpool(sp);
 
-	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
-	CHECK_OBJ_NOTNULL(sp->req, REQ_MAGIC);
-	MPL_AssertSane(sp->req);
 	sm = sp->mem;
 	CHECK_OBJ_NOTNULL(sm, SESSMEM_MAGIC);
-	pp = sm->pool;
-	CHECK_OBJ_NOTNULL(pp, SESSPOOL_MAGIC);
 	wrk = sp->wrk;
 	CHECK_OBJ_ORNULL(wrk, WORKER_MAGIC);
 
@@ -348,6 +350,9 @@ SES_Delete(struct sess *sp, const char *reason, double now)
 	assert(!isnan(sp->t_open));
 	assert(sp->fd < 0);
 
+	if (sp->req != NULL)
+		SES_ReleaseReq(sp);
+
 	AZ(sp->vcl);
 	if (*sp->addr == '\0')
 		strcpy(sp->addr, "-");
@@ -362,11 +367,6 @@ SES_Delete(struct sess *sp, const char *reason, double now)
 	    b->sess, b->req, b->pipe, b->pass,
 	    b->fetch, b->hdrbytes, b->bodybytes);
 
-	CHECK_OBJ_NOTNULL(sp->req, REQ_MAGIC);
-	MPL_AssertSane(sp->req);
-	MPL_Free(pp->mpl_req, sp->req);
-	sp->req = NULL;
-
 	if (sm->workspace != cache_param->sess_workspace ||
 	    sm->nhttp != (uint16_t)cache_param->http_max_hdr ||
 	    pp->nsess > cache_param->max_sess) {
@@ -392,45 +392,73 @@ SES_Delete(struct sess *sp, const char *reason, double now)
 }
 
 /*--------------------------------------------------------------------
+ * Alloc/Free/Clean sp->req
+ */
+
+void
+SES_GetReq(struct sess *sp)
+{
+	struct sesspool *pp;
+
+	pp = ses_getpool(sp);
+	AZ(sp->req);
+	sp->req = MPL_Get(pp->mpl_req, NULL);
+	AN(sp->req);
+	sp->req->magic = REQ_MAGIC;
+}
+
+void
+SES_ReleaseReq(struct sess *sp)
+{
+	struct sesspool *pp;
+
+	pp = ses_getpool(sp);
+	CHECK_OBJ_NOTNULL(sp->req, REQ_MAGIC);
+	MPL_AssertSane(sp->req);
+	MPL_Free(pp->mpl_req, sp->req);
+	sp->req = NULL;
+}
+
+/*--------------------------------------------------------------------
  * Create and delete pools
  */
 
 struct sesspool *
-SES_NewPool(struct pool *pp, unsigned pool_no)
+SES_NewPool(struct pool *wp, unsigned pool_no)
 {
-	struct sesspool *sp;
+	struct sesspool *pp;
 	char nb[8];
 
-	ALLOC_OBJ(sp, SESSPOOL_MAGIC);
-	AN(sp);
-	sp->pool = pp;
-	VTAILQ_INIT(&sp->freelist);
-	Lck_New(&sp->mtx, lck_sessmem);
+	ALLOC_OBJ(pp, SESSPOOL_MAGIC);
+	AN(pp);
+	pp->pool = wp;
+	VTAILQ_INIT(&pp->freelist);
+	Lck_New(&pp->mtx, lck_sessmem);
 	bprintf(nb, "req%u", pool_no);
-	sp->req_size = sizeof (struct req);
-	sp->mpl_req = MPL_New(nb, &cache_param->req_pool, &sp->req_size);
-	return (sp);
+	pp->req_size = sizeof (struct req);
+	pp->mpl_req = MPL_New(nb, &cache_param->req_pool, &pp->req_size);
+	return (pp);
 }
 
 void
-SES_DeletePool(struct sesspool *sp, struct worker *wrk)
+SES_DeletePool(struct sesspool *pp, struct worker *wrk)
 {
 	struct sessmem *sm;
 
-	CHECK_OBJ_NOTNULL(sp, SESSPOOL_MAGIC);
+	CHECK_OBJ_NOTNULL(pp, SESSPOOL_MAGIC);
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
-	Lck_Lock(&sp->mtx);
-	while (!VTAILQ_EMPTY(&sp->freelist)) {
-		sm = VTAILQ_FIRST(&sp->freelist);
+	Lck_Lock(&pp->mtx);
+	while (!VTAILQ_EMPTY(&pp->freelist)) {
+		sm = VTAILQ_FIRST(&pp->freelist);
 		CHECK_OBJ_NOTNULL(sm, SESSMEM_MAGIC);
-		VTAILQ_REMOVE(&sp->freelist, sm, list);
+		VTAILQ_REMOVE(&pp->freelist, sm, list);
 		FREE_OBJ(sm);
 		wrk->stats.sessmem_free++;
-		sp->nsess--;
+		pp->nsess--;
 	}
-	AZ(sp->nsess);
-	Lck_Unlock(&sp->mtx);
-	Lck_Delete(&sp->mtx);
-	MPL_Destroy(&sp->mpl_req);
-	FREE_OBJ(sp);
+	AZ(pp->nsess);
+	Lck_Unlock(&pp->mtx);
+	Lck_Delete(&pp->mtx);
+	MPL_Destroy(&pp->mpl_req);
+	FREE_OBJ(pp);
 }
diff --git a/bin/varnishd/waiter/cache_waiter.c b/bin/varnishd/waiter/cache_waiter.c
index 8aa8684..44e9721 100644
--- a/bin/varnishd/waiter/cache_waiter.c
+++ b/bin/varnishd/waiter/cache_waiter.c
@@ -68,6 +68,7 @@ WAIT_Enter(struct sess *sp)
 
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 	CHECK_OBJ_NOTNULL(sp->wrk, WORKER_MAGIC);
+	AZ(sp->req);
 	AZ(sp->vcl);
 	assert(sp->fd >= 0);
 	sp->wrk = NULL;



More information about the varnish-commit mailing list