[master] 512dddc Hindsight is always so glaring sharp...

Poul-Henning Kamp phk at varnish-cache.org
Tue Feb 7 09:49:04 CET 2012


commit 512dddc60ace8c4eaacfde351dfa06756fb32d8d
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Tue Feb 7 08:47:05 2012 +0000

    Hindsight is always so glaring sharp...
    
    It was a mistake to put the gunzip delivery buffers on the worker
    stack, given that only a minority of clients (mostly spiders and robots)
    don't grok "Content-Encoding: gzip".
    
    Move them to malloc space.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index ad4f426..b89ef23 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -262,9 +262,6 @@ struct stream_ctx {
 #define STREAM_CTX_MAGIC	0x8213728b
 
 	struct vgz		*vgz;
-	void			*obuf;
-	ssize_t			obuf_len;
-	ssize_t			obuf_ptr;
 
 	/* Next byte we will take from storage */
 	ssize_t			stream_next;
@@ -786,8 +783,12 @@ int VGZ_Gzip(struct vgz *, const void **, size_t *len, enum vgz_flag);
 int VGZ_Gunzip(struct vgz *, const void **, size_t *len);
 int VGZ_Destroy(struct vgz **, int vsl_id);
 void VGZ_UpdateObj(const struct vgz*, struct object *);
+
+int VGZ_WrwInit(struct vgz *vg);
 int VGZ_WrwGunzip(struct worker *w, struct vgz *, const void *ibuf,
-    ssize_t ibufl, char *obuf, ssize_t obufl, ssize_t *obufp);
+    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_center.c b/bin/varnishd/cache/cache_center.c
index c933175..a8148b5 100644
--- a/bin/varnishd/cache/cache_center.c
+++ b/bin/varnishd/cache/cache_center.c
@@ -941,8 +941,6 @@ cnt_streambody(struct sess *sp, struct worker *wrk, struct req *req)
 {
 	int i;
 	struct stream_ctx sctx;
-	uint8_t obuf[sp->wrk->res_mode & RES_GUNZIP ?
-	    cache_param->gzip_stack_buffer : 1];
 
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
@@ -954,12 +952,6 @@ cnt_streambody(struct sess *sp, struct worker *wrk, struct req *req)
 	AZ(wrk->sctx);
 	wrk->sctx = &sctx;
 
-	if (wrk->res_mode & RES_GUNZIP) {
-		sctx.vgz = VGZ_NewUngzip(wrk, "U S -");
-		sctx.obuf = obuf;
-		sctx.obuf_len = sizeof (obuf);
-	}
-
 	RES_StreamStart(sp);
 
 	AssertObjCorePassOrBusy(req->obj->objcore);
@@ -985,8 +977,6 @@ cnt_streambody(struct sess *sp, struct worker *wrk, struct req *req)
 	req->restarts = 0;
 
 	RES_StreamEnd(sp);
-	if (wrk->res_mode & RES_GUNZIP)
-		(void)VGZ_Destroy(&sctx.vgz, sp->vsl_id);
 
 	wrk->sctx = NULL;
 	assert(WRW_IsReleased(wrk));
diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index a7e80cd..0175c3d 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -224,8 +224,6 @@ ESI_Deliver(struct sess *sp)
 	uint8_t tailbuf[8 + 5];
 	int isgzip;
 	struct vgz *vgz = NULL;
-	char obuf[cache_param->gzip_stack_buffer];
-	ssize_t obufl = 0;
 	size_t dl;
 	const void *dp;
 	int i;
@@ -233,9 +231,6 @@ ESI_Deliver(struct sess *sp)
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 	st = sp->req->obj->esidata;
 	AN(st);
-	assert(sizeof obuf >= 1024);
-
-	obuf[0] = 0;	/* For flexelint */
 
 	p = st->ptr;
 	e = st->ptr + st->len;
@@ -264,16 +259,14 @@ ESI_Deliver(struct sess *sp)
 
 	if (isgzip && !sp->req->gzip_resp) {
 		vgz = VGZ_NewUngzip(sp->wrk, "U D E");
+		AZ(VGZ_WrwInit(vgz));
 
 		/* Feed a gzip header to gunzip to make it happy */
 		VGZ_Ibuf(vgz, gzip_hdr, sizeof gzip_hdr);
-		VGZ_Obuf(vgz, obuf, sizeof obuf);
 		i = VGZ_Gunzip(vgz, &dp, &dl);
 		assert(i == VGZ_OK);
 		assert(VGZ_IbufEmpty(vgz));
 		assert(dl == 0);
-
-		obufl = 0;
 	}
 
 	st = VTAILQ_FIRST(&sp->req->obj->store);
@@ -328,8 +321,7 @@ ESI_Deliver(struct sess *sp)
 					 */
 					AN(vgz);
 					i = VGZ_WrwGunzip(sp->wrk, vgz,
-						st->ptr + off, l2,
-						obuf, sizeof obuf, &obufl);
+						st->ptr + off, l2);
 					if (WRW_Error(sp->wrk)) {
 						SES_Close(sp, "remote closed");
 						p = e;
@@ -379,10 +371,8 @@ ESI_Deliver(struct sess *sp)
 			q++;
 			r = (void*)strchr((const char*)q, '\0');
 			AN(r);
-			if (obufl > 0) {
-				(void)WRW_Write(sp->wrk, obuf, obufl);
-				obufl = 0;
-			}
+			if (vgz != NULL)
+				VGZ_WrwFlush(sp->wrk, vgz);
 			if (WRW_Flush(sp->wrk)) {
 				SES_Close(sp, "remote closed");
 				p = e;
@@ -399,8 +389,7 @@ ESI_Deliver(struct sess *sp)
 		}
 	}
 	if (vgz != NULL) {
-		if (obufl > 0)
-			(void)WRW_Write(sp->wrk, obuf, obufl);
+		VGZ_WrwFinish(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_gzip.c b/bin/varnishd/cache/cache_gzip.c
index 1b732ca..63a18cd 100644
--- a/bin/varnishd/cache/cache_gzip.c
+++ b/bin/varnishd/cache/cache_gzip.c
@@ -83,7 +83,12 @@ struct vgz {
 	char			*tmp_snapshot;
 	int			last_i;
 
-	struct storage		*obuf;
+	struct storage		*st_obuf;
+
+	/* Wrw stuff */
+	char			*wrw_buf;
+	ssize_t			wrw_sz;
+	ssize_t			wrw_len;
 
 	z_stream		vz;
 };
@@ -260,7 +265,7 @@ VGZ_ObufStorage(struct worker *wrk, struct vgz *vg)
 	if (st == NULL)
 		return (-1);
 
-	vg->obuf = st;
+	vg->st_obuf = st;
 	VGZ_Obuf(vg, st->ptr + st->len, st->space - st->len);
 
 	return (0);
@@ -287,8 +292,8 @@ VGZ_Gunzip(struct vgz *vg, const void **pptr, size_t *plen)
 		*pptr = before;
 		l = (const uint8_t *)vg->vz.next_out - before;
 		*plen = l;
-		if (vg->obuf != NULL)
-			vg->obuf->len += l;
+		if (vg->st_obuf != NULL)
+			vg->st_obuf->len += l;
 	}
 	vg->last_i = i;
 	if (i == Z_OK)
@@ -330,8 +335,8 @@ VGZ_Gzip(struct vgz *vg, const void **pptr, size_t *plen, enum vgz_flag flags)
 		*pptr = before;
 		l = (const uint8_t *)vg->vz.next_out - before;
 		*plen = l;
-		if (vg->obuf != NULL)
-			vg->obuf->len += l;
+		if (vg->st_obuf != NULL)
+			vg->st_obuf->len += l;
 	}
 	vg->last_i = i;
 	if (i == Z_OK)
@@ -344,41 +349,63 @@ VGZ_Gzip(struct vgz *vg, const void **pptr, size_t *plen, enum vgz_flag flags)
 }
 
 /*--------------------------------------------------------------------
+ */
+
+int
+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;
+		return (-1);
+	}
+	VGZ_Obuf(vg, vg->wrw_buf, vg->wrw_sz);
+	return (0);
+}
+
+/*--------------------------------------------------------------------
  * Gunzip ibuf into outb, if it runs full, emit it with WRW.
  * Leave flushing to caller, more data may be coming.
  */
 
 int
 VGZ_WrwGunzip(struct worker *wrk, struct vgz *vg, const void *ibuf,
-    ssize_t ibufl, char *obuf, ssize_t obufl, ssize_t *obufp)
+    ssize_t ibufl)
 {
 	int i;
 	size_t dl;
 	const void *dp;
 
+	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
-	assert(obufl > 16);
+	AN(vg->wrw_buf);
 	VGZ_Ibuf(vg, ibuf, ibufl);
 	if (ibufl == 0)
 		return (VGZ_OK);
-	VGZ_Obuf(vg, obuf + *obufp, obufl - *obufp);
 	do {
-		if (obufl == *obufp)
+		if (vg->wrw_len == vg->wrw_sz)
 			i = VGZ_STUCK;
 		else {
 			i = VGZ_Gunzip(vg, &dp, &dl);
-			*obufp += dl;
+			vg->wrw_len += dl;
 		}
 		if (i < VGZ_OK) {
 			/* XXX: VSL ? */
 			return (-1);
 		}
-		if (obufl == *obufp || i == VGZ_STUCK) {
-			wrk->acct_tmp.bodybytes += *obufp;
-			(void)WRW_Write(wrk, obuf, *obufp);
+		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);
 			(void)WRW_Flush(wrk);
-			*obufp = 0;
-			VGZ_Obuf(vg, obuf + *obufp, obufl - *obufp);
+			vg->wrw_len = 0;
+			VGZ_Obuf(vg, vg->wrw_buf, vg->wrw_sz);
 		}
 	} while (!VGZ_IbufEmpty(vg));
 	if (i == VGZ_STUCK)
@@ -389,6 +416,35 @@ VGZ_WrwGunzip(struct worker *wrk, struct vgz *vg, const void *ibuf,
 /*--------------------------------------------------------------------*/
 
 void
+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)
+		return;
+	(void)WRW_Write(wrk, vg->wrw_buf, vg->wrw_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;
+}
+
+/*--------------------------------------------------------------------*/
+
+void
 VGZ_UpdateObj(const struct vgz *vg, struct object *obj)
 {
 
@@ -412,6 +468,7 @@ 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",
diff --git a/bin/varnishd/cache/cache_response.c b/bin/varnishd/cache/cache_response.c
index 5724f07..6206c18 100644
--- a/bin/varnishd/cache/cache_response.c
+++ b/bin/varnishd/cache/cache_response.c
@@ -161,15 +161,13 @@ res_WriteGunzipObj(const struct sess *sp)
 	struct storage *st;
 	unsigned u = 0;
 	struct vgz *vg;
-	char obuf[cache_param->gzip_stack_buffer];
-	ssize_t obufl = 0;
 	int i;
 
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 
 	vg = VGZ_NewUngzip(sp->wrk, "U D -");
+	AZ(VGZ_WrwInit(vg));
 
-	VGZ_Obuf(vg, obuf, sizeof obuf);
 	VTAILQ_FOREACH(st, &sp->req->obj->store, list) {
 		CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 		CHECK_OBJ_NOTNULL(st, STORAGE_MAGIC);
@@ -177,16 +175,11 @@ res_WriteGunzipObj(const struct sess *sp)
 
 		VSC_C_main->n_objwrite++;
 
-		i = VGZ_WrwGunzip(sp->wrk, vg,
-		    st->ptr, st->len,
-		    obuf, sizeof obuf, &obufl);
+		i = VGZ_WrwGunzip(sp->wrk, vg, st->ptr, st->len);
 		/* XXX: error check */
 		(void)i;
 	}
-	if (obufl) {
-		(void)WRW_Write(sp->wrk, obuf, obufl);
-		(void)WRW_Flush(sp->wrk);
-	}
+	VGZ_WrwFinish(sp->wrk, vg);
 	(void)VGZ_Destroy(&vg, sp->vsl_id);
 	assert(u == sp->req->obj->len);
 }
@@ -346,11 +339,12 @@ RES_StreamStart(struct sess *sp)
 	AN(sp->req->wantbody);
 
 	WRW_Reserve(sp->wrk, &sp->fd);
-	/*
-	 * Always remove C-E if client don't grok it
-	 */
-	if (sp->wrk->res_mode & RES_GUNZIP)
+
+	if (sp->wrk->res_mode & RES_GUNZIP) {
+		sctx->vgz = VGZ_NewUngzip(sp->wrk, "U S -");
+		AZ(VGZ_WrwInit(sctx->vgz));
 		http_Unset(sp->req->resp, H_Content_Encoding);
+	}
 
 	if (!(sp->wrk->res_mode & RES_CHUNKED) &&
 	    sp->wrk->busyobj->h_content_length != NULL)
@@ -388,8 +382,7 @@ RES_StreamPoll(struct worker *wrk)
 		l2 = st->len + l - sctx->stream_next;
 		ptr = st->ptr + (sctx->stream_next - l);
 		if (wrk->res_mode & RES_GUNZIP) {
-			(void)VGZ_WrwGunzip(wrk, sctx->vgz, ptr, l2,
-			    sctx->obuf, sctx->obuf_len, &sctx->obuf_ptr);
+			(void)VGZ_WrwGunzip(wrk, sctx->vgz, ptr, l2);
 		} else {
 			(void)WRW_Write(wrk, ptr, l2);
 		}
@@ -425,8 +418,11 @@ RES_StreamEnd(struct sess *sp)
 	sctx = sp->wrk->sctx;
 	CHECK_OBJ_NOTNULL(sctx, STREAM_CTX_MAGIC);
 
-	if (sp->wrk->res_mode & RES_GUNZIP && sctx->obuf_ptr > 0)
-		(void)WRW_Write(sp->wrk, sctx->obuf, sctx->obuf_ptr);
+	if (sp->wrk->res_mode & RES_GUNZIP) {
+		AN(sctx->vgz);
+		VGZ_WrwFinish(sp->wrk, sctx->vgz);
+		(void)VGZ_Destroy(&sctx->vgz, sp->vsl_id);
+	}
 	if (sp->wrk->res_mode & RES_CHUNKED &&
 	    !(sp->wrk->res_mode & RES_ESI_CHILD))
 		WRW_EndChunk(sp->wrk);



More information about the varnish-commit mailing list