[6.0] 4675b78a8 Speed up vsm_findseg

Martin Blix Grydeland martin at varnish-software.com
Fri Oct 18 13:23:08 UTC 2019


commit 4675b78a887c696e51e4053b7f2b13d6751452f6
Author: Martin Blix Grydeland <martin at varnish-software.com>
Date:   Fri Aug 30 11:18:12 2019 +0200

    Speed up vsm_findseg
    
    This speeds up vsm_findseg by keeping a direct pointer to the struct
    vsm_seg it points to in the vsm_fantom.
    
    To prevent stale vsm_fantoms in an application from causing segfaults, the
    last serial number seen is also kept on the fantom. If this serial number
    does not match, the slow path of iterating all segs to find the right
    match is done, and the fantom is updated to make any subsequent calls on
    the same fantom take the fast path.
    
    With this patch the fantoms store 2 serial numbers and a direct pointer to
    the seg. To fit this in struct vsm_fantom without breaking the ABI, the
    unused chunk pointer value has been repurposed, giving us two uintptr_t
    priv variables to work with. The direct segment pointer occupies one, and
    the two serial numbers occupy one half each of the other. This limits the
    bits for each serial to only 16 bits on 32 bit systems. But only direct
    matches is done, and it is unlikely that a stale fantom should have the
    exact right value to be accepted as valid.

diff --git a/include/vapi/vsm.h b/include/vapi/vsm.h
index cd7387032..a852e4c77 100644
--- a/include/vapi/vsm.h
+++ b/include/vapi/vsm.h
@@ -44,7 +44,7 @@ struct vsm;
 
 struct vsm_fantom {
 	uintptr_t		priv;		/* VSM private */
-	struct VSM_chunk	*chunk;
+	uintptr_t		priv2;		/* VSM private */
 	void			*b;		/* first byte of payload */
 	void			*e;		/* first byte past payload */
 	char			*class;
diff --git a/lib/libvarnishapi/vsm.c b/lib/libvarnishapi/vsm.c
index 3f26ca516..edb6ab4e5 100644
--- a/lib/libvarnishapi/vsm.c
+++ b/lib/libvarnishapi/vsm.c
@@ -72,6 +72,17 @@ const struct vsm_valid VSM_valid[1] = {{"valid"}};
 
 static vlu_f vsm_vlu_func;
 
+#define VSM_PRIV_SHIFT							\
+	(sizeof (uintptr_t) * 4)
+#define VSM_PRIV_MASK							\
+	(~((uintptr_t)UINTPTR_MAX << VSM_PRIV_SHIFT))
+#define VSM_PRIV_LOW(u)							\
+	((uintptr_t)(u) & VSM_PRIV_MASK)
+#define VSM_PRIV_HIGH(u)						\
+	(((uintptr_t)(u) >> VSM_PRIV_SHIFT) & VSM_PRIV_MASK)
+#define VSM_PRIV_MERGE(low, high)					\
+	(VSM_PRIV_LOW(low) | (VSM_PRIV_LOW(high) << VSM_PRIV_SHIFT))
+
 /*--------------------------------------------------------------------*/
 
 struct vsm_set;
@@ -528,7 +539,8 @@ vsm_vlu_plus(struct vsm *vd, struct vsm_set *vs, const char *line)
 		vg->av = av;
 		vg->set = vs;
 		vg->flags = VSM_FLAG_MARKSCAN;
-		vg->serial = ++vd->serial;
+		vg->serial = vd->serial;
+
 		VTAILQ_INSERT_TAIL(&vs->segs, vg, list);
 		if (ac == 4) {
 			vg->flags |= VSM_FLAG_CLUSTER;
@@ -586,6 +598,11 @@ vsm_vlu_func(void *priv, const char *line)
 	CHECK_OBJ_NOTNULL(vd, VSM_MAGIC);
 	AN(line);
 
+	/* Up the serial counter. This wraps at UINTPTR_MAX/2
+	 * because thats the highest value we can store in struct
+	 * vsm_fantom. */
+	vd->serial = VSM_PRIV_LOW(vd->serial + 1);
+
 	switch (line[0]) {
 	case '#':
 		i = vsm_vlu_hash(vd, vs, line);
@@ -612,6 +629,7 @@ vsm_readlines(struct vsm_set *vs)
 	int i;
 
 	do {
+		assert(vs->fd >= 0);
 		i = VLU_Fd(vs->vlu, vs->fd);
 	} while (!i);
 	assert(i == -2);
@@ -779,22 +797,44 @@ vsm_findseg(const struct vsm *vd, const struct vsm_fantom *vf)
 	struct vsm_seg *vg;
 	uintptr_t x;
 
-	x = vf->priv;
+	x = VSM_PRIV_HIGH(vf->priv);
+	if (x == vd->serial) {
+		vg = (struct vsm_seg *)vf->priv2;
+		if (!VALID_OBJ(vg, VSM_SEG_MAGIC) ||
+		    vg->serial != VSM_PRIV_LOW(vf->priv))
+			WRONG("Corrupt fantom");
+		return (vg);
+	}
+
+	x = VSM_PRIV_LOW(vf->priv);
 	vs = vd->mgt;
 	VTAILQ_FOREACH(vg, &vs->segs, list)
 		if (vg->serial == x)
-			return (vg);
-	VTAILQ_FOREACH(vg, &vs->stale, list)
-		if (vg->serial == x)
-			return (vg);
+			break;
+	if (vg == NULL) {
+		VTAILQ_FOREACH(vg, &vs->stale, list)
+			if (vg->serial == x)
+				break;
+	}
 	vs = vd->child;
-	VTAILQ_FOREACH(vg, &vs->segs, list)
-		if (vg->serial == x)
-			return (vg);
-	VTAILQ_FOREACH(vg, &vs->stale, list)
-		if (vg->serial == x)
-			return (vg);
-	return (NULL);
+	if (vg == NULL) {
+		VTAILQ_FOREACH(vg, &vs->segs, list)
+			if (vg->serial == x)
+				break;
+	}
+	if (vg == NULL) {
+		VTAILQ_FOREACH(vg, &vs->stale, list)
+			if (vg->serial == x)
+				break;
+	}
+	if (vg == NULL)
+		return (NULL);
+
+	/* Update the fantom with the new priv so that lookups will be
+	 * fast on the next call. Note that this casts away the const. */
+	((struct vsm_fantom *)TRUST_ME(vf))->priv =
+	    VSM_PRIV_MERGE(vg->serial, vd->serial);
+	return (vg);
 }
 
 /*--------------------------------------------------------------------*/
@@ -839,7 +879,8 @@ VSM__itern(struct vsm *vd, struct vsm_fantom *vf)
 		}
 	}
 	memset(vf, 0, sizeof *vf);
-	vf->priv = vg->serial;
+	vf->priv = VSM_PRIV_MERGE(vg->serial, vd->serial);
+	vf->priv2 = (uintptr_t)vg;
 	vf->class = vg->av[4];
 	vf->ident = vg->av[5];
 	AN(vf->class);
@@ -862,7 +903,7 @@ VSM_Map(struct vsm *vd, struct vsm_fantom *vf)
 	if (vg == NULL)
 		return (vsm_diag(vd, "VSM_Map: bad fantom"));
 
-	assert(vg->serial == vf->priv);
+	assert(vg->serial == VSM_PRIV_LOW(vf->priv));
 	assert(vg->av[4] == vf->class);
 	assert(vg->av[5] == vf->ident);
 


More information about the varnish-commit mailing list