Deoptimize the central object matching loop in the hash code:

With the advent of prefetch and degraded mode, the invariants of
objectheads change so that more than one object can be busy at any
one time.

Thus we can no longer assume that the busy object or one subsequent to
it, is the one we eventually desire, and we must start our search from
the front of the list again.

As an amusing sidenote: this eliminates the only "goto" in all of varnishd.

git-svn-id: svn+ssh://projects.linpro.no/svn/varnish/trunk@2388 d4fa192b-c00b-0410-8231-f00ffab90ce4

diff --git a/varnish-cache/bin/varnishd/cache_hash.c b/varnish-cache/bin/varnishd/cache_hash.c
index 644f44147354d73b86c222031db14bb25590c2e9..9135712759dca46415d94c18d5e6982a680cd649 100644 (file)
--- a/varnish-cache/bin/varnishd/cache_hash.c
+++ b/varnish-cache/bin/varnishd/cache_hash.c
@@ -185,44 +185,44 @@ HSH_Lookup(struct sess *sp)
        HSH_Prealloc(sp);
        if (sp->obj != NULL) {
                CHECK_OBJ_NOTNULL(sp->obj, OBJECT_MAGIC);
-               o = sp->obj;
-               oh = o->objhead;
+               oh = sp->obj->objhead;
+               HSH_Deref(sp->obj);
                CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
                LOCK(&oh->mtx);
-               goto were_back;
+       } else {
+               oh = hash->lookup(sp, w->nobjhead);
+               CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
+               if (oh == w->nobjhead)
+                       w->nobjhead = NULL;
+               LOCK(&oh->mtx);
        }
 
-       oh = hash->lookup(sp, w->nobjhead);
-       CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
-       if (oh == w->nobjhead)
-               w->nobjhead = NULL;
-       LOCK(&oh->mtx);
        VTAILQ_FOREACH(o, &oh->objects, list) {
-               o->refcnt++;
                if (o->busy) {
                        VTAILQ_INSERT_TAIL(&o->waitinglist, sp, list);
                        sp->obj = o;
+                       o->refcnt++;
                        UNLOCK(&oh->mtx);
                        return (NULL);
                }
-       were_back:
-               if (!o->cacheable) {
-                       /* ignore */
-               } else if (o->ttl == 0) {
-                       /* Object banned but not reaped yet */
-               } else if (o->ttl <= sp->t_req) {
-                       /* Object expired */
-               } else if (BAN_CheckObject(o,
-                   h->hd[HTTP_HDR_URL].b, oh->hash)) {
+               if (!o->cacheable)
+                       continue;
+               if (o->ttl == 0) 
+                       continue;
+               if (o->ttl <= sp->t_req) 
+                       continue;
+               if (BAN_CheckObject(o, h->hd[HTTP_HDR_URL].b, oh->hash)) {
                        o->ttl = 0;
                        WSP(sp, SLT_ExpBan, "%u was banned", o->xid);
                        if (o->timer_idx != 0)
                                EXP_TTLchange(o);
-               } else if (o->vary == NULL || VRY_Match(sp, o->vary))
+                       continue;
+               }
+               if (o->vary == NULL || VRY_Match(sp, o->vary))
                        break;
-               o->refcnt--;
        }
        if (o != NULL) {
+               o->refcnt++;
                UNLOCK(&oh->mtx);
                (void)hash->deref(oh);
                return (o);