[experimental-ims] d3f869a Remove sendfile(2) support, it doesn't seem to actually make any difference in practice and complicates the code and increases the size of storage data structures.

Geoff Simmons geoff at varnish-cache.org
Tue Feb 14 17:49:34 CET 2012


commit d3f869a0fded126050564b4244d0fbb22bfe22b4
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon Feb 13 11:42:31 2012 +0000

    Remove sendfile(2) support, it doesn't seem to actually make any
    difference in practice and complicates the code and increases the
    size of storage data structures.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index e376b66..3fafda1 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -371,10 +371,6 @@ struct storage {
 	unsigned		magic;
 #define STORAGE_MAGIC		0x1a4e51c0
 
-#ifdef SENDFILE_WORKS
-	int			fd;
-	off_t			where;
-#endif
 
 	VTAILQ_ENTRY(storage)	list;
 	struct stevedore	*stevedore;
@@ -916,9 +912,6 @@ unsigned WRW_Flush(struct worker *w);
 unsigned WRW_FlushRelease(struct worker *w);
 unsigned WRW_Write(struct worker *w, const void *ptr, int len);
 unsigned WRW_WriteH(struct worker *w, const txt *hh, const char *suf);
-#ifdef SENDFILE_WORKS
-void WRW_Sendfile(struct worker *w, int fd, off_t off, unsigned len);
-#endif  /* SENDFILE_WORKS */
 
 /* cache_session.c [SES] */
 struct sess *SES_Alloc(void);
diff --git a/bin/varnishd/cache/cache_response.c b/bin/varnishd/cache/cache_response.c
index 6c48468..2b22829 100644
--- a/bin/varnishd/cache/cache_response.c
+++ b/bin/varnishd/cache/cache_response.c
@@ -220,20 +220,6 @@ res_WriteDirObj(const struct sess *sp, ssize_t low, ssize_t high)
 		ptr += len;
 
 		sp->wrk->acct_tmp.bodybytes += len;
-#ifdef SENDFILE_WORKS
-		/*
-		 * XXX: the overhead of setting up sendfile is not
-		 * XXX: epsilon and maybe not even delta, so avoid
-		 * XXX: engaging sendfile for small objects.
-		 * XXX: Should use getpagesize() ?
-		 */
-		if (st->fd >= 0 &&
-		    st->len >= cache_param->sendfile_threshold) {
-			VSC_C_main->n_objsendfile++;
-			WRW_Sendfile(sp->wrk, st->fd, st->where + off, len);
-			continue;
-		}
-#endif /* SENDFILE_WORKS */
 		VSC_C_main->n_objwrite++;
 		(void)WRW_Write(sp->wrk, st->ptr + off, len);
 	}
diff --git a/bin/varnishd/cache/cache_wrw.c b/bin/varnishd/cache/cache_wrw.c
index e33be6c..4f9bbc5 100644
--- a/bin/varnishd/cache/cache_wrw.c
+++ b/bin/varnishd/cache/cache_wrw.c
@@ -35,17 +35,6 @@
 #include "config.h"
 
 #include <sys/types.h>
-#ifdef SENDFILE_WORKS
-#  if defined(__FreeBSD__) || defined(__DragonFly__)
-#    include <sys/socket.h>
-#  elif defined(__linux__)
-#    include <sys/sendfile.h>
-#  elif defined(__sun)
-#    include <sys/sendfile.h>
-#  else
-#     error Unknown sendfile() implementation
-#  endif
-#endif /* SENDFILE_WORKS */
 #include <sys/uio.h>
 
 #include <stdio.h>
@@ -290,69 +279,4 @@ WRW_EndChunk(struct worker *wrk)
 }
 
 
-#ifdef SENDFILE_WORKS
-void
-WRW_Sendfile(struct worker *wrk, int fd, off_t off, unsigned len)
-{
-	struct wrw *wrw;
-
-	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
-	wrw = &wrk->wrw;
-	AN(wrw->wfd);
-	assert(fd >= 0);
-	assert(len > 0);
-
-#if defined(__FreeBSD__) || defined(__DragonFly__)
-	do {
-		struct sf_hdtr sfh;
-		memset(&sfh, 0, sizeof sfh);
-		if (wrw->niov > 0) {
-			sfh.headers = wrw->iov;
-			sfh.hdr_cnt = wrw->niov;
-		}
-		if (sendfile(fd, *wrw->wfd, off, len, &sfh, NULL, 0) != 0)
-			wrw->werr++;
-		wrw->liov = 0;
-		wrw->niov = 0;
-	} while (0);
-#elif defined(__linux__)
-	do {
-		if (WRW_Flush(wrk) == 0 &&
-		    sendfile(*wrw->wfd, fd, &off, len) != len)
-			wrw->werr++;
-	} while (0);
-#elif defined(__sun) && defined(HAVE_SENDFILEV)
-	do {
-		sendfilevec_t svvec[cache_param->http_headers * 2 + 1];
-		size_t xferred = 0, expected = 0;
-		int i;
-		for (i = 0; i < wrw->niov; i++) {
-			svvec[i].sfv_fd = SFV_FD_SELF;
-			svvec[i].sfv_flag = 0;
-			svvec[i].sfv_off = (off_t) wrw->iov[i].iov_base;
-			svvec[i].sfv_len = wrw->iov[i].iov_len;
-			expected += svvec[i].sfv_len;
-		}
-		svvec[i].sfv_fd = fd;
-		svvec[i].sfv_flag = 0;
-		svvec[i].sfv_off = off;
-		svvec[i].sfv_len = len;
-		expected += svvec[i].sfv_len;
-		if (sendfilev(*wrw->wfd, svvec, i, &xferred) == -1 ||
-		    xferred != expected)
-			wrw->werr++;
-		wrw->liov = 0;
-		wrw->niov = 0;
-	} while (0);
-#elif defined(__sun) && defined(HAVE_SENDFILE)
-	do {
-		if (WRW_Flush(wrk) == 0 &&
-		    sendfile(*wrw->wfd, fd, &off, len) != len)
-			wrw->werr++;
-	} while (0);
-#else
-#error Unknown sendfile() implementation
-#endif
-}
-#endif /* SENDFILE_WORKS */
 
diff --git a/bin/varnishd/common/params.h b/bin/varnishd/common/params.h
index 1e51656..7694435 100644
--- a/bin/varnishd/common/params.h
+++ b/bin/varnishd/common/params.h
@@ -102,10 +102,6 @@ struct params {
 	ssize_t			fetch_maxchunksize;
 	unsigned		nuke_limit;
 
-#ifdef SENDFILE_WORKS
-	/* Sendfile object minimum size */
-	ssize_t			sendfile_threshold;
-#endif
 
 	/* VCL traces */
 	unsigned		vcl_trace;
diff --git a/bin/varnishd/mgt/mgt_param.c b/bin/varnishd/mgt/mgt_param.c
index 1abb273..97492ed 100644
--- a/bin/varnishd/mgt/mgt_param.c
+++ b/bin/varnishd/mgt/mgt_param.c
@@ -863,13 +863,6 @@ static const struct parspec input_parspec[] = {
 		"fragmentation.\n",
 		EXPERIMENTAL,
 		"256m", "bytes" },
-#ifdef SENDFILE_WORKS
-	{ "sendfile_threshold",
-		tweak_bytes, &mgt_param.sendfile_threshold, 0, 0,
-		"The minimum size of objects transmitted with sendfile.",
-		EXPERIMENTAL,
-		"1E", "bytes" },
-#endif /* SENDFILE_WORKS */
 	{ "vcl_trace", tweak_bool,  &mgt_param.vcl_trace, 0, 0,
 		"Trace VCL execution in the shmlog.\n"
 		"Enabling this will allow you to see the path each "
diff --git a/bin/varnishd/storage/storage_file.c b/bin/varnishd/storage/storage_file.c
index 9eb44d9..680b870 100644
--- a/bin/varnishd/storage/storage_file.c
+++ b/bin/varnishd/storage/storage_file.c
@@ -482,10 +482,6 @@ smf_alloc(struct stevedore *st, size_t size)
 	smf->s.ptr = smf->ptr;
 	smf->s.len = 0;
 	smf->s.stevedore = st;
-#ifdef SENDFILE_WORKS
-	smf->s.fd = smf->sc->fd;
-	smf->s.where = smf->offset;
-#endif
 	return (&smf->s);
 }
 
diff --git a/bin/varnishd/storage/storage_malloc.c b/bin/varnishd/storage/storage_malloc.c
index 156c832..2f34c98 100644
--- a/bin/varnishd/storage/storage_malloc.c
+++ b/bin/varnishd/storage/storage_malloc.c
@@ -117,9 +117,6 @@ sma_alloc(struct stevedore *st, size_t size)
 	sma->s.priv = sma;
 	sma->s.len = 0;
 	sma->s.space = size;
-#ifdef SENDFILE_WORKS
-	sma->s.fd = -1;
-#endif
 	sma->s.stevedore = st;
 	sma->s.magic = STORAGE_MAGIC;
 	return (&sma->s);
diff --git a/bin/varnishd/storage/storage_persistent.c b/bin/varnishd/storage/storage_persistent.c
index f90594e..ff01838 100644
--- a/bin/varnishd/storage/storage_persistent.c
+++ b/bin/varnishd/storage/storage_persistent.c
@@ -450,9 +450,6 @@ smp_allocx(struct stevedore *st, size_t min_size, size_t max_size,
 	ss->space = max_size;
 	ss->priv = sc;
 	ss->stevedore = st;
-#ifdef SENDFILE_WORKS
-	ss->fd = sc->fd;
-#endif
 	if (ssg != NULL)
 		*ssg = sg;
 	return (ss);
diff --git a/bin/varnishd/storage/storage_synth.c b/bin/varnishd/storage/storage_synth.c
index e9e9b2f..ba3fa24 100644
--- a/bin/varnishd/storage/storage_synth.c
+++ b/bin/varnishd/storage/storage_synth.c
@@ -87,9 +87,6 @@ SMS_Makesynth(struct object *obj)
 	sto->priv = vsb;
 	sto->len = 0;
 	sto->space = 0;
-#ifdef SENDFILE_WORKS
-	sto->fd = -1;
-#endif
 	sto->stevedore = &sms_stevedore;
 	sto->magic = STORAGE_MAGIC;
 
diff --git a/configure.ac b/configure.ac
index 72d2b7c..952b71b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -198,40 +198,6 @@ AC_CHECK_FUNCS([pthread_mutex_isowned_np])
 AC_CHECK_FUNCS([pthread_timedjoin_np])
 LIBS="${save_LIBS}"
 
-# sendfile is tricky: there are multiple versions, and most of them
-# don't work.
-case $target in
-*-*-freebsd*)
-	AC_CACHE_CHECK([whether sendfile works],
-	  [ac_cv_so_sendfile_works],
-	  [AC_RUN_IFELSE(
-	    [AC_LANG_PROGRAM([[
-	#include <sys/types.h>
-	#include <sys/socket.h>
-	#include <sys/uio.h>
-	    ]],[[
-		return (SF_SYNC == 0);
-	    ]])],
-	    [ac_cv_so_sendfile_works=yes],
-	    [ac_cv_so_sendfile_works=no])
-	  ])
-	;;
-#*-*-solaris*)
-#	save_LIBS="${LIBS}"
-#	LIBS="${NET_LIBS}"
-#	AC_CHECK_LIB(sendfile, sendfile)
-#	AC_CHECK_FUNCS([sendfile])
-#	AC_CHECK_FUNCS([sendfilev])
-#	NET_LIBS="${LIBS}"
-#	LIBS="${save_LIBS}"
-*)
-	AC_MSG_WARN([won't look for sendfile() on $target])
-	;;
-esac
-if test "$ac_cv_so_sendfile_works" = yes; then
-	AC_DEFINE([SENDFILE_WORKS], [1], [Define if SENDFILE works])
-fi
-
 # Support for visibility attribute 
 save_CFLAGS="${CFLAGS}" 
 CFLAGS="${CFLAGS} -Werror" 



More information about the varnish-commit mailing list