From: Eric Wong <[email protected]>

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

Signed-off-by: Eric Wong <[email protected]>
Signed-off-by: Alyssa Ross <[email protected]>
---
v2: rebase
v1: https://lists.zx2c4.com/pipermail/cgit/2021-March/004630.html

 cache.c       |  7 +++++++
 cgit.c        |  2 +-
 filter.c      | 22 +++++++++++++++++++++-
 html.c        |  2 +-
 ui-snapshot.c |  3 +++
 5 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/cache.c b/cache.c
index e70af13..13cceac 100644
--- a/cache.c
+++ b/cache.c
@@ -272,6 +272,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 ca318e8..3dc55c3 100644
--- a/cgit.c
+++ b/cgit.c
@@ -684,7 +684,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 c778d05..781f8e3 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,17 +145,32 @@ void cgit_init_filters(void)
 #endif
 
 #ifndef NO_LUA
-static ssize_t (*libc_write)(int fd, const void *buf, size_t count);
+static size_t (*libc_fwrite)(const void *buf, size_t size, size_t n, FILE *);
+static ssize_t (*libc_write)(int fd, const void *buf, size_t size);
 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)
 {
+       /*
+        * we need to wrap both functions since the Lua filter may
+        * have code which calls write(2) directly, bypassing fwrite(3)
+        */
+       libc_fwrite = dlsym(RTLD_NEXT, "fwrite");
+       if (!libc_fwrite)
+               die("Could not locate libc's write function");
        libc_write = dlsym(RTLD_NEXT, "write");
        if (!libc_write)
                die("Could not locate libc's write function");
 }
 
+size_t fwrite(const void *buf, size_t size, size_t n, FILE *f)
+{
+       if (f != stdout || !filter_write)
+               return libc_fwrite(buf, size, n, f);
+       return filter_write(current_write_filter, buf, size * n);
+}
+
 ssize_t write(int fd, const void *buf, size_t count)
 {
        if (fd != STDOUT_FILENO || !filter_write)
@@ -305,6 +322,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 0bac34b..ced781a 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 d157222..b3039d9 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -39,6 +39,9 @@ static int write_archive_type(const char *format, const char 
*hex, const char *p
        /* strvec guarantees a trailing NULL entry. */
        memcpy(nargv, argv.v, sizeof(char *) * (argv.nr + 1));
 
+       if (fflush(stdout))
+               return errno;
+
        result = write_archive(argv.nr, nargv, NULL, the_repository, NULL, 0);
        strvec_clear(&argv);
        free(nargv);

base-commit: 044821677c774cd24f25f1818ea51d09cc64b006
-- 
2.53.0

Reply via email to