From mboxrd@z Thu Jan  1 00:00:00 1970
From: jamie.couture at gmail.com (Jamie Couture)
Date: Sat, 18 Jan 2014 16:32:15 -0500
Subject: [PATCH 2/4] cache: use sendfile() instead of a pair of read() +
 write()
In-Reply-To: <1390076700-16626-3-git-send-email-sebastian@breakpoint.cc>
References: <1390076700-16626-1-git-send-email-sebastian@breakpoint.cc>
 <1390076700-16626-3-git-send-email-sebastian@breakpoint.cc>
Message-ID: <20140118213215.GA10922@neptune>

On Sat, Jan 18, 2014 at 09:24:58PM +0100, Sebastian Andrzej Siewior wrote:
> sendfile() does the same job and avoids to copy the content into userland
> and back. One has to define NO_SENDFILE in case the OS (kernel / libc)
> does not supported. It is disabled by default on non-linux environemnts.
> According to the glibc, sendfile64() was added in Linux 2.4 (so it has
> been there for a while) but after browsing over the mapage of FreeBSD's I
> noticed that the prototype is little different.
> 
> Signed-off-by: Sebastian Andrzej Siewior <sebastian at breakpoint.cc>
> ---
>  Makefile |  1 +
>  cache.c  | 26 +++++++++++++++++++++++++-
>  cgit.mk  |  8 ++++++++
>  3 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 2dc92df..05b97d7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -29,6 +29,7 @@ DOC_PDF  = $(patsubst %.txt,%.pdf,$(MAN_TXT))
>  # j, z, t. (representing long long int, char, intmax_t, size_t, ptrdiff_t).
>  # some C compilers supported these specifiers prior to C99 as an extension.
>  #
> +# Define HAVE_LINUX_SENDFILE to use sendfile()
>  
>  #-include config.mak
>  
> diff --git a/cache.c b/cache.c
> index fcd461f..9e7eeb0 100644
> --- a/cache.c
> +++ b/cache.c
> @@ -13,6 +13,9 @@
>   *
>   */
>  
> +#ifdef HAVE_LINUX_SENDFILE
> +#include <sys/sendfile.h>
> +#endif
>  #include "cgit.h"
>  #include "cache.h"
>  #include "html.h"
> @@ -30,7 +33,6 @@ struct cache_slot {
>  	const char *lock_name;
>  	int match;
>  	struct stat cache_st;
> -	struct stat lock_st;

Hmm. It looks like struct stat lock_st member, introduced in
939d32fd, was never used anywhere in cgit.  Would it make sense to
separate this line in a differnt commit, explaining the unused
member should be removed?

Not to nit, but I'm wondering if the subtly of removing this from
the cache_slot struct is worth a seperate commit.  Arguably it
doesn't hurt the way it is now, but it did throw me.

>  	int bufsize;
>  	char buf[CACHE_BUFSIZE];
>  };
> @@ -81,6 +83,23 @@ static int close_slot(struct cache_slot *slot)
>  /* Print the content of the active cache slot (but skip the key). */
>  static int print_slot(struct cache_slot *slot)
>  {
> +#ifdef HAVE_LINUX_SENDFILE
> +	off_t start_off;
> +	int ret;
> +
> +	start_off = slot->keylen + 1;
> +
> +	do {
> +		ret = sendfile(STDOUT_FILENO, slot->cache_fd, &start_off,
> +				slot->cache_st.st_size - start_off);
> +		if (ret < 0) {
> +			if (errno == EAGAIN || errno == EINTR)
> +				continue;
> +			return errno;
> +		}
> +		return 0;
> +	} while (1);
> +#else
>  	ssize_t i, j;
>  
>  	i = lseek(slot->cache_fd, slot->keylen + 1, SEEK_SET);
> @@ -97,6 +116,7 @@ static int print_slot(struct cache_slot *slot)
>  		return errno;
>  	else
>  		return 0;
> +#endif
>  }
>  
>  /* Check if the slot has expired */
> @@ -188,6 +208,10 @@ static int fill_slot(struct cache_slot *slot)
>  	/* Generate cache content */
>  	slot->fn();
>  
> +	/* update stat info */
> +	if (fstat(slot->lock_fd, &slot->cache_st))
> +		return errno;
> +
>  	/* Restore stdout */
>  	if (dup2(tmp, STDOUT_FILENO) == -1)
>  		return errno;
> diff --git a/cgit.mk b/cgit.mk
> index 056c3f9..3b8b79a 100644
> --- a/cgit.mk
> +++ b/cgit.mk
> @@ -68,6 +68,14 @@ ifeq ($(findstring BSD,$(uname_S)),)
>  	CGIT_LIBS += -ldl
>  endif
>  
> +# glibc 2.1+ offers sendfile which the most common C library on Linux
> +ifeq ($(uname_S),Linux)
> +	HAVE_LINUX_SENDFILE = YesPlease
> +endif
> +
> +ifdef HAVE_LINUX_SENDFILE
> +	CGIT_CFLAGS += -DHAVE_LINUX_SENDFILE
> +endif
>  
>  CGIT_OBJ_NAMES += cgit.o
>  CGIT_OBJ_NAMES += cache.o
> -- 
> 1.8.5.2
> 
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit