From df07dbc7914eced446c77afbdd4b7c6f3479681e Mon Sep 17 00:00:00 2001 From: Moti Cohen Date: Mon, 11 Dec 2023 17:37:44 +0200 Subject: [PATCH] fread() instead of read() + Add GNUC format validator (#37) This commit attempts to reflect also part of the work from previous messy commit: 1. Add GNUC format validator for RDB_log() and RDB_reportError() 2. fread() instead of read() to improve performance. >> Add GNUC format validator for RDB_log() and RDB_reportError()** In case of GNUC compiplation add attribute((format(printf, X, Y))) To verify during compilation correctness of the provided FORMAT versus variadic arguments that follows. >> fread() instead of read() to improve performance** Following a preliminary benchmark that involved reading from a file-descriptor (fd) I have found out that assisting openfd() outperform around ~50% better than using lower level read(). This is because when using bare read(), it makes multiple system-calls to read a small amount of data whereas fread() attempts to read a big chunk of data to user-space, even if requested a small amount. I also tested using bare read() with a buffer that i managed inside readerFileDesc.c and it reached Similar results to fread(). Note, the benchmark relied on librdb library that is integrated into another utility (rl_rdbloader), the adjustment is relatively self-contained and provides a reliable indication of its effectiveness. --- api/librdb-api.h | 12 +++++++++--- src/ext/readerFileDesc.c | 2 ++ src/lib/parser.c | 4 ++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/api/librdb-api.h b/api/librdb-api.h index c2e2ebe..41f2386 100644 --- a/api/librdb-api.h +++ b/api/librdb-api.h @@ -11,9 +11,6 @@ extern "C" { #define _LIBRDB_API __attribute__((visibility("default"))) #endif -#ifndef __GNUC__ -#define __attribute__(a) -#endif typedef char *RdbBulk; typedef char *RdbBulkCopy; @@ -439,7 +436,12 @@ _LIBRDB_API void RDB_setMaxRawSize(RdbParser *p, size_t maxSize); /* logger */ _LIBRDB_API void RDB_setLogLevel(RdbParser *p, RdbLogLevel l); _LIBRDB_API void RDB_setLogger(RdbParser *p, RdbLoggerCB f); +#ifdef __GNUC__ +_LIBRDB_API void RDB_log(RdbParser *p, RdbLogLevel lvl, const char *format, ...) + __attribute__((format(printf, 3, 4))); +#else _LIBRDB_API void RDB_log(RdbParser *p, RdbLogLevel lvl, const char *format, ...); +#endif /* Following function returns a hint for the total number of items in the current * parsed key context - to assist with memory allocation or other optimizations. @@ -476,8 +478,12 @@ _LIBRDB_API void RDB_pauseParser(RdbParser *p); ****************************************************************/ _LIBRDB_API RdbRes RDB_getErrorCode(RdbParser *p); _LIBRDB_API const char *RDB_getErrorMessage(RdbParser *p); +#ifdef __GNUC__ _LIBRDB_API void RDB_reportError(RdbParser *p, RdbRes e, const char *msg, ...) __attribute__((format(printf, 3, 4))); +#else +_LIBRDB_API void RDB_reportError(RdbParser *p, RdbRes e, const char *msg, ...); +#endif /**************************************************************** * Memory management diff --git a/src/ext/readerFileDesc.c b/src/ext/readerFileDesc.c index 9267695..e5ed754 100644 --- a/src/ext/readerFileDesc.c +++ b/src/ext/readerFileDesc.c @@ -33,6 +33,7 @@ static RdbStatus readFileDesc(void *data, void *buf, size_t len) { size_t totalBytesRead = 0; while (1) { + totalBytesRead += fread((char *)buf + totalBytesRead, 1, len - totalBytesRead, ctx->file); if (totalBytesRead == len) { @@ -75,6 +76,7 @@ RdbxReaderFileDesc *RDBX_createReaderFileDesc(RdbParser *p, int fd, int fdCloseW return NULL; } + /* Use fdopen and fread instead of read in order to enjoy read-ahead buffering and less syscalls. */ FILE *file = fdopen(fd, "r"); if (file == NULL) { diff --git a/src/lib/parser.c b/src/lib/parser.c index ba23c32..76d6210 100644 --- a/src/lib/parser.c +++ b/src/lib/parser.c @@ -469,7 +469,7 @@ _LIBRDB_API void RDB_reportError(RdbParser *p, RdbRes e, const char *msg, ...) { vsnprintf(p->errorMsg + nchars, MAX_ERROR_MSG - nchars, msg, args); va_end(args); - RDB_log(p, RDB_LOG_ERR, p->errorMsg); + RDB_log(p, RDB_LOG_ERR, "%s", p->errorMsg); } _LIBRDB_API const char *RDB_getErrorMessage(RdbParser *p) { @@ -659,7 +659,7 @@ static RdbStatus parserMainLoop(RdbParser *p) { if (status != RDB_STATUS_OK) at += snprintf(buff+at, sizeof(buff)-1-at, " >>> [Status=%s]", getStatusString(status)); - RDB_log(p, RDB_LOG_DBG, buff); + RDB_log(p, RDB_LOG_DBG, "%s", buff); if (status != RDB_STATUS_OK) break;