[master] 0f7d9a3 Cave in to the microoptimization demands and remove a pointless unlock/lock sequence when using critbit, at the expense of a slightly more ornate API for hashers.

Poul-Henning Kamp phk at varnish-cache.org
Thu Apr 19 09:35:05 CEST 2012


commit 0f7d9a361da8436bf01670be6a86f0b4d17a09ac
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Thu Apr 19 07:33:41 2012 +0000

    Cave in to the microoptimization demands and remove a pointless
    unlock/lock sequence when using critbit, at the expense of a
    slightly more ornate API for hashers.
    
    Also make the "noh" argument optional, as originally intended.

diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index e746813..6facf0a 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -259,7 +259,7 @@ HSH_Insert(struct worker *wrk, const void *digest, struct objcore *oc)
 	AN(wrk->nobjhead);
 	oh = hash->lookup(wrk, digest, &wrk->nobjhead);
 	CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
-	Lck_Lock(&oh->mtx);
+	Lck_AssertHeld(&oh->mtx);
 	assert(oh->refcnt > 0);
 
 	/* Insert (precreated) objcore in objecthead */
@@ -310,6 +310,7 @@ HSH_Lookup(struct sess *sp)
 		 */
 		CHECK_OBJ_NOTNULL(req->hash_objhead, OBJHEAD_MAGIC);
 		oh = req->hash_objhead;
+		Lck_Lock(&oh->mtx);
 		req->hash_objhead = NULL;
 	} else {
 		AN(wrk->nobjhead);
@@ -317,7 +318,8 @@ HSH_Lookup(struct sess *sp)
 	}
 
 	CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
-	Lck_Lock(&oh->mtx);
+	Lck_AssertHeld(&oh->mtx);
+
 	assert(oh->refcnt > 0);
 	busy_found = 0;
 	grace_oc = NULL;
diff --git a/bin/varnishd/hash/hash_classic.c b/bin/varnishd/hash/hash_classic.c
index 87e0561..9121ec1 100644
--- a/bin/varnishd/hash/hash_classic.c
+++ b/bin/varnishd/hash/hash_classic.c
@@ -120,8 +120,8 @@ hcl_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	AN(digest);
-	AN(noh);
-	CHECK_OBJ_NOTNULL(*noh, OBJHEAD_MAGIC);
+	if (noh != NULL)
+		CHECK_OBJ_NOTNULL(*noh, OBJHEAD_MAGIC);
 
 	assert(sizeof oh->digest >= sizeof hdigest);
 	memcpy(&hdigest, digest, sizeof hdigest);
@@ -130,6 +130,7 @@ hcl_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
 
 	Lck_Lock(&hp->mtx);
 	VTAILQ_FOREACH(oh, &hp->head, hoh_list) {
+		CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
 		i = memcmp(oh->digest, digest, sizeof oh->digest);
 		if (i < 0)
 			continue;
@@ -137,9 +138,15 @@ hcl_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
 			break;
 		oh->refcnt++;
 		Lck_Unlock(&hp->mtx);
+		Lck_Lock(&oh->mtx);
 		return (oh);
 	}
 
+	if (noh == NULL) {
+		Lck_Unlock(&hp->mtx);
+		return (NULL);
+	}
+
 	if (oh != NULL)
 		VTAILQ_INSERT_BEFORE(oh, *noh, hoh_list);
 	else
@@ -152,6 +159,7 @@ hcl_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
 	oh->hoh_head = hp;
 
 	Lck_Unlock(&hp->mtx);
+	Lck_Lock(&oh->mtx);
 	return (oh);
 }
 
diff --git a/bin/varnishd/hash/hash_critbit.c b/bin/varnishd/hash/hash_critbit.c
index 5bdab86..08be561 100644
--- a/bin/varnishd/hash/hash_critbit.c
+++ b/bin/varnishd/hash/hash_critbit.c
@@ -425,9 +425,10 @@ hcb_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	AN(digest);
-	AN(noh);
-	CHECK_OBJ_NOTNULL(*noh, OBJHEAD_MAGIC);
-	assert((*noh)->refcnt == 1);
+	if (noh != NULL) {
+		CHECK_OBJ_NOTNULL(*noh, OBJHEAD_MAGIC);
+		assert((*noh)->refcnt == 1);
+	}
 
 	/* First try in read-only mode without holding a lock */
 
@@ -440,11 +441,11 @@ hcb_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
 		 * under us, so fall through and try with the lock held.
 		 */
 		u = oh->refcnt;
-		if (u > 0)
+		if (u > 0) {
 			oh->refcnt++;
-		Lck_Unlock(&oh->mtx);
-		if (u > 0)
 			return (oh);
+		}
+		Lck_Unlock(&oh->mtx);
 	}
 
 	while (1) {
@@ -455,23 +456,27 @@ hcb_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
 		oh = hcb_insert(wrk, &hcb_root, digest, noh);
 		Lck_Unlock(&hcb_mtx);
 
+		if (oh == NULL)
+			return (NULL);
+
+		Lck_Lock(&oh->mtx);
+
 		CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
-		if (*noh == NULL) {
+		if (noh != NULL && *noh == NULL) {
 			assert(oh->refcnt > 0);
 			VSC_C_main->hcb_insert++;
 			return (oh);
 		}
-		Lck_Lock(&oh->mtx);
 		/*
 		 * A refcount of zero indicates that the tree changed
 		 * under us, so fall through and try with the lock held.
 		 */
 		u = oh->refcnt;
-		if (u > 0)
+		if (u > 0) {
 			oh->refcnt++;
-		Lck_Unlock(&oh->mtx);
-		if (u > 0)
 			return (oh);
+		}
+		Lck_Unlock(&oh->mtx);
 	}
 }
 
diff --git a/bin/varnishd/hash/hash_simple_list.c b/bin/varnishd/hash/hash_simple_list.c
index d70e76f..d99bb89 100644
--- a/bin/varnishd/hash/hash_simple_list.c
+++ b/bin/varnishd/hash/hash_simple_list.c
@@ -67,8 +67,8 @@ hsl_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	AN(digest);
-	AN(noh);
-	CHECK_OBJ_NOTNULL(*noh, OBJHEAD_MAGIC);
+	if (noh != NULL)
+		CHECK_OBJ_NOTNULL(*noh, OBJHEAD_MAGIC);
 
 	Lck_Lock(&hsl_mtx);
 	VTAILQ_FOREACH(oh, &hsl_head, hoh_list) {
@@ -79,9 +79,13 @@ hsl_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
 			break;
 		oh->refcnt++;
 		Lck_Unlock(&hsl_mtx);
+		Lck_Lock(&oh->mtx);
 		return (oh);
 	}
 
+	if (noh == NULL)
+		return (NULL);
+
 	if (oh != NULL)
 		VTAILQ_INSERT_BEFORE(oh, *noh, hoh_list);
 	else
@@ -91,6 +95,7 @@ hsl_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
 	*noh = NULL;
 	memcpy(oh->digest, digest, sizeof oh->digest);
 	Lck_Unlock(&hsl_mtx);
+	Lck_Lock(&oh->mtx);
 	return (oh);
 }
 



More information about the varnish-commit mailing list