Optimizations made possible with HSH_Cancel() in VDP_Close()

Now 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/misc.c

@@ -32,6 +32,7 @@
 #include "config.h"
 
 #include "cache/cache_varnishd.h"
+#include "cache/cache_filter.h"
 
 #include "misc.h"
 #include "debug.h"
@@ -49,7 +50,7 @@ req_fini(struct req **reqp, struct worker *wrk)
 
 	VSLdbg(req, "req_fini called");
 
-	req->acct.resp_bodybytes += VDP_Close(req->vdc);
+	req->acct.resp_bodybytes += VDP_Close(req->vdc, req->objcore, NULL);
 	Req_Cleanup(req->sp, wrk, req);
 	Req_Release(req);
 }

src/node.c

@@ -644,10 +644,7 @@ fini_subreq(const struct vdp_ctx *vdx, struct node *node)
 	AZ(node->subreq.oc);
 
 	// need before pesi_destroy
-	(void) VDP_Close(subreq->vdc);
-
-	/* bottom of cnt_transmit() */
-	HSH_Cancel(vdx->wrk, subreq->objcore, boc);
+	(void) VDP_Close(subreq->vdc, subreq->objcore, boc);
 
 	if (boc != NULL)
 		HSH_DerefBoc(vdx->wrk, subreq->objcore);
@@ -735,14 +732,13 @@ 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
 	 */
 	(void) VDP_DeliverObj(subreq->vdc, subreq->objcore);
-	subreq->acct.resp_bodybytes += VDP_Close(subreq->vdc);
+	subreq->acct.resp_bodybytes +=
+		VDP_Close(subreq->vdc, subreq->objcore, NULL);
 
 	// fini_subreq() happening later
 
@@ -918,7 +914,8 @@ worklist_push(struct vdp_ctx *vdx, struct bytes_tree *tree,
 				assert(next == NULL);
 			else
 				parent->nexus.req->acct.resp_bodybytes +=
-					VDP_Close(parent->nexus.req->vdc);
+					VDP_Close(parent->nexus.req->vdc,
+					    parent->nexus.oc, NULL);
 		}
 		node = next;
 	}

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);
@@ -272,7 +254,7 @@ vped_task(struct worker *wrk, void *priv)
 		 */
 
 		// finish the request
-		(void) VDP_Close(req->vdc);
+		(void) VDP_Close(req->vdc, req->objcore, NULL);
 
 		if (req->transport_priv != NULL)
 			assert(req->transport_priv == pesi);
@@ -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);
@@ -1339,20 +1284,19 @@ vped_deliver(struct req *req, struct boc *boc, int wantbody)
 	assert(parent->type == T_NEXUS);
 
 	if (wantbody == 0) {
-		(void) VDP_Close(req->vdc);
+		(void) VDP_Close(req->vdc, req->objcore, boc);
 		return;
 	}
 
 	VSLdbgv(req, "vped_deliver: ObjGetLen=%lu",
 		ObjGetLen(req->wrk, req->objcore));
 	if (boc == NULL && ObjGetLen(req->wrk, req->objcore) == 0) {
-		(void) VDP_Close(req->vdc);
+		(void) VDP_Close(req->vdc, 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;
 
@@ -1367,7 +1311,7 @@ vped_deliver(struct req *req, struct boc *boc, int wantbody)
 		if (req->objcore->flags & OC_F_FAILED) {
 			/* No way of signalling errors in the middle of
 			   the ESI body. Omit this ESI fragment. */
-			(void) VDP_Close(req->vdc);
+			(void) VDP_Close(req->vdc, req->objcore, boc);
 			return;
 		}
 
@@ -1375,7 +1319,7 @@ vped_deliver(struct req *req, struct boc *boc, int wantbody)
 		if (vgzgz == NULL) {
 			VSLb(req->vsl, SLT_Error,
 			     "Insufficient workspace for ESI gzip data");
-			(void) VDP_Close(req->vdc);
+			(void) VDP_Close(req->vdc, req->objcore, boc);
 			return;
 		}
 		INIT_OBJ(vgzgz, VPED_GZGZ_PRIV_MAGIC);
@@ -1412,72 +1356,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));