[master] 42f03cc Move the last g(un)zip buffers from the wrk->stack to malloc(3).

Poul-Henning Kamp phk at varnish-cache.org
Tue Feb 7 14:59:19 CET 2012


commit 42f03cc1c580d1acb811eb63284584d98393a171
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Tue Feb 7 13:57:39 2012 +0000

    Move the last g(un)zip buffers from the wrk->stack to malloc(3).
    
    Rename the gzip_stack_buffer param to gzip_buffer.
    Remove the gzip_tmp_space param.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index b89ef23..f7bef9d 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -788,7 +788,6 @@ int VGZ_WrwInit(struct vgz *vg);
 int VGZ_WrwGunzip(struct worker *w, struct vgz *, const void *ibuf,
     ssize_t ibufl);
 void VGZ_WrwFlush(struct worker *wrk, struct vgz *vg);
-void VGZ_WrwFinish(struct worker *wrk, struct vgz *vg);
 
 /* Return values */
 #define VGZ_ERROR	-1
diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index 0175c3d..a9bc694 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -389,7 +389,7 @@ ESI_Deliver(struct sess *sp)
 		}
 	}
 	if (vgz != NULL) {
-		VGZ_WrwFinish(sp->wrk, vgz);
+		VGZ_WrwFlush(sp->wrk, vgz);
 		(void)VGZ_Destroy(&vgz, sp->vsl_id);
 	}
 	if (sp->req->gzip_resp && sp->req->esi_level == 0) {
diff --git a/bin/varnishd/cache/cache_esi_fetch.c b/bin/varnishd/cache/cache_esi_fetch.c
index 4b9d04d..64adf51 100644
--- a/bin/varnishd/cache/cache_esi_fetch.c
+++ b/bin/varnishd/cache/cache_esi_fetch.c
@@ -315,17 +315,17 @@ vfp_esi_begin(struct worker *wrk, size_t estimate)
 	if (bo->is_gzip && bo->do_gunzip) {
 		bo->vgz_rx = VGZ_NewUngzip(wrk, "U F E");
 		VEP_Init(wrk, NULL);
-		vef->ibuf_sz = cache_param->gzip_stack_buffer;
+		vef->ibuf_sz = cache_param->gzip_buffer;
 	} else if (bo->is_gunzip && bo->do_gzip) {
 		vef->vgz = VGZ_NewGzip(wrk, "G F E");
 		VEP_Init(wrk, vfp_vep_callback);
-		vef->ibuf_sz = cache_param->gzip_stack_buffer;
+		vef->ibuf_sz = cache_param->gzip_buffer;
 	} else if (bo->is_gzip) {
 		bo->vgz_rx = VGZ_NewUngzip(wrk, "U F E");
 		vef->vgz = VGZ_NewGzip(wrk, "G F E");
 		VEP_Init(wrk, vfp_vep_callback);
-		vef->ibuf_sz = cache_param->gzip_stack_buffer;
-		vef->ibuf2_sz = cache_param->gzip_stack_buffer;
+		vef->ibuf_sz = cache_param->gzip_buffer;
+		vef->ibuf2_sz = cache_param->gzip_buffer;
 	} else {
 		VEP_Init(wrk, NULL);
 	}
diff --git a/bin/varnishd/cache/cache_gzip.c b/bin/varnishd/cache/cache_gzip.c
index 63a18cd..402633f 100644
--- a/bin/varnishd/cache/cache_gzip.c
+++ b/bin/varnishd/cache/cache_gzip.c
@@ -86,34 +86,13 @@ struct vgz {
 	struct storage		*st_obuf;
 
 	/* Wrw stuff */
-	char			*wrw_buf;
-	ssize_t			wrw_sz;
-	ssize_t			wrw_len;
+	char			*m_buf;
+	ssize_t			m_sz;
+	ssize_t			m_len;
 
 	z_stream		vz;
 };
 
-/*--------------------------------------------------------------------*/
-
-static voidpf
-vgz_alloc(voidpf opaque, uInt items, uInt size)
-{
-	struct vgz *vg;
-
-	CAST_OBJ_NOTNULL(vg, opaque, VGZ_MAGIC);
-
-	return (WS_Alloc(vg->tmp, items * size));
-}
-
-static void
-vgz_free(voidpf opaque, voidpf address)
-{
-	struct vgz *vg;
-
-	CAST_OBJ_NOTNULL(vg, opaque, VGZ_MAGIC);
-	(void)address;
-}
-
 /*--------------------------------------------------------------------
  * Set up a gunzip instance
  */
@@ -127,30 +106,10 @@ vgz_alloc_vgz(struct worker *wrk, const char *id)
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	ws = wrk->ws;
 	WS_Assert(ws);
-	// XXX: we restore workspace in esi:include
-	// vg = (void*)WS_Alloc(ws, sizeof *vg);
 	ALLOC_OBJ(vg, VGZ_MAGIC);
 	AN(vg);
-	memset(vg, 0, sizeof *vg);
-	vg->magic = VGZ_MAGIC;
 	vg->wrk = wrk;
 	vg->id = id;
-
-	switch (cache_param->gzip_tmp_space) {
-	case 0:
-	case 1:
-		/* malloc, the default */
-		break;
-	case 2:
-		vg->tmp = wrk->ws;
-		vg->tmp_snapshot = WS_Snapshot(vg->tmp);
-		vg->vz.zalloc = vgz_alloc;
-		vg->vz.zfree = vgz_free;
-		vg->vz.opaque = vg;
-		break;
-	default:
-		assert(0 == __LINE__);
-	}
 	return (vg);
 }
 
@@ -210,6 +169,27 @@ VGZ_NewGzip(struct worker *wrk, const char *id)
 	return (vg);
 }
 
+/*--------------------------------------------------------------------
+ */
+
+static int
+vgz_getmbuf(struct vgz *vg)
+{
+
+	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
+	AZ(vg->m_sz);
+	AZ(vg->m_len);
+	AZ(vg->m_buf);
+
+	vg->m_sz = cache_param->gzip_buffer;
+	vg->m_buf = malloc(vg->m_sz);
+	if (vg->m_buf == NULL) {
+		vg->m_sz = 0;
+		return (-1);
+	}
+	return (0);
+}
+
 /*--------------------------------------------------------------------*/
 
 void
@@ -356,17 +336,11 @@ VGZ_WrwInit(struct vgz *vg)
 {
 
 	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
-	AZ(vg->wrw_sz);
-	AZ(vg->wrw_len);
-	AZ(vg->wrw_buf);
-
-	vg->wrw_sz = cache_param->gzip_stack_buffer;
-	vg->wrw_buf = malloc(vg->wrw_sz);
-	if (vg->wrw_buf == NULL) {
-		vg->wrw_sz = 0;
+
+	if (vgz_getmbuf(vg))
 		return (-1);
-	}
-	VGZ_Obuf(vg, vg->wrw_buf, vg->wrw_sz);
+
+	VGZ_Obuf(vg, vg->m_buf, vg->m_sz);
 	return (0);
 }
 
@@ -385,27 +359,27 @@ VGZ_WrwGunzip(struct worker *wrk, struct vgz *vg, const void *ibuf,
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
-	AN(vg->wrw_buf);
+	AN(vg->m_buf);
 	VGZ_Ibuf(vg, ibuf, ibufl);
 	if (ibufl == 0)
 		return (VGZ_OK);
 	do {
-		if (vg->wrw_len == vg->wrw_sz)
+		if (vg->m_len == vg->m_sz)
 			i = VGZ_STUCK;
 		else {
 			i = VGZ_Gunzip(vg, &dp, &dl);
-			vg->wrw_len += dl;
+			vg->m_len += dl;
 		}
 		if (i < VGZ_OK) {
 			/* XXX: VSL ? */
 			return (-1);
 		}
-		if (vg->wrw_len == vg->wrw_sz || i == VGZ_STUCK) {
-			wrk->acct_tmp.bodybytes += vg->wrw_len;
-			(void)WRW_Write(wrk, vg->wrw_buf, vg->wrw_len);
+		if (vg->m_len == vg->m_sz || i == VGZ_STUCK) {
+			wrk->acct_tmp.bodybytes += vg->m_len;
+			(void)WRW_Write(wrk, vg->m_buf, vg->m_len);
 			(void)WRW_Flush(wrk);
-			vg->wrw_len = 0;
-			VGZ_Obuf(vg, vg->wrw_buf, vg->wrw_sz);
+			vg->m_len = 0;
+			VGZ_Obuf(vg, vg->m_buf, vg->m_sz);
 		}
 	} while (!VGZ_IbufEmpty(vg));
 	if (i == VGZ_STUCK)
@@ -421,25 +395,11 @@ VGZ_WrwFlush(struct worker *wrk, struct vgz *vg)
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
 
-	if (vg->wrw_len ==  0)
+	if (vg->m_len ==  0)
 		return;
-	(void)WRW_Write(wrk, vg->wrw_buf, vg->wrw_len);
+	(void)WRW_Write(wrk, vg->m_buf, vg->m_len);
 	(void)WRW_Flush(wrk);
-	vg->wrw_len = 0;
-}
-
-/*--------------------------------------------------------------------*/
-
-void
-VGZ_WrwFinish(struct worker *wrk, struct vgz *vg)
-{
-	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
-	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
-
-	VGZ_WrwFlush(wrk, vg);
-	free(vg->wrw_buf);
-	vg->wrw_buf = 0;
-	vg->wrw_sz = 0;
+	vg->m_len = 0;
 }
 
 /*--------------------------------------------------------------------*/
@@ -468,7 +428,6 @@ VGZ_Destroy(struct vgz **vgp, int vsl_id)
 	vg = *vgp;
 	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
 	*vgp = NULL;
-	AZ(vg->wrw_buf);
 
 	if (vsl_id < 0)
 		WSLB(vg->wrk, SLT_Gzip, "%s %jd %jd %jd %jd %jd",
@@ -494,6 +453,8 @@ VGZ_Destroy(struct vgz **vgp, int vsl_id)
 		i = inflateEnd(&vg->vz);
 	if (vg->last_i == Z_STREAM_END && i == Z_OK)
 		i = Z_STREAM_END;
+	if (vg->m_buf)
+		free(vg->m_buf);
 	FREE_OBJ(vg);
 	if (i == Z_OK)
 		return (VGZ_OK);
@@ -518,6 +479,7 @@ vfp_gunzip_begin(struct worker *wrk, size_t estimate)
 	CHECK_OBJ_NOTNULL(wrk->busyobj, BUSYOBJ_MAGIC);
 	AZ(wrk->busyobj->vgz_rx);
 	wrk->busyobj->vgz_rx = VGZ_NewUngzip(wrk, "U F -");
+	XXXAZ(vgz_getmbuf(wrk->busyobj->vgz_rx));
 }
 
 static int __match_proto__()
@@ -526,7 +488,6 @@ vfp_gunzip_bytes(struct worker *wrk, struct http_conn *htc, ssize_t bytes)
 	struct vgz *vg;
 	ssize_t l, wl;
 	int i = -100;
-	uint8_t	ibuf[cache_param->gzip_stack_buffer];
 	size_t dl;
 	const void *dp;
 
@@ -538,13 +499,13 @@ vfp_gunzip_bytes(struct worker *wrk, struct http_conn *htc, ssize_t bytes)
 	AZ(vg->vz.avail_in);
 	while (bytes > 0 || vg->vz.avail_in > 0) {
 		if (vg->vz.avail_in == 0 && bytes > 0) {
-			l = sizeof ibuf;
+			l = vg->m_sz;
 			if (l > bytes)
 				l = bytes;
-			wl = HTC_Read(wrk, htc, ibuf, l);
+			wl = HTC_Read(wrk, htc, vg->m_buf, l);
 			if (wl <= 0)
 				return (wl);
-			VGZ_Ibuf(vg, ibuf, wl);
+			VGZ_Ibuf(vg, vg->m_buf, wl);
 			bytes -= wl;
 		}
 
@@ -586,7 +547,6 @@ struct vfp vfp_gunzip = {
         .end    =       vfp_gunzip_end,
 };
 
-
 /*--------------------------------------------------------------------
  * VFP_GZIP
  *
@@ -602,6 +562,7 @@ vfp_gzip_begin(struct worker *wrk, size_t estimate)
 	CHECK_OBJ_NOTNULL(wrk->busyobj, BUSYOBJ_MAGIC);
 	AZ(wrk->busyobj->vgz_rx);
 	wrk->busyobj->vgz_rx = VGZ_NewGzip(wrk, "G F -");
+	XXXAZ(vgz_getmbuf(wrk->busyobj->vgz_rx));
 }
 
 static int __match_proto__()
@@ -610,7 +571,6 @@ vfp_gzip_bytes(struct worker *wrk, struct http_conn *htc, ssize_t bytes)
 	struct vgz *vg;
 	ssize_t l, wl;
 	int i = -100;
-	uint8_t ibuf[cache_param->gzip_stack_buffer];
 	size_t dl;
 	const void *dp;
 
@@ -622,13 +582,13 @@ vfp_gzip_bytes(struct worker *wrk, struct http_conn *htc, ssize_t bytes)
 	AZ(vg->vz.avail_in);
 	while (bytes > 0 || !VGZ_IbufEmpty(vg)) {
 		if (VGZ_IbufEmpty(vg) && bytes > 0) {
-			l = sizeof ibuf;
+			l = vg->m_sz;
 			if (l > bytes)
 				l = bytes;
-			wl = HTC_Read(wrk, htc, ibuf, l);
+			wl = HTC_Read(wrk, htc, vg->m_buf, l);
 			if (wl <= 0)
 				return (wl);
-			VGZ_Ibuf(vg, ibuf, wl);
+			VGZ_Ibuf(vg, vg->m_buf, wl);
 			bytes -= wl;
 		}
 		if (VGZ_ObufStorage(wrk, vg))
@@ -695,6 +655,7 @@ vfp_testgzip_begin(struct worker *wrk, size_t estimate)
 	(void)estimate;
 	wrk->busyobj->vgz_rx = VGZ_NewUngzip(wrk, "u F -");
 	CHECK_OBJ_NOTNULL(wrk->busyobj->vgz_rx, VGZ_MAGIC);
+	XXXAZ(vgz_getmbuf(wrk->busyobj->vgz_rx));
 }
 
 static int __match_proto__()
@@ -703,7 +664,6 @@ vfp_testgzip_bytes(struct worker *wrk, struct http_conn *htc, ssize_t bytes)
 	struct vgz *vg;
 	ssize_t l, wl;
 	int i = -100;
-	uint8_t	obuf[cache_param->gzip_stack_buffer];
 	size_t dl;
 	const void *dp;
 	struct storage *st;
@@ -732,7 +692,7 @@ vfp_testgzip_bytes(struct worker *wrk, struct http_conn *htc, ssize_t bytes)
 			RES_StreamPoll(wrk);
 
 		while (!VGZ_IbufEmpty(vg)) {
-			VGZ_Obuf(vg, obuf, sizeof obuf);
+			VGZ_Obuf(vg, vg->m_buf, vg->m_sz);
 			i = VGZ_Gunzip(vg, &dp, &dl);
 			if (i == VGZ_END && !VGZ_IbufEmpty(vg))
 				return(FetchError(wrk, "Junk after gzip data"));
diff --git a/bin/varnishd/cache/cache_response.c b/bin/varnishd/cache/cache_response.c
index 6206c18..6c48468 100644
--- a/bin/varnishd/cache/cache_response.c
+++ b/bin/varnishd/cache/cache_response.c
@@ -179,7 +179,7 @@ res_WriteGunzipObj(const struct sess *sp)
 		/* XXX: error check */
 		(void)i;
 	}
-	VGZ_WrwFinish(sp->wrk, vg);
+	VGZ_WrwFlush(sp->wrk, vg);
 	(void)VGZ_Destroy(&vg, sp->vsl_id);
 	assert(u == sp->req->obj->len);
 }
@@ -420,7 +420,7 @@ RES_StreamEnd(struct sess *sp)
 
 	if (sp->wrk->res_mode & RES_GUNZIP) {
 		AN(sctx->vgz);
-		VGZ_WrwFinish(sp->wrk, sctx->vgz);
+		VGZ_WrwFlush(sp->wrk, sctx->vgz);
 		(void)VGZ_Destroy(&sctx->vgz, sp->vsl_id);
 	}
 	if (sp->wrk->res_mode & RES_CHUNKED &&
diff --git a/bin/varnishd/common/params.h b/bin/varnishd/common/params.h
index 6c1899a..015e5d2 100644
--- a/bin/varnishd/common/params.h
+++ b/bin/varnishd/common/params.h
@@ -181,8 +181,7 @@ struct params {
 	unsigned		http_range_support;
 
 	unsigned		http_gzip_support;
-	unsigned		gzip_stack_buffer;
-	unsigned		gzip_tmp_space;
+	unsigned		gzip_buffer;
 	unsigned		gzip_level;
 	unsigned		gzip_window;
 	unsigned		gzip_memlevel;
diff --git a/bin/varnishd/mgt/mgt_param.c b/bin/varnishd/mgt/mgt_param.c
index c984aae..60d49f8 100644
--- a/bin/varnishd/mgt/mgt_param.c
+++ b/bin/varnishd/mgt/mgt_param.c
@@ -1084,17 +1084,6 @@ static const struct parspec input_parspec[] = {
 		"Varnish reference.",
 		EXPERIMENTAL,
 		"on", "bool" },
-	{ "gzip_tmp_space", tweak_uint, &mgt_param.gzip_tmp_space, 0, 2,
-		"Where temporary space for gzip/gunzip is allocated:\n"
-		"  0 - malloc\n"
-		"  2 - thread workspace\n"
-		"\n"
-		"If you have much gzip/gunzip activity, it may be an"
-		" advantage to use workspace for these allocations to reduce"
-		" malloc activity.  Be aware that gzip needs 256+KB and gunzip"
-		" needs 32+KB of workspace (64+KB if ESI processing).",
-		EXPERIMENTAL,
-		"0", "" },
 	{ "gzip_level", tweak_uint, &mgt_param.gzip_level, 0, 9,
 		"Gzip compression level: 0=debug, 1=fast, 9=best",
 		0,
@@ -1109,11 +1098,11 @@ static const struct parspec input_parspec[] = {
 		"Memory impact is 1=1k, 2=2k, ... 9=256k.",
 		0,
 		"8", ""},
-	{ "gzip_stack_buffer",
-		tweak_bytes_u, &mgt_param.gzip_stack_buffer,
+	{ "gzip_buffer",
+		tweak_bytes_u, &mgt_param.gzip_buffer,
 	        2048, UINT_MAX,
-		"Size of stack buffer used for gzip processing.\n"
-		"The stack buffers are used for in-transit data,"
+		"Size of malloc buffer used for gzip processing.\n"
+		"These buffers are used for in-transit data,"
 		" for instance gunzip'ed data being sent to a client."
 		"Making this space to small results in more overhead,"
 		" writes to sockets etc, making it too big is probably"
diff --git a/bin/varnishtest/tests/e00020.vtc b/bin/varnishtest/tests/e00020.vtc
index 5b40c16..f9b73a4 100644
--- a/bin/varnishtest/tests/e00020.vtc
+++ b/bin/varnishtest/tests/e00020.vtc
@@ -22,7 +22,6 @@ varnish v1 -vcl+backend {
 
 varnish v1 -cliok "param.set esi_syntax 4"
 varnish v1 -cliok "param.set http_gzip_support true"
-varnish v1 -cliok "param.set gzip_tmp_space 2"
 
 client c1 {
 	txreq 
diff --git a/bin/varnishtest/tests/e00022.vtc b/bin/varnishtest/tests/e00022.vtc
index 897f3ee..1f13cd3 100644
--- a/bin/varnishtest/tests/e00022.vtc
+++ b/bin/varnishtest/tests/e00022.vtc
@@ -25,7 +25,6 @@ varnish v1 -arg "-p thread_pool_stack=262144" -vcl+backend {
 
 varnish v1 -cliok "param.set esi_syntax 0xc"
 varnish v1 -cliok "param.set http_gzip_support true"
-varnish v1 -cliok "param.set gzip_tmp_space 1"
 varnish v1 -cliok "param.set gzip_window 8"
 varnish v1 -cliok "param.set gzip_memlevel 1"
 



More information about the varnish-commit mailing list