Skip to content

Commit

Permalink
fread() instead of read() + Add GNUC format validator (#37)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
moticless authored Dec 11, 2023
1 parent 46a463e commit df07dbc
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 5 deletions.
12 changes: 9 additions & 3 deletions api/librdb-api.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/ext/readerFileDesc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/lib/parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;

Expand Down

0 comments on commit df07dbc

Please sign in to comment.