[RFC] use buffered stdio (lightly tested)

[RFC] use buffered stdio (lightly tested)

[e]

Our generation of HTML triggers many small write(2) syscalls
which is inefficient.

Time output on a horrible query against my git.git mirror
shows significant performance improvement:

QUERY_STRING=id=2b93bfac0f5bcabbf60f174f4e7bfa9e318e64d5&id2=d6da71a9d16b8cf27f9d8f90692d3625c849cbc8
PATH_INFO=/mirrors/git.git/diff
export QUERY_STRING PATH_INFO
time ./cgit >/dev/null

Before:
real    0m1.585s
user    0m0.904s
sys     0m0.658s

After:
real    0m0.750s
user    0m0.666s
sys     0m0.076s
---
 cache.c       |  7 +++++++
 cgit.c        |  2 +-
 filter.c      | 21 +++++++++++++--------
 html.c        |  2 +-
 ui-snapshot.c |  3 +++
 5 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/cache.c b/cache.c
index 2c70be7..580c0e8 100644
--- a/cache.c
+++ b/cache.c
@@ -265,6 +265,13 @@ static int process_slot(struct cache_slot *slot)
 {
 	int err;
 
+	/*
+	 * Make sure any buffered data is flushed before we redirect,
+	 * do sendfile(2) or write(2)
+	 */
+	if (fflush(stdout))
+		return errno;
+
 	err = open_slot(slot);
 	if (!err && slot->match) {
 		if (is_expired(slot)) {
diff --git a/cgit.c b/cgit.c
index 2f07e6d..55ba735 100644
--- a/cgit.c
+++ b/cgit.c
@@ -667,7 +667,7 @@ static inline void authenticate_post(void)
 		len = MAX_AUTHENTICATION_POST_BYTES;
 	if ((len = read(STDIN_FILENO, buffer, len)) < 0)
 		die_errno("Could not read POST from stdin");
-	if (write(STDOUT_FILENO, buffer, len) < 0)
+	if (fwrite(buffer, 1, len, stdout) < len)
 		die_errno("Could not write POST to stdout");
 	cgit_close_filter(ctx.cfg.auth_filter);
 	exit(0);
diff --git a/filter.c b/filter.c
index 70f5b74..2c387c1 100644
--- a/filter.c
+++ b/filter.c
@@ -48,6 +48,7 @@ static int open_exec_filter(struct cgit_filter *base, va_list ap)
 	for (i = 0; i < filter->base.argument_count; i++)
 		filter->argv[i + 1] = va_arg(ap, char *);
 
+	chk_zero(fflush(stdout), "unable to flush STDOUT");
 	filter->old_stdout = chk_positive(dup(STDOUT_FILENO),
 		"Unable to duplicate STDOUT");
 	chk_zero(pipe(pipe_fh), "Unable to create pipe to subprocess");
@@ -71,6 +72,7 @@ static int close_exec_filter(struct cgit_filter *base)
 	struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base;
 	int i, exit_status = 0;
 
+	chk_zero(fflush(stdout), "unable to flush STDOUT");
 	chk_non_negative(dup2(filter->old_stdout, STDOUT_FILENO),
 		"Unable to restore STDOUT");
 	close(filter->old_stdout);
@@ -143,22 +145,22 @@ void cgit_init_filters(void)
 #endif
 
 #ifndef NO_LUA
-static ssize_t (*libc_write)(int fd, const void *buf, size_t count);
+static ssize_t (*libc_fwrite)(const void *buf, size_t size, size_t n, FILE *);
 static ssize_t (*filter_write)(struct cgit_filter *base, const void *buf, size_t count) = NULL;
 static struct cgit_filter *current_write_filter = NULL;
 
 void cgit_init_filters(void)
 {
-	libc_write = dlsym(RTLD_NEXT, "write");
-	if (!libc_write)
-		die("Could not locate libc's write function");
+	libc_fwrite = dlsym(RTLD_NEXT, "fwrite");
+	if (!libc_fwrite)
+		die("Could not locate libc's fwrite function");
 }
 
-ssize_t write(int fd, const void *buf, size_t count)
+size_t fwrite(const void *buf, size_t size, size_t n, FILE *f)
 {
-	if (fd != STDOUT_FILENO || !filter_write)
-		return libc_write(fd, buf, count);
-	return filter_write(current_write_filter, buf, count);
+	if (f != stdout || !filter_write)
+		return libc_fwrite(buf, size, n, f);
+	return filter_write(current_write_filter, buf, size * n);
 }
 
 static inline void hook_write(struct cgit_filter *filter, ssize_t (*new_write)(struct cgit_filter *base, const void *buf, size_t count))
@@ -305,6 +307,9 @@ static int open_lua_filter(struct cgit_filter *base, va_list ap)
 	struct lua_filter *filter = (struct lua_filter *)base;
 	int i;
 
+	if (fflush(stdout))
+		return 1;
+
 	if (init_lua_filter(filter))
 		return 1;
 
diff --git a/html.c b/html.c
index 138c649..ca9db91 100644
--- a/html.c
+++ b/html.c
@@ -80,7 +80,7 @@ char *fmtalloc(const char *format, ...)
 
 void html_raw(const char *data, size_t size)
 {
-	if (write(STDOUT_FILENO, data, size) != size)
+	if (fwrite(data, 1, size, stdout) != size)
 		die_errno("write error on html output");
 }
 
diff --git a/ui-snapshot.c b/ui-snapshot.c
index 9461d51..b9f03de 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -37,6 +37,9 @@ static int write_archive_type(const char *format, const char *hex, const char *p
 	/* argv_array guarantees a trailing NULL entry. */
 	memcpy(nargv, argv.argv, sizeof(char *) * (argv.argc + 1));
 
+	if (fflush(stdout))
+		return errno;
+
 	result = write_archive(argv.argc, nargv, NULL, the_repository, NULL, 0);
 	argv_array_clear(&argv);
 	free(nargv);
-- 
EW

[RFC] use buffered stdio (lightly tested)

[e]

> index 70f5b74..2c387c1 100644
> --- a/filter.c
> +++ b/filter.c
> @@ -143,22 +145,22 @@ void cgit_init_filters(void)
>  #endif
>  
>  #ifndef NO_LUA
> -static ssize_t (*libc_write)(int fd, const void *buf, size_t count);
> +static ssize_t (*libc_fwrite)(const void *buf, size_t size, size_t n, FILE *);

Oops, that should be size_t:

  static size_t (*libc_fwrite)(const void *buf, size_t size, size_t n, FILE *);

I also think we need to keep hooking write(2) in addition to
fwrite(3), since (I think) the Lua script may call write(2)
outside of the API we provide it.

[RFC] use buffered stdio (lightly tested)

[e]

In the future, I think it could be even more beneficial to
buffer to zlib (if the client accepts gzip) and bypass stdio
buffering in those cases.  Smaller responses via gzip means less
IPC overhead and wakeups for the reader.

cgit is linked against zlib anyways because of git, so no extra
linkage, either.  But we'll need uncompressed code paths anyways
for clients which don't support gzip.