[master] 89631a9ae reshuffle HSH_Lookup() code a bit to condense and clarify
hermunn
hermunn at varnish-software.com
Mon Jul 2 12:39:08 UTC 2018
commit 89631a9aed793f14d64cd16315b0e11ea0642e0a
Author: Nils Goroll <nils.goroll at uplex.de>
Date: Tue Jun 19 17:02:36 2018 +0200
reshuffle HSH_Lookup() code a bit to condense and clarify
With the req.grace reintroduction, we got two identical cases where
we insert a busy object: When there is no exp_oc (obviously) or
when the exp_oc does not fulfil the req.grace limit. This lead
to some code duplication.
IMHO, handling first the case where we need to insert a busy object
(with or without exp_oc) and then handling the exception that we
do not wait on a busy object if the exp_oc is in grace is cleaner.
Please note that comparing this change using diff -u does not
make much sense, I believe the simplicity of the reshuffling
change becomes clear when looking at the old and new code side-by-side
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 38531b149..3fbbc702d 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -478,58 +478,45 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp,
busy_found = 0;
}
- if (exp_oc != NULL && EXP_Ttl_grace(req, exp_oc) < req->t_req) {
- /* the newest object is out of grace */
- if (!busy_found) {
- /*
- * here we get to insert the busy object. This
- * translates to a MISS with a 304 candidate.
- */
+ if (!busy_found) {
+ /* Insert objcore in objecthead */
+ *bocp = hsh_insert_busyobj(wrk, oh);
+
+ if (exp_oc != NULL) {
assert(oh->refcnt > 1);
assert(exp_oc->objhead == oh);
exp_oc->refcnt++;
- /* Insert objcore in objecthead and release mutex */
- *bocp = hsh_insert_busyobj(wrk, oh);
- /* NB: no deref of objhead, new object inherits reference */
Lck_Unlock(&oh->mtx);
*ocp = exp_oc;
- return (HSH_MISS);
+
+ if (EXP_Ttl_grace(req, exp_oc) < req->t_req) {
+ retval = HSH_MISS;
+ } else {
+ if (exp_oc->hits < LONG_MAX)
+ exp_oc->hits++;
+ retval = HSH_EXPBUSY;
+ }
} else {
- /* we have no use for this very expired object */
- exp_oc = NULL;
+ Lck_Unlock(&oh->mtx);
+ retval = HSH_MISS;
}
+
+ return (retval);
}
- if (exp_oc != NULL) {
- /*
- * here the object is within grace, so we expect it to
- * be delivered
- */
+
+ AN(busy_found);
+ if (exp_oc != NULL && EXP_Ttl_grace(req, exp_oc) >= req->t_req) {
+ /* we do not wait on the busy object if in grace */
assert(oh->refcnt > 1);
assert(exp_oc->objhead == oh);
exp_oc->refcnt++;
-
- if (!busy_found) {
- *bocp = hsh_insert_busyobj(wrk, oh);
- retval = HSH_EXPBUSY;
- } else {
- AZ(req->hash_ignore_busy);
- retval = HSH_EXP;
- }
- if (exp_oc->hits < LONG_MAX)
- exp_oc->hits++;
Lck_Unlock(&oh->mtx);
- if (retval == HSH_EXP)
- assert(HSH_DerefObjHead(wrk, &oh));
*ocp = exp_oc;
- return (retval);
- }
- if (!busy_found) {
- /* Insert objcore in objecthead and release mutex */
- *bocp = hsh_insert_busyobj(wrk, oh);
- /* NB: no deref of objhead, new object inherits reference */
- Lck_Unlock(&oh->mtx);
- return (HSH_MISS);
+ assert(HSH_DerefObjHead(wrk, &oh));
+ if (exp_oc->hits < LONG_MAX)
+ exp_oc->hits++;
+ return (HSH_EXP);
}
/* There are one or more busy objects, wait for them */
More information about the varnish-commit
mailing list