From: jamie.couture at gmail.com (Jamie Couture)
Subject: [PATCH 2/4] cache: use sendfile() instead of a pair of read() + write()
Date: Sat, 18 Jan 2014 16:32:15 -0500 [thread overview]
Message-ID: <20140118213215.GA10922@neptune> (raw)
In-Reply-To: <1390076700-16626-3-git-send-email-sebastian@breakpoint.cc>
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
next prev parent reply other threads:[~2014-01-18 21:32 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-18 20:24 my out-of-tree patches for cgit sebastian
2014-01-18 20:24 ` [PATCH 1/4] snapshots: Don't allow sneaked in snapshots requests sebastian
2014-02-01 14:54 ` sebastian
2014-02-02 14:49 ` cgit
2014-01-18 20:24 ` [PATCH 2/4] cache: use sendfile() instead of a pair of read() + write() sebastian
2014-01-18 21:32 ` jamie.couture [this message]
2014-01-19 14:12 ` Jason
2014-01-19 14:13 ` Jason
2014-01-19 15:17 ` Jason
2014-01-18 20:24 ` [PATCH 3/4] summary: Add tag head line in the dowload section sebastian
2014-01-19 14:17 ` Jason
2014-02-02 14:43 ` cgit
2014-01-18 20:25 ` [PATCH 4/4] repolist: add a git link on front page sebastian
2014-01-19 14:19 ` Jason
2014-02-01 14:53 ` sebastian
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140118213215.GA10922@neptune \
--to=cgit@lists.zx2c4.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).