Skip to content

Commit

Permalink
Optimize 304 template object copying
Browse files Browse the repository at this point in the history
When copying a stale object after a 304 revalidation, we iterated over
it and allocated new storage for each storage segment.

So, at best, we kept the fragmentation of the existing object, or made
it even worse. This is particularly relevant when the existing object
was created from a chunked response, in which case the original
segments might have been particularly small and, consequently, many in
number.

Because we wait for the stale object to complete, we know the total
length (OA_LEN) upfront, and can ask the storage serving the copy for
exactly the right length. This is much more efficient, as
fragmentation is lowered and the storage engine might make a much
better allocation decision when it knows the full length rather than
getting piecemeal requests.

We implement the improved allocation scheme through additional state
kept for the duration of the template object copy: struct
vbf_objiter_priv holds the pointer to the newly created busy object,
the total yet unallocated length (initialized to OA_LEN of the existing
object) and pointer/length to the unused portion of the currently open
segment, if any.
  • Loading branch information
nigoroll committed Dec 4, 2023
1 parent 1dc285a commit d7e0b26
Showing 1 changed file with 42 additions and 10 deletions.
52 changes: 42 additions & 10 deletions bin/varnishd/cache/cache_fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -761,28 +761,56 @@ vbf_stp_fetchend(struct worker *wrk, struct busyobj *bo)
/*--------------------------------------------------------------------
*/

struct vbf_objiter_priv {
unsigned magic;
#define VBF_OBITER_PRIV_MAGIC 0x3c272a17
struct busyobj *bo;
// not yet allocated
ssize_t l;
// current allocation
uint8_t *p;
ssize_t pl;
};

static int v_matchproto_(objiterate_f)
vbf_objiterate(void *priv, unsigned flush, const void *ptr, ssize_t len)
{
struct busyobj *bo;
struct vbf_objiter_priv *vop;
ssize_t l;
const uint8_t *ps = ptr;
uint8_t *pd;

CAST_OBJ_NOTNULL(bo, priv, BUSYOBJ_MAGIC);
CAST_OBJ_NOTNULL(vop, priv, VBF_OBITER_PRIV_MAGIC);
CHECK_OBJ_NOTNULL(vop->bo, BUSYOBJ_MAGIC);

flush &= OBJ_ITER_FLUSH;

while (len > 0) {
l = len;
if (VFP_GetStorage(bo->vfc, &l, &pd) != VFP_OK)
return (1);
l = vmin(l, len);
memcpy(pd, ps, l);
VFP_Extend(bo->vfc, l, flush && l == len ? VFP_END : VFP_OK);
if (vop->pl == 0) {
vop->p = NULL;
AN(vop->l);
vop->pl = vop->l;
if (VFP_GetStorage(vop->bo->vfc, &vop->pl, &vop->p)
!= VFP_OK)
return (1);
if (vop->pl < vop->l)
vop->l -= vop->pl;
else
vop->l = 0;
}
AN(vop->pl);
AN(vop->p);

l = vmin(vop->pl, len);
memcpy(vop->p, ps, l);
VFP_Extend(vop->bo->vfc, l,
flush && l == len ? VFP_END : VFP_OK);
ps += l;
vop->p += l;
len -= l;
vop->pl -= l;
}
if (flush)
AZ(vop->l);
return (0);
}

Expand All @@ -792,6 +820,7 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
struct boc *stale_boc;
enum boc_state_e stale_state;
struct objcore *oc, *stale_oc;
struct vbf_objiter_priv vop[1];

CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
Expand Down Expand Up @@ -851,7 +880,10 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
ObjSetState(wrk, oc, BOS_STREAM);
}

if (ObjIterate(wrk, stale_oc, bo, vbf_objiterate, 0))
INIT_OBJ(vop, VBF_OBITER_PRIV_MAGIC);
vop->bo = bo;
vop->l = ObjGetLen(bo->wrk, stale_oc);
if (ObjIterate(wrk, stale_oc, vop, vbf_objiterate, 0))
(void)VFP_Error(bo->vfc, "Template object failed");

if (bo->vfc->failed) {
Expand Down

0 comments on commit d7e0b26

Please sign in to comment.