From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 17210 invoked from network); 19 Mar 2021 20:38:46 -0000 Received: from lists.zx2c4.com (165.227.139.114) by inbox.vuxu.org with ESMTPUTF8; 19 Mar 2021 20:38:46 -0000 Received: by lists.zx2c4.com (ZX2C4 Mail Server) with ESMTP id c3c6369a; Fri, 19 Mar 2021 20:38:25 +0000 (UTC) Return-Path: Received: from dcvr.yhbt.net (dcvr.yhbt.net [64.71.152.64]) by lists.zx2c4.com (ZX2C4 Mail Server) with ESMTPS id 3c5a65e8 (TLSv1.3:AEAD-AES256-GCM-SHA384:256:NO) for ; Fri, 19 Mar 2021 20:38:23 +0000 (UTC) Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 2733C1F9FC; Fri, 19 Mar 2021 20:38:22 +0000 (UTC) Date: Fri, 19 Mar 2021 20:38:22 +0000 From: Eric Wong To: cgit@lists.zx2c4.com Subject: [PATCH (resend)] use buffered stdio Message-ID: <20210319203822.GA30217@dcvr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-BeenThere: cgit@lists.zx2c4.com X-Mailman-Version: 2.1.30rc1 Precedence: list List-Id: List for cgit developers and users List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: cgit-bounces@lists.zx2c4.com Sender: "CGit" 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 --- I've been running this with Lua commit filter for the past two years without known problems. I don't use the cgit cache, though, so extra eyes on that would be appreciated. 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 55199e8..578b73b 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 08d81a1..40825cb 100644 --- a/cgit.c +++ b/cgit.c @@ -674,7 +674,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..fba26aa 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 7f81965..cefcf5e 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 18361a6..2801393 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 /* 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);