[master] 5c82680 Integrate the waiting list into the objhead (again). The net saving of one pointer does not amortize the extra CPU overhead and cache misses.

Poul-Henning Kamp phk at FreeBSD.org
Wed Jan 6 21:20:54 CET 2016


commit 5c8268062bf06a2700dd27331c29c48d650c9197
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Wed Jan 6 20:19:25 2016 +0000

    Integrate the waiting list into the objhead (again).  The net saving
    of one pointer does not amortize the extra CPU overhead and cache misses.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 4d60624..1e82ea2 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -121,7 +121,6 @@ struct sess;
 struct suckaddr;
 struct vrt_priv;
 struct vsb;
-struct waitinglist;
 struct worker;
 struct v1l;
 
@@ -329,7 +328,6 @@ struct worker {
 	struct pool		*pool;
 	struct objhead		*nobjhead;
 	struct objcore		*nobjcore;
-	struct waitinglist	*nwaitinglist;
 	void			*nhashpriv;
 	struct dstat		stats[1];
 	struct vsl_log		*vsl;		// borrowed from req/bo
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index efdffff..d7411b0 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -795,7 +795,7 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
 	http_TimeHeader(bo->beresp, "Date: ", now);
 	http_SetHeader(bo->beresp, "Server: Varnish");
 
-	if (bo->fetch_objcore->objhead->waitinglist != NULL) {
+	if (!VTAILQ_EMPTY(&bo->fetch_objcore->objhead->waitinglist)) {
 		/*
 		 * If there is a waitinglist, it means that there is no
 		 * grace-able object, so cache the error return for a
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 0bb827d..e22289d 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -90,6 +90,7 @@ hsh_newobjhead(void)
 	XXXAN(oh);
 	oh->refcnt = 1;
 	VTAILQ_INIT(&oh->objcs);
+	VTAILQ_INIT(&oh->waitinglist);
 	Lck_New(&oh->mtx, lck_objhdr);
 	return (oh);
 }
@@ -99,7 +100,6 @@ hsh_newobjhead(void)
 static void
 hsh_prealloc(struct worker *wrk)
 {
-	struct waitinglist *wl;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 
@@ -113,15 +113,6 @@ hsh_prealloc(struct worker *wrk)
 	}
 	CHECK_OBJ_NOTNULL(wrk->nobjhead, OBJHEAD_MAGIC);
 
-	if (wrk->nwaitinglist == NULL) {
-		ALLOC_OBJ(wl, WAITINGLIST_MAGIC);
-		XXXAN(wl);
-		VTAILQ_INIT(&wl->list);
-		wrk->nwaitinglist = wl;
-		wrk->stats->n_waitinglist++;
-	}
-	CHECK_OBJ_NOTNULL(wrk->nwaitinglist, WAITINGLIST_MAGIC);
-
 	if (hash->prep != NULL)
 		hash->prep(wrk);
 }
@@ -163,11 +154,6 @@ HSH_Cleanup(struct worker *wrk)
 		wrk->nobjhead = NULL;
 		wrk->stats->n_objecthead--;
 	}
-	if (wrk->nwaitinglist != NULL) {
-		FREE_OBJ(wrk->nwaitinglist);
-		wrk->nwaitinglist = NULL;
-		wrk->stats->n_waitinglist--;
-	}
 	if (wrk->nhashpriv != NULL) {
 		/* XXX: If needed, add slinger method for this */
 		free(wrk->nhashpriv);
@@ -181,6 +167,7 @@ HSH_DeleteObjHead(struct worker *wrk, struct objhead *oh)
 
 	AZ(oh->refcnt);
 	assert(VTAILQ_EMPTY(&oh->objcs));
+	assert(VTAILQ_EMPTY(&oh->waitinglist));
 	Lck_Delete(&oh->mtx);
 	wrk->stats->n_objecthead--;
 	FREE_OBJ(oh);
@@ -482,13 +469,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp,
 	AZ(req->hash_ignore_busy);
 
 	if (wait_for_busy) {
-		CHECK_OBJ_NOTNULL(wrk->nwaitinglist, WAITINGLIST_MAGIC);
-		if (oh->waitinglist == NULL) {
-			oh->waitinglist = wrk->nwaitinglist;
-			wrk->nwaitinglist = NULL;
-		}
-		VTAILQ_INSERT_TAIL(&oh->waitinglist->list,
-		    req, w_list);
+		VTAILQ_INSERT_TAIL(&oh->waitinglist, req, w_list);
 		if (DO_DEBUG(DBG_WAITINGLIST))
 			VSLb(req->vsl, SLT_Debug, "on waiting list <%p>", oh);
 	} else {
@@ -517,21 +498,18 @@ hsh_rush(struct worker *wrk, struct objhead *oh)
 	unsigned u;
 	struct req *req;
 	struct sess *sp;
-	struct waitinglist *wl;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
 	Lck_AssertHeld(&oh->mtx);
-	wl = oh->waitinglist;
-	CHECK_OBJ_NOTNULL(wl, WAITINGLIST_MAGIC);
 	for (u = 0; u < cache_param->rush_exponent; u++) {
-		req = VTAILQ_FIRST(&wl->list);
+		req = VTAILQ_FIRST(&oh->waitinglist);
 		if (req == NULL)
 			break;
 		CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 		wrk->stats->busy_wakeup++;
 		AZ(req->wrk);
-		VTAILQ_REMOVE(&wl->list, req, w_list);
+		VTAILQ_REMOVE(&oh->waitinglist, req, w_list);
 		DSL(DBG_WAITINGLIST, req->vsl->wid, "off waiting list");
 		if (SES_Reschedule_Req(req)) {
 			/*
@@ -548,22 +526,17 @@ hsh_rush(struct worker *wrk, struct objhead *oh)
 				CNT_AcctLogCharge(wrk->stats, req);
 				Req_Release(req);
 				SES_Delete(sp, SC_OVERLOAD, NAN);
-				req = VTAILQ_FIRST(&wl->list);
+				req = VTAILQ_FIRST(&oh->waitinglist);
 				if (req == NULL)
 					break;
 				CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
-				VTAILQ_REMOVE(&wl->list, req, w_list);
+				VTAILQ_REMOVE(&oh->waitinglist, req, w_list);
 				DSL(DBG_WAITINGLIST, req->vsl->wid,
 				    "kill from waiting list");
 			}
 			break;
 		}
 	}
-	if (VTAILQ_EMPTY(&wl->list)) {
-		oh->waitinglist = NULL;
-		FREE_OBJ(wl);
-		wrk->stats->n_waitinglist--;
-	}
 }
 
 /*---------------------------------------------------------------------
@@ -706,7 +679,7 @@ HSH_Unbusy(struct worker *wrk, struct objcore *oc)
 	VTAILQ_REMOVE(&oh->objcs, oc, list);
 	VTAILQ_INSERT_HEAD(&oh->objcs, oc, list);
 	oc->flags &= ~OC_F_BUSY;
-	if (oh->waitinglist != NULL)
+	if (!VTAILQ_EMPTY(&oh->waitinglist))
 		hsh_rush(wrk, oh);
 	Lck_Unlock(&oh->mtx);
 }
@@ -781,7 +754,7 @@ HSH_DerefObjCore(struct worker *wrk, struct objcore **ocp)
 	r = --oc->refcnt;
 	if (!r)
 		VTAILQ_REMOVE(&oh->objcs, oc, list);
-	if (oh->waitinglist != NULL)
+	if (!VTAILQ_EMPTY(&oh->waitinglist))
 		hsh_rush(wrk, oh);
 	Lck_Unlock(&oh->mtx);
 	if (r != 0)
@@ -814,13 +787,13 @@ HSH_DerefObjHead(struct worker *wrk, struct objhead **poh)
 	CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
 
 	if (oh == private_oh) {
-		AZ(oh->waitinglist);
+		assert(VTAILQ_EMPTY(&oh->waitinglist));
 		Lck_Lock(&oh->mtx);
 		assert(oh->refcnt > 1);
 		oh->refcnt--;
 		Lck_Unlock(&oh->mtx);
 		return(1);
-	} else if (oh->waitinglist != NULL) {
+	} else if (!VTAILQ_EMPTY(&oh->waitinglist)) {
 		Lck_Lock(&oh->mtx);
 		hsh_rush(wrk, oh);
 		Lck_Unlock(&oh->mtx);
diff --git a/bin/varnishd/hash/hash_critbit.c b/bin/varnishd/hash/hash_critbit.c
index c4239b6..da671f5 100644
--- a/bin/varnishd/hash/hash_critbit.c
+++ b/bin/varnishd/hash/hash_critbit.c
@@ -406,8 +406,6 @@ hcb_deref(struct objhead *oh)
 		hcb_delete(&hcb_root, oh);
 		VTAILQ_INSERT_TAIL(&cool_h, oh, hoh_list);
 		Lck_Unlock(&hcb_mtx);
-		assert(VTAILQ_EMPTY(&oh->objcs));
-		AZ(oh->waitinglist);
 	}
 	Lck_Unlock(&oh->mtx);
 #ifdef PHK
diff --git a/bin/varnishd/hash/hash_slinger.h b/bin/varnishd/hash/hash_slinger.h
index 736f1b5..1fa219e 100644
--- a/bin/varnishd/hash/hash_slinger.h
+++ b/bin/varnishd/hash/hash_slinger.h
@@ -77,12 +77,6 @@ struct objcore *HSH_Private(struct worker *wrk);
 
 #ifdef VARNISH_CACHE_CHILD
 
-struct waitinglist {
-	unsigned		magic;
-#define WAITINGLIST_MAGIC	0x063a477a
-	VTAILQ_HEAD(, req)	list;
-};
-
 struct objhead {
 	unsigned		magic;
 #define OBJHEAD_MAGIC		0x1b96615d
@@ -91,7 +85,7 @@ struct objhead {
 	struct lock		mtx;
 	VTAILQ_HEAD(,objcore)	objcs;
 	uint8_t			digest[DIGEST_LEN];
-	struct waitinglist	*waitinglist;
+	VTAILQ_HEAD(, req)	waitinglist;
 
 	/*----------------------------------------------------
 	 * The fields below are for the sole private use of



More information about the varnish-commit mailing list