Optimizations made possible with transport_hsh_cancel in Varnish-Cache

these optimizations are possible if Varnish-Cache accepts the
transport_hsh_cancel PR:

If the transport gets to decide when to call HSH_Cancel(), we can keep
references also to private leaf objects.

For ESI objects, which we also need to inspect when building the
delivery tree, we still need to make copies in pesi_buf_bytes because,
if the final flag to ObjIterate() is set, the storage engine can (and
usually will) free segments "behind" delivery.

src/node.c

@@ -735,8 +735,6 @@ push_subreq(struct req *req, struct bytes_tree *tree,
 
 	subreq = subreq_fixup(node, req);
 
-	AZ(subreq->objcore->flags & OC_F_FINAL);
-
 	/*
 	 * XXX how should we handle a failed subreq?
 	 * https://github.com/varnishcache/varnish-cache/issues/3264

src/pesi_flags.h

-extern int block_final, front_push;
+extern int front_push;
 
 #define PF_HAS_TASK		1U
 /* vcl-controlled flags */
 #define PF_CFG_SERIAL		(1U<<1)
 #define PF_CFG_THREAD		(1U<<2)
 /* undocumented for now */
-#define PF_CFG_BLOCK_FINAL	(1U<<3)
 #define PF_CFG_FRONT_PUSH	(1U<<4)
 
 #define PF_CFG_DEFAULT					\
 	( PF_CFG_THREAD				\
-	| (block_final ? PF_CFG_BLOCK_FINAL : 0)	\
 	| (front_push ? PF_CFG_FRONT_PUSH : 0)		\
 	)
 
 #define PF_MASK_CFG		\
 	( PF_CFG_SERIAL	\
 	| PF_CFG_THREAD	\
-	| PF_CFG_BLOCK_FINAL	\
 	| PF_CFG_FRONT_PUSH	\
 	)

src/vdp_pesi.c

@@ -60,16 +60,6 @@
 
 #include "misc.h"
 
-/*lint -esym(843, block_final, front_push)
- * we want to keep the option to tune these at runtime in a debugger
- */
-
-/*
- * whether to use blocking threads for final (private/hfm/fpm) objects
- *
- * if false, buffering will be used
- */
-int block_final = 0;
 /*
  * whether to push bytes early
  *
@@ -222,14 +212,6 @@ vped_task(struct worker *wrk, void *priv)
 	VCL_Rel(&req->vcl);
 
 	switch (node->type) {
-	case T_FINAL:
-		assert(pesi == req->transport_priv);
-		req->transport_priv = NULL;
-		pesi_destroy(&pesi);
-		req_fini(&req, wrk);
-		AZ(pesi);
-		task_fini(pesi_tasks, pesi);
-		break;
 	case T_SUBREQ:
 		assert(pesi == req->transport_priv);
 		Lck_Lock(node->subreq.shared_lock);
@@ -428,27 +410,6 @@ vped_include(struct req *preq, const char *src, const char *host,
 
 	VSLdbgv(preq, "vped_include: attempt new thread req=%p", req);
 
-	if (pesi->flags & PF_CFG_BLOCK_FINAL) {
-		/* XXX because of T_FINAL blocking in the subreq thread, these
-		 * threads *have* to run - which is why PF_CFG_BLOCK_FINAL is
-		 * not available from VCL
-		 *
-		 * ordering should ensure that we run top->bottom left-right in
-		 * the ESI tree, so delivery should be able to let T_FINAL
-		 * threads run, and anything but T_FINAL should terminate
-		 * eventually, but if we spend all available threads on T_FINAL,
-		 * we're doomed.
-		 *
-		 * -- find a way to reserve or check for T_FINAL
-		 *
-		 * for now, force the other flags:
-		 * - serial mode off
-		 * - thread mode on
-		 */
-		pesi->flags &= ~PF_CFG_SERIAL;
-		pesi->flags |=  PF_CFG_THREAD;
-	}
-
 	if (pesi->flags & PF_CFG_SERIAL) {
 		vped_task(wrk, req);
 		return (0);
@@ -482,7 +443,7 @@ vped_include(struct req *preq, const char *src, const char *host,
 
 /* ------------------------------------------------------------
  * buffer/reference vdp bytes pushed from the pesi vdp
- * or from storage
+ * for literal segments inbetween includes
  */
 
 static int v_matchproto_(vdp_init_f)
@@ -557,18 +518,15 @@ pesi_buf_bytes(struct vdp_ctx *vdx, enum vdp_action act, void **priv,
 	 * non-api knowledge from VDP_DeliverObj()
 	 *  final = req->objcore->flags & (OC_F_PRIVATE | OC_F_HFM | OC_F_HFP);
 	 *
-	 * if data is coming from a varnish-cache simple storage, we know that
-	 * - for non-final objects
-	 * - as long as we keep a reference
+	 * as long as we keep a reference on a non-final object, it will
+	 * not go away, so we can keep a reference only and avoid copying
 	 *
-	 * the storage will not go away, so any vector we get pushed will remain
-	 * valid. We only do this check once
+	 * XXX TODO: Add a way in varnish-cache to veto the final flag to
+	 * ObjIterate() ?
 	 *
 	 */
-	if (VSTAILQ_FIRST(&parent->nexus.children) == NULL &&
-	    (req->objcore->flags & OC_F_FINAL) == 0 &&
-	    req->objcore->stobj->stevedore->methods == &SML_methods) {
-		AZ(parent->nexus.oc);
+	if (parent->nexus.oc == NULL &&
+	    (req->objcore->flags & OC_F_FINAL) == 0) {
 
 		parent->nexus.oc = req->objcore;
 		HSH_Ref(parent->nexus.oc);
@@ -583,6 +541,7 @@ pesi_buf_bytes(struct vdp_ctx *vdx, enum vdp_action act, void **priv,
 		node->data.ptr = ptr;
 	}
 	else {
+		AN(req->objcore->flags & OC_F_FINAL);
 		VSLdbg(vdx, "bytes_add: allocating storage buffer");
 		node->data.stvbuf = STV_AllocBuf(vdx->wrk, stv_transient, len);
 		if (node->data.stvbuf == NULL) {
@@ -1275,19 +1234,6 @@ vped_close_vdp(struct req *req, int skip, const struct vdp *vdp)
 	vdc->nxt = nxt;
 }
 
-static const struct vdp *
-vped_next_vdp(struct req *req)
-{
-	struct vdp_entry *vdpe;
-	struct vdp_ctx *vdc;
-
-	vdc = req->vdc;
-	vdpe = VTAILQ_FIRST(&vdc->vdp);
-
-	CHECK_OBJ_NOTNULL(vdpe, VDP_ENTRY_MAGIC);
-	return (vdpe->vdp);
-}
-
 static void
 push_vdps(VRT_CTX, struct req *req, struct vped_gzgz_priv *vgzgz,
     struct nexus_gzip *gz)
@@ -1310,14 +1256,13 @@ push_vdps(VRT_CTX, struct req *req, struct vped_gzgz_priv *vgzgz,
 static void v_matchproto_(vtr_deliver_f)
 vped_deliver(struct req *req, struct boc *boc, int wantbody)
 {
-	int obj_gzipped, obj_esi, obj_final;
+	int obj_gzipped, obj_esi;
 	struct pesi *pesi;
 	struct bytes_tree *tree;
 	struct node *node, *parent;
 	struct nexus_gzip *gz;
 	struct vped_gzgz_priv *vgzgz = NULL;
 	struct vrt_ctx ctx[1];
-	const struct vdp *vdp;
 
 	VSLdbgv(req, "vped_deliver: req=%p boc=%p wantbody=%d", req, boc,
 		wantbody);
@@ -1340,6 +1285,7 @@ vped_deliver(struct req *req, struct boc *boc, int wantbody)
 
 	if (wantbody == 0) {
 		(void) VDP_Close(req->vdc);
+		HSH_Cancel(req->wrk, req->objcore, boc);
 		return;
 	}
 
@@ -1347,12 +1293,12 @@ vped_deliver(struct req *req, struct boc *boc, int wantbody)
 		ObjGetLen(req->wrk, req->objcore));
 	if (boc == NULL && ObjGetLen(req->wrk, req->objcore) == 0) {
 		(void) VDP_Close(req->vdc);
+		HSH_Cancel(req->wrk, req->objcore, boc);
 		return;
 	}
 
 	obj_gzipped = ObjCheckFlag(req->wrk, req->objcore, OF_GZIPED);
 	obj_esi = ObjHasAttr(req->wrk, req->objcore, OA_ESIDATA);
-	obj_final = (req->objcore->flags & OC_F_FINAL) != 0;
 
 	gz = &parent->nexus.gzip;
 
@@ -1368,6 +1314,7 @@ vped_deliver(struct req *req, struct boc *boc, int wantbody)
 			/* No way of signalling errors in the middle of
 			   the ESI body. Omit this ESI fragment. */
 			(void) VDP_Close(req->vdc);
+			HSH_Cancel(req->wrk, req->objcore, boc);
 			return;
 		}
 
@@ -1376,6 +1323,7 @@ vped_deliver(struct req *req, struct boc *boc, int wantbody)
 			VSLb(req->vsl, SLT_Error,
 			     "Insufficient workspace for ESI gzip data");
 			(void) VDP_Close(req->vdc);
+			HSH_Cancel(req->wrk, req->objcore, boc);
 			return;
 		}
 		INIT_OBJ(vgzgz, VPED_GZGZ_PRIV_MAGIC);
@@ -1412,72 +1360,8 @@ vped_deliver(struct req *req, struct boc *boc, int wantbody)
 		vped_close_vdp(req, 0, &VDP_pesi_buf);
 		return;
 	}
-	else if (obj_final && (pesi->flags & PF_CFG_BLOCK_FINAL) == 0) {
-		VSLdbg(req, "vped_deliver: T_NEXUS buffering no ESI");
-
-		AZ(VDP_Push(ctx, req->vdc, req->ws, &VDP_pesi_buf, pesi));
-		AN(parent);
-		assert(parent->type == T_NEXUS);
-		push_vdps(ctx, req, vgzgz, gz);
-		AZ(VDP_Push(ctx, req->vdc, req->ws,
-		    &vped_to_parent, parent->nexus.req));
-
-		(void)VDP_DeliverObj(req->vdc, req->objcore);
-
-		/* pesi_buf does not pesi_destroy() on fini */
-		req->transport_priv = NULL;
-		pesi_destroy(&pesi);
-		vdp = vped_next_vdp(req);
-		while (vdp != &VDP_pesi_buf) {
-			vped_close_vdp(req, 0, vdp);
-			vdp = vped_next_vdp(req);
-		}
-		vped_close_vdp(req, 0, &VDP_pesi_buf);
-		return;
-	}
-	else if (obj_final) {
-		VSLdbg(req, "vped_deliver: T_FINAL");
-
-		push_vdps(ctx, req, vgzgz, gz);
-		AZ(VDP_Push(ctx, req->vdc, req->ws,
-		    &vped_to_parent, parent->nexus.req));
-
-		node_mutate_prep(tree, node);
-
-		node->final.shared_lock = &tree->nodes_lock;
-		AZ(pthread_cond_init(&node->final.fi_cond, NULL));
-		assert(node->final.fi_state == FI_READY);
-
-		node_mutate_lock(tree, node, T_FINAL, ST_DATA);
-
-		/*
-		 * we deliberately take the nodes lock under the tree mutex (via
-		 * node_mutate_lock) to ensure unpending never gets unlocked
-		 * access to ensure we get the signal
-		 */
-		Lck_Lock(node->final.shared_lock);
-
-		node_mutate_unlock(tree);
-
-		if (node->final.fi_state < FI_GO)
-			AZ(Lck_CondWait(&node->final.fi_cond,
-					node->final.shared_lock));
-
-		assert(node->final.fi_state == FI_GO);
-
-		(void)VDP_DeliverObj(req->vdc, req->objcore);
-
-		req->acct.resp_bodybytes += VDP_Close(req->vdc);
-
-		node->final.fi_state = FI_DONE;
-		AZ(pthread_cond_signal(&node->final.fi_cond));
-		Lck_Unlock(node->final.shared_lock);
-
-		return;
-	}
 	VSLdbg(req, "vped_deliver: T_SUBREQ");
 
-	AZ(req->objcore->flags & OC_F_FINAL);
 	push_vdps(ctx, req, vgzgz, gz);
 	AZ(VDP_Push(ctx, req->vdc, req->ws,
 	    &vped_to_parent, parent->nexus.req));