Skip to content

Commit

Permalink
hash: Retire the HSH_DerefObjCore(rushmax) argument
Browse files Browse the repository at this point in the history
Now that waiting lists are owned by objcores, the rush policy no longer
needs to leak everywhere. Either all the waiters are rushed at once for
cacheable objects, or according to rush_exponent.
  • Loading branch information
dridi committed Oct 3, 2023
1 parent 0e005de commit 05d9052
Show file tree
Hide file tree
Showing 12 changed files with 43 additions and 54 deletions.
2 changes: 1 addition & 1 deletion bin/varnishd/cache/cache_ban_lurker.c
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ ban_lurker_test_ban(struct worker *wrk, struct vsl_log *vsl, struct ban *bt,
if (i)
ObjSendEvent(wrk, oc, OEV_BANCHG);
}
(void)HSH_DerefObjCore(wrk, &oc, 0);
(void)HSH_DerefObjCore(wrk, &oc);
}
}

Expand Down
6 changes: 2 additions & 4 deletions bin/varnishd/cache/cache_busyobj.c
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,8 @@ VBO_ReleaseBusyObj(struct worker *wrk, struct busyobj **pbo)
if (WS_Overflowed(bo->ws))
wrk->stats->ws_backend_overflow++;

if (bo->fetch_objcore != NULL) {
(void)HSH_DerefObjCore(wrk, &bo->fetch_objcore,
HSH_RUSH_POLICY);
}
if (bo->fetch_objcore != NULL)
(void)HSH_DerefObjCore(wrk, &bo->fetch_objcore);

VRT_Assign_Backend(&bo->director_req, NULL);
VRT_Assign_Backend(&bo->director_resp, NULL);
Expand Down
6 changes: 3 additions & 3 deletions bin/varnishd/cache/cache_expire.c
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ EXP_Insert(struct worker *wrk, struct objcore *oc)
ObjSendEvent(wrk, oc, OEV_EXPIRE);
tmpoc = oc;
assert(oc->refcnt >= 2); /* Silence coverity */
(void)HSH_DerefObjCore(wrk, &oc, 0);
(void)HSH_DerefObjCore(wrk, &oc);
AZ(oc);
assert(tmpoc->refcnt >= 1); /* Silence coverity */
}
Expand Down Expand Up @@ -280,7 +280,7 @@ exp_inbox(struct exp_priv *ep, struct objcore *oc, unsigned flags)
assert(oc->refcnt > 0);
AZ(oc->exp_flags);
ObjSendEvent(ep->wrk, oc, OEV_EXPIRE);
(void)HSH_DerefObjCore(ep->wrk, &oc, 0);
(void)HSH_DerefObjCore(ep->wrk, &oc);
return;
}

Expand Down Expand Up @@ -357,7 +357,7 @@ exp_expire(struct exp_priv *ep, vtim_real now)
VSLb(&ep->vsl, SLT_ExpKill, "EXP_Expired xid=%ju t=%.0f",
VXID(ObjGetXID(ep->wrk, oc)), EXP_Ttl(NULL, oc) - now);
ObjSendEvent(ep->wrk, oc, OEV_EXPIRE);
(void)HSH_DerefObjCore(ep->wrk, &oc, 0);
(void)HSH_DerefObjCore(ep->wrk, &oc);
}
return (0);
}
Expand Down
10 changes: 5 additions & 5 deletions bin/varnishd/cache/cache_fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,7 @@ vbf_fetch_thread(struct worker *wrk, void *priv)
CHECK_OBJ_NOTNULL(bo->stale_oc, OBJCORE_MAGIC);
/* We don't want the oc/stevedore ops in fetching thread */
if (!ObjCheckFlag(wrk, bo->stale_oc, OF_IMSCAND))
(void)HSH_DerefObjCore(wrk, &bo->stale_oc, 0);
(void)HSH_DerefObjCore(wrk, &bo->stale_oc);
}
#endif

Expand All @@ -1085,7 +1085,7 @@ vbf_fetch_thread(struct worker *wrk, void *priv)
http_Teardown(bo->beresp);
// can not make assumptions about the number of references here #3434
if (bo->bereq_body != NULL)
(void) HSH_DerefObjCore(bo->wrk, &bo->bereq_body, 0);
(void) HSH_DerefObjCore(bo->wrk, &bo->bereq_body);

if (oc->boc->state == BOS_FINISHED) {
AZ(oc->flags & OC_F_FAILED);
Expand All @@ -1095,7 +1095,7 @@ vbf_fetch_thread(struct worker *wrk, void *priv)
// AZ(oc->boc); // XXX

if (bo->stale_oc != NULL)
(void)HSH_DerefObjCore(wrk, &bo->stale_oc, 0);
(void)HSH_DerefObjCore(wrk, &bo->stale_oc);

wrk->vsl = NULL;
HSH_DerefBoc(wrk, oc);
Expand Down Expand Up @@ -1184,7 +1184,7 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc,
wrk->stats->bgfetch_no_thread++;
(void)vbf_stp_fail(req->wrk, bo);
if (bo->stale_oc != NULL)
(void)HSH_DerefObjCore(wrk, &bo->stale_oc, 0);
(void)HSH_DerefObjCore(wrk, &bo->stale_oc);
HSH_DerefBoc(wrk, oc);
SES_Rel(bo->sp);
THR_SetBusyobj(NULL);
Expand All @@ -1209,5 +1209,5 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc,
assert(oc->boc == boc);
HSH_DerefBoc(wrk, oc);
if (mode == VBF_BACKGROUND)
(void)HSH_DerefObjCore(wrk, &oc, HSH_RUSH_POLICY);
(void)HSH_DerefObjCore(wrk, &oc);
}
30 changes: 11 additions & 19 deletions bin/varnishd/cache/cache_hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ struct rush {
static const struct hash_slinger *hash;
static struct objhead *private_oh;

static void hsh_rush1(const struct worker *, struct objcore *,
struct rush *, int);
static void hsh_rush1(const struct worker *, struct objcore *, struct rush *);
static void hsh_rush2(struct worker *, struct rush *);
static int hsh_deref_objhead(struct worker *wrk, struct objhead **poh);
static int hsh_deref_objhead_unlock(struct worker *wrk, struct objhead **poh);
Expand Down Expand Up @@ -649,7 +648,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp)
CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
AZ(oc->flags);
hsh_rush_move(oc, rush_oc);
hsh_rush1(wrk, oc, &rush, HSH_RUSH_POLICY);
hsh_rush1(wrk, oc, &rush);
assert(VTAILQ_EMPTY(&oc->waitinglist));
break;
case HSH_BUSY:
Expand Down Expand Up @@ -763,7 +762,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp)

if (rush_oc != NULL) {
hsh_rush2(wrk, &rush);
(void)HSH_DerefObjCore(wrk, &rush_oc, HSH_RUSH_POLICY);
(void)HSH_DerefObjCore(wrk, &rush_oc);
}

return (lr);
Expand All @@ -774,21 +773,14 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp)
*/

static void
hsh_rush1(const struct worker *wrk, struct objcore *oc, struct rush *r, int max)
hsh_rush1(const struct worker *wrk, struct objcore *oc, struct rush *r)
{
int i;
int max, i;
unsigned xid = 0;
struct req *req;

if (max == 0)
return;
if (max == HSH_RUSH_POLICY) {
AZ(oc->flags & OC_F_BUSY);
if (oc->flags != 0)
max = cache_param->rush_exponent;
else
max = INT_MAX;
}
AZ(oc->flags & OC_F_BUSY);
max = oc->flags != 0 ? cache_param->rush_exponent : INT_MAX;
assert(max > 0);

CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
Expand Down Expand Up @@ -927,7 +919,7 @@ HSH_Purge(struct worker *wrk, struct objhead *oh, vtim_real ttl_now,
for (i = 0; i < j; i++) {
CHECK_OBJ_NOTNULL(ocp[i], OBJCORE_MAGIC);
EXP_Rearm(ocp[i], ttl_now, ttl, grace, keep);
(void)HSH_DerefObjCore(wrk, &ocp[i], 0);
(void)HSH_DerefObjCore(wrk, &ocp[i]);
AZ(ocp[i]);
total++;
}
Expand Down Expand Up @@ -1073,7 +1065,7 @@ HSH_Unbusy(struct worker *wrk, struct objcore *oc)
VTAILQ_REMOVE(&oh->objcs, oc, hsh_list);
VTAILQ_INSERT_HEAD(&oh->objcs, oc, hsh_list);
oc->flags &= ~OC_F_BUSY;
hsh_rush1(wrk, oc, &rush, HSH_RUSH_POLICY);
hsh_rush1(wrk, oc, &rush);
Lck_Unlock(&oh->mtx);
EXP_Insert(wrk, oc); /* Does nothing unless EXP_RefNewObjcore was
* called */
Expand Down Expand Up @@ -1204,7 +1196,7 @@ HSH_DerefBoc(struct worker *wrk, struct objcore *oc)
*/

int
HSH_DerefObjCore(struct worker *wrk, struct objcore **ocp, int rushmax)
HSH_DerefObjCore(struct worker *wrk, struct objcore **ocp)
{
struct objcore *oc;
struct objhead *oh;
Expand All @@ -1226,7 +1218,7 @@ HSH_DerefObjCore(struct worker *wrk, struct objcore **ocp, int rushmax)
VTAILQ_REMOVE(&oh->objcs, oc, hsh_list);
if (!VTAILQ_EMPTY(&oc->waitinglist)) {
assert(oc->refcnt > 0);
hsh_rush1(wrk, oc, &rush, rushmax);
hsh_rush1(wrk, oc, &rush);
}
Lck_Unlock(&oh->mtx);
hsh_rush2(wrk, &rush);
Expand Down
3 changes: 1 addition & 2 deletions bin/varnishd/cache/cache_objhead.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ struct boc *HSH_RefBoc(const struct objcore *);
void HSH_DerefBoc(struct worker *wrk, struct objcore *);
void HSH_DeleteObjHead(const struct worker *, struct objhead *);

int HSH_DerefObjCore(struct worker *, struct objcore **, int rushmax);
#define HSH_RUSH_POLICY -1
int HSH_DerefObjCore(struct worker *, struct objcore **);

enum lookup_e HSH_Lookup(struct req *, struct objcore **, struct objcore **);
void HSH_Ref(struct objcore *o);
Expand Down
10 changes: 5 additions & 5 deletions bin/varnishd/cache/cache_req_body.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
if (STV_NewObject(req->wrk, req->body_oc, stv, hint) == 0) {
req->req_body_status = BS_ERROR;
HSH_DerefBoc(req->wrk, req->body_oc);
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc));
(void)VFP_Error(vfc, "Object allocation failed:"
" Ran out of space in %s", stv->vclname);
return (-1);
Expand All @@ -95,7 +95,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
if (VFP_Open(ctx, vfc) < 0) {
req->req_body_status = BS_ERROR;
HSH_DerefBoc(req->wrk, req->body_oc);
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc));
return (-1);
}

Expand Down Expand Up @@ -141,7 +141,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)
VSLb_ts_req(req, "ReqBody", VTIM_real());
if (func != NULL) {
HSH_DerefBoc(req->wrk, req->body_oc);
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc));
if (vfps != VFP_END) {
req->req_body_status = BS_ERROR;
if (r == 0)
Expand All @@ -155,7 +155,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv)

if (vfps != VFP_END) {
req->req_body_status = BS_ERROR;
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0));
AZ(HSH_DerefObjCore(req->wrk, &req->body_oc));
return (-1);
}

Expand Down Expand Up @@ -276,7 +276,7 @@ VRB_Free(struct req *req)
if (req->body_oc == NULL)
return;

r = HSH_DerefObjCore(req->wrk, &req->body_oc, 0);
r = HSH_DerefObjCore(req->wrk, &req->body_oc);

// each busyobj may have gained a reference
assert (r >= 0);
Expand Down
22 changes: 11 additions & 11 deletions bin/varnishd/cache/cache_req_fsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ cnt_deliver(struct worker *wrk, struct req *req)
ObjTouch(req->wrk, req->objcore, req->t_prev);

if (Resp_Setup_Deliver(req)) {
(void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY);
(void)HSH_DerefObjCore(wrk, &req->objcore);
req->err_code = 500;
req->req_step = R_STP_SYNTH;
return (REQ_FSM_MORE);
Expand All @@ -239,7 +239,7 @@ cnt_deliver(struct worker *wrk, struct req *req)

if (wrk->vpi->handling != VCL_RET_DELIVER) {
HSH_Cancel(wrk, req->objcore, NULL);
(void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY);
(void)HSH_DerefObjCore(wrk, &req->objcore);
http_Teardown(req->resp);

switch (wrk->vpi->handling) {
Expand Down Expand Up @@ -400,7 +400,7 @@ cnt_synth(struct worker *wrk, struct req *req)
VSLb(req->vsl, SLT_Error, "Could not get storage");
req->doclose = SC_OVERLOAD;
VSLb_ts_req(req, "Resp", W_TIM_real(wrk));
(void)HSH_DerefObjCore(wrk, &req->objcore, 1);
(void)HSH_DerefObjCore(wrk, &req->objcore);
http_Teardown(req->resp);
return (REQ_FSM_DONE);
}
Expand Down Expand Up @@ -499,7 +499,7 @@ cnt_transmit(struct worker *wrk, struct req *req)
if (boc != NULL)
HSH_DerefBoc(wrk, req->objcore);

(void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY);
(void)HSH_DerefObjCore(wrk, &req->objcore);
http_Teardown(req->resp);

req->filter_list = NULL;
Expand All @@ -526,7 +526,7 @@ cnt_fetch(struct worker *wrk, struct req *req)
if (req->objcore->flags & OC_F_FAILED) {
req->err_code = 503;
req->req_step = R_STP_SYNTH;
(void)HSH_DerefObjCore(wrk, &req->objcore, 1);
(void)HSH_DerefObjCore(wrk, &req->objcore);
AZ(req->objcore);
return (REQ_FSM_MORE);
}
Expand Down Expand Up @@ -661,10 +661,10 @@ cnt_lookup(struct worker *wrk, struct req *req)
}

/* Drop our object, we won't need it */
(void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY);
(void)HSH_DerefObjCore(wrk, &req->objcore);

if (busy != NULL) {
(void)HSH_DerefObjCore(wrk, &busy, 0);
(void)HSH_DerefObjCore(wrk, &busy);
VRY_Clear(req);
}

Expand All @@ -691,7 +691,7 @@ cnt_miss(struct worker *wrk, struct req *req)
wrk->stats->cache_miss++;
VBF_Fetch(wrk, req, req->objcore, req->stale_oc, VBF_NORMAL);
if (req->stale_oc != NULL)
(void)HSH_DerefObjCore(wrk, &req->stale_oc, 0);
(void)HSH_DerefObjCore(wrk, &req->stale_oc);
req->req_step = R_STP_FETCH;
return (REQ_FSM_MORE);
case VCL_RET_FAIL:
Expand All @@ -711,8 +711,8 @@ cnt_miss(struct worker *wrk, struct req *req)
}
VRY_Clear(req);
if (req->stale_oc != NULL)
(void)HSH_DerefObjCore(wrk, &req->stale_oc, 0);
AZ(HSH_DerefObjCore(wrk, &req->objcore, 1));
(void)HSH_DerefObjCore(wrk, &req->stale_oc);
AZ(HSH_DerefObjCore(wrk, &req->objcore));
return (REQ_FSM_MORE);
}

Expand Down Expand Up @@ -1081,7 +1081,7 @@ cnt_purge(struct worker *wrk, struct req *req)

(void)HSH_Purge(wrk, boc->objhead, req->t_req, 0, 0, 0);

AZ(HSH_DerefObjCore(wrk, &boc, 1));
AZ(HSH_DerefObjCore(wrk, &boc));

VCL_purge_method(req->vcl, wrk, req, NULL, NULL);
switch (wrk->vpi->handling) {
Expand Down
2 changes: 1 addition & 1 deletion bin/varnishd/cache/cache_vrt_var.c
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ VRT_u_bereq_body(VRT_CTX)
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC);
if (ctx->bo->bereq_body != NULL) {
(void)HSH_DerefObjCore(ctx->bo->wrk, &ctx->bo->bereq_body, 0);
(void)HSH_DerefObjCore(ctx->bo->wrk, &ctx->bo->bereq_body);
http_Unset(ctx->bo->bereq, H_Content_Length);
}

Expand Down
2 changes: 1 addition & 1 deletion bin/varnishd/storage/storage_lru.c
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,6 @@ LRU_NukeOne(struct worker *wrk, struct lru *lru)
ObjSlim(wrk, oc);

VSLb(wrk->vsl, SLT_ExpKill, "LRU xid=%ju", VXID(ObjGetXID(wrk, oc)));
(void)HSH_DerefObjCore(wrk, &oc, 0); // Ref from HSH_Snipe
(void)HSH_DerefObjCore(wrk, &oc); // Ref from HSH_Snipe
return (1);
}
2 changes: 1 addition & 1 deletion bin/varnishd/storage/storage_persistent_silo.c
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ smp_load_seg(struct worker *wrk, const struct smp_sc *sc,
HSH_Insert(wrk, so->hash, oc, ban);
AN(oc->ban);
HSH_DerefBoc(wrk, oc); // XXX Keep it an stream resurrection?
(void)HSH_DerefObjCore(wrk, &oc, HSH_RUSH_POLICY);
(void)HSH_DerefObjCore(wrk, &oc);
wrk->stats->n_vampireobject++;
}
Pool_Sumstat(wrk);
Expand Down
2 changes: 1 addition & 1 deletion include/tbl/params.h
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ PARAM_SIMPLE(
/* descr */
"How many parked request we start for each completed request on "
"the object.\n"
"NB: Even with the implict delay of delivery, this parameter "
"NB: Even with the implicit delay of delivery, this parameter "
"controls an exponential increase in number of worker threads.",
/* flags */ EXPERIMENTAL
)
Expand Down

0 comments on commit 05d9052

Please sign in to comment.