[master] 06cf60a02 Add VRT_priv_task_lookup()

[Nils Goroll]

commit 06cf60a020a81e49d001ee42e4d85f16580f3fe0
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Tue Jan 5 12:47:20 2021 +0100

    Add VRT_priv_task_lookup()
    
    VRT_priv_task() inserts a priv_task if it does not exist.
    
    In some cases, it is more efficient to only insert a priv_task under
    certain conditions. These require an API function to only return a
    priv_task if it exists. This is VRT_priv_task_lookup().
    
    A TODO comment has been added for a sensible performance optimization,
    avoiding one VRBT traversal for the VRT_priv_task() case. I would
    address this after this suggestion got accepted, if so.

diff --git a/bin/varnishd/cache/cache_vrt_priv.c b/bin/varnishd/cache/cache_vrt_priv.c
index 9987abe21..1685ec06e 100644
--- a/bin/varnishd/cache/cache_vrt_priv.c
+++ b/bin/varnishd/cache/cache_vrt_priv.c
@@ -116,19 +116,36 @@ vrt_priv_dyncmp(const struct vrt_priv *vp1, const struct vrt_priv *vp2)
 VRBT_GENERATE_STATIC(vrt_privs, vrt_priv, entry, vrt_priv_dyncmp)
 
 static struct vmod_priv *
-vrt_priv_dynamic(struct ws *ws, struct vrt_privs *privs, uintptr_t vmod_id)
+vrt_priv_dynamic_get(struct ws *ws, struct vrt_privs *privs, uintptr_t vmod_id)
 {
 	struct vrt_priv *vp;
+
 	const struct vrt_priv needle = {.vmod_id = vmod_id};
 
+	vp = VRBT_FIND(vrt_privs, privs, &needle);
+	if (vp == NULL)
+		return (NULL);
+
+	CHECK_OBJ(vp, VRT_PRIV_MAGIC);
+	assert(vp->vmod_id == vmod_id);
+	return (vp->priv);
+}
+
+static struct vmod_priv *
+vrt_priv_dynamic(struct ws *ws, struct vrt_privs *privs, uintptr_t vmod_id)
+{
+	struct vrt_priv *vp;
+	static struct vmod_priv *r;
+
 	AN(vmod_id);
 
-	vp = VRBT_FIND(vrt_privs, privs, &needle);
-	if (vp) {
-		CHECK_OBJ(vp, VRT_PRIV_MAGIC);
-		assert(vp->vmod_id == vmod_id);
-		return (vp->priv);
-	}
+	/*
+	 * TODO: for insert == 1, we can avoid the VRBT_FIND:
+	 * call only VRT_INSERT and reset the ws if the element existed
+	 */
+	r = vrt_priv_dynamic_get(ws, privs, vmod_id);
+	if (r)
+		return (r);
 
 	vp = WS_Alloc(ws, sizeof *vp);
 	if (vp == NULL)
@@ -162,6 +179,17 @@ vrt_priv_task_context(VRT_CTX)
 	return (cli_task_privs);
 }
 
+struct vmod_priv *
+VRT_priv_task_get(VRT_CTX, const void *vmod_id)
+{
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+
+	return (vrt_priv_dynamic_get(
+	    ctx->ws,
+	    vrt_priv_task_context(ctx),
+	    (uintptr_t)vmod_id));
+}
+
 struct vmod_priv *
 VRT_priv_task(VRT_CTX, const void *vmod_id)
 {
diff --git a/bin/varnishtest/tests/m00000.vtc b/bin/varnishtest/tests/m00000.vtc
index 48fc749c0..49c9cc234 100644
--- a/bin/varnishtest/tests/m00000.vtc
+++ b/bin/varnishtest/tests/m00000.vtc
@@ -47,6 +47,7 @@ varnish v1 -vcl+backend {
 	}
 
 	sub priv_task {
+		debug.test_priv_task_get();
 		debug.test_priv_task("foo");
 	}
 
diff --git a/include/vrt.h b/include/vrt.h
index fe888060e..23de2361b 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -59,6 +59,7 @@
  *	struct vmod_priv free member replaced with methods
  *	VRT_CTX_Assert() added
  *	VRT_ban_string() signature changed
+ *	VRT_priv_task_get() added
  * 12.0 (2020-09-15)
  *	Added VRT_DirectorResolve()
  *	Added VCL_STRING VRT_BLOB_string(VRT_CTX, VCL_BLOB)
@@ -597,6 +598,7 @@ struct vmod_priv {
 
 void VRT_priv_fini(const struct vmod_priv *p);
 struct vmod_priv *VRT_priv_task(VRT_CTX, const void *vmod_id);
+struct vmod_priv *VRT_priv_task_get(VRT_CTX, const void *vmod_id);
 struct vmod_priv *VRT_priv_top(VRT_CTX, const void *vmod_id);
 
 /* Stevedore related functions */
diff --git a/vmod/vmod_debug.c b/vmod/vmod_debug.c
index 2288c4ff1..bb663ea94 100644
--- a/vmod/vmod_debug.c
+++ b/vmod/vmod_debug.c
@@ -243,6 +243,14 @@ xyzzy_test_priv_call(VRT_CTX, struct vmod_priv *priv)
 	}
 }
 
+VCL_VOID v_matchproto_(td_debug_test_priv_task_get)
+xyzzy_test_priv_task_get(VRT_CTX)
+{
+
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+	AZ(VRT_priv_task_get(ctx, NULL));
+}
+
 static void
 priv_task_free(void *ptr)
 {
@@ -848,7 +856,7 @@ xyzzy_priv_perf(VRT_CTX, VCL_INT size, VCL_INT rounds)
 	t0 = VTIM_mono();
 	for (r = 0; r < rounds; r++) {
 		for (s = 1; s <= size; s++) {
-			p = VRT_priv_task(ctx, (void *)(uintptr_t)s);
+			p = VRT_priv_task_get(ctx, (void *)(uintptr_t)s);
 			AN(p);
 			check += (uintptr_t)p->priv;
 			p->priv = (void *)(uintptr_t)(s * rounds + r);
@@ -1035,7 +1043,7 @@ xyzzy_get_ip(VRT_CTX)
 
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 
-	priv = VRT_priv_task(ctx, &store_ip_token);
+	priv = VRT_priv_task_get(ctx, &store_ip_token);
 	AN(priv);
 	AZ(priv->methods);
 
diff --git a/vmod/vmod_debug.vcc b/vmod/vmod_debug.vcc
index 4eee74547..fa81c8cf8 100644
--- a/vmod/vmod_debug.vcc
+++ b/vmod/vmod_debug.vcc
@@ -56,6 +56,10 @@ $Function VOID test_priv_vcl(PRIV_VCL)
 
 Test function for VCL private pointers
 
+$Function VOID test_priv_task_get()
+
+Assert that the priv_task for the NULL pointer is NULL.
+
 $Function STRING test_priv_task(PRIV_TASK, STRING s="")
 
 Test function for TASK private pointers