open fd with fdopen() to improve performance (#36)

Optimize reading from fd by using fdopen()

api/librdb-api.h

@@ -11,6 +11,10 @@ extern "C" {
 #define _LIBRDB_API __attribute__((visibility("default")))
 #endif
 
+#ifndef __GNUC__
+#define __attribute__(a)
+#endif
+
 typedef char *RdbBulk;
 typedef char *RdbBulkCopy;
 
@@ -45,6 +49,7 @@ typedef enum RdbRes {
     RDB_ERR_GENERAL,
 
     RDB_ERR_FAIL_ALLOC,
+    RDB_ERR_FAILED_OPEN_FILE,
     RDB_ERR_INVALID_CONFIGURATION,
     RDB_ERR_FAILED_CREATE_PARSER,
     RDB_ERR_FAILED_OPEN_LOG_FILE,
@@ -471,7 +476,8 @@ _LIBRDB_API void RDB_pauseParser(RdbParser *p);
  ****************************************************************/
 _LIBRDB_API RdbRes RDB_getErrorCode(RdbParser *p);
 _LIBRDB_API const char *RDB_getErrorMessage(RdbParser *p);
-_LIBRDB_API void RDB_reportError(RdbParser *p, RdbRes e, const char *msg, ...);
+_LIBRDB_API void RDB_reportError(RdbParser *p, RdbRes e, const char *msg, ...)
+                                        __attribute__((format(printf, 3, 4)));
 
 /****************************************************************
  * Memory management

api/librdb-ext-api.h

@@ -26,7 +26,6 @@ typedef enum {
     RDBX_ERR_RESP_FAILED_ALLOC,
 
     /* rdb2json errors */
-    RDBX_ERR_FAILED_OPEN_FILE,
     RDBX_ERR_R2J_INVALID_STATE,
     RDBX_ERR_R2J_INVALID_LEVEL,
 

src/ext/handlersToJson.c

@@ -1,4 +1,5 @@
 #include <assert.h>
+#include <errno.h>
 #include <stdio.h>
 #include <string.h>
 #include <ctype.h>
@@ -97,8 +98,8 @@ static RdbxToJson *initRdbToJsonCtx(RdbParser *p, const char *outfilename, RdbxT
         f = stdout;
         outfilename = _STDOUT_STR;
     } else if (!(f = fopen(outfilename, "w"))) {
-        RDB_reportError(p, (RdbRes) RDBX_ERR_FAILED_OPEN_FILE,
-                        "HandlersRdbToJson: Failed to open file");
+        RDB_reportError(p, RDB_ERR_FAILED_OPEN_FILE,
+                        "HandlersRdbToJson: Failed to open file. errno=%d: %s", errno, strerror(errno));
         return NULL;
     }
 

src/ext/readerFile.c

@@ -49,8 +49,7 @@ RdbxReaderFile *RDBX_createReaderFile(RdbParser *p, const char *filename) {
     FILE *f;
 
     if (filename == NULL) {
-        RDB_reportError(p, RDB_ERR_FAILED_OPEN_RDB_FILE,
-                        "Filename is not provided", filename);
+        RDB_reportError(p, RDB_ERR_FAILED_OPEN_RDB_FILE, "Filename is not provided");
         return NULL;
     }
 

src/ext/readerFileDesc.c

@@ -1,70 +1,61 @@
+#define _POSIX_C_SOURCE 1 /* Required in order to use file-descriptors */
+
 #include <stdio.h>
 #include <fcntl.h>
 #include <string.h>
-#include <assert.h>
 #include <errno.h>
 #include <unistd.h>
 #include "common.h"
 
+
 struct RdbxReaderFileDesc {
     RdbParser *parser;
     int fdCloseWhenDone;
     int fd;
+    FILE *file;
 };
 
 static void deleteReaderFileDesc(RdbParser *p, void *rdata) {
     if (!rdata) return;
 
-    RdbxReaderFileDesc *readerData = (RdbxReaderFileDesc *) rdata;
-    if (readerData->fdCloseWhenDone)
-        close(readerData->fd);
-    RDB_free(p, readerData);
+    RdbxReaderFileDesc *ctx = (RdbxReaderFileDesc *) rdata;
+    if (ctx->file)
+        fclose(ctx->file);
+
+    if (ctx->fdCloseWhenDone)
+        close(ctx->fd);
+    RDB_free(p, ctx);
 }
 
 /* Attempts to read entire len, otherwise returns error */
 static RdbStatus readFileDesc(void *data, void *buf, size_t len) {
     RdbxReaderFileDesc *ctx = (RdbxReaderFileDesc *)data;
     size_t totalBytesRead = 0;
 
-    while (totalBytesRead < len) {
-        ssize_t bytesRead = read(ctx->fd, (char *)buf + totalBytesRead, len - totalBytesRead);
-
-        /* read some data */
-        if (likely(bytesRead > 0)) {
-            totalBytesRead += bytesRead;
-            continue;
-        }
-
-        /* didn't read any data. Stop. */
-        if (bytesRead == 0) {
-            break;
-        }
-
-        assert(bytesRead == -1);
-
-        /* If interrupted, retry read */
-        if (errno == EINTR)
-            continue;
-
-        /* Wrongly configured to nonblocking mode (Not supported at the moment) */
-        if (errno == EAGAIN || errno == EWOULDBLOCK) {
-            RDB_reportError(ctx->parser, RDB_ERR_NONBLOCKING_READ_FD,
-                            "readFileDesc(): Unexpected EAGAIN|EWOULDBLOCK. The fd must be set to blocking mode");
-            return RDB_STATUS_ERROR;
+    while (1) {
+        totalBytesRead += fread((char *)buf + totalBytesRead, 1, len - totalBytesRead, ctx->file);
+
+        if (totalBytesRead == len) {
+            return RDB_STATUS_OK;
+        } else {
+            if (feof(ctx->file)) {
+                RDB_reportError(ctx->parser, RDB_ERR_FAILED_READ_RDB_FILE,
+                    "readFileDesc(fd=%d): Reached EOF. Not all requested bytes were read", ctx->fd);
+                return RDB_STATUS_ERROR;
+            } else if (ferror(ctx->file)) {
+                /* Wrongly configured to nonblocking mode (Not supported at the moment) */
+                if (errno == EAGAIN || errno == EWOULDBLOCK) {
+                    RDB_reportError(ctx->parser, RDB_ERR_NONBLOCKING_READ_FD,
+                        "readFileDesc(fd=%d): Unexpected EAGAIN|EWOULDBLOCK. The fd must be set to blocking mode", ctx->fd);
+                    return RDB_STATUS_ERROR;
+                }
+
+                RDB_reportError(ctx->parser, RDB_ERR_FAILED_READ_RDB_FILE,
+                    "readFileDesc(fd=%d): System errno %d : %s", ctx->fd, errno, strerror(errno));
+                return RDB_STATUS_ERROR;
+            }
         }
-
-        RDB_reportError(ctx->parser, RDB_ERR_FAILED_READ_RDB_FILE,
-                        "readFileDesc(): Read failed with errno=%d", errno);
-        return RDB_STATUS_ERROR;
     }
-
-    if (totalBytesRead < len) {
-        RDB_reportError(ctx->parser, RDB_ERR_FAILED_READ_RDB_FILE,
-                        "readFileDesc(): Not all requested bytes were read");
-        return RDB_STATUS_ERROR;
-    }
-
-    return RDB_STATUS_OK;
 }
 
 RdbxReaderFileDesc *RDBX_createReaderFileDesc(RdbParser *p, int fd, int fdCloseWhenDone) {
@@ -74,19 +65,29 @@ RdbxReaderFileDesc *RDBX_createReaderFileDesc(RdbParser *p, int fd, int fdCloseW
     if (flags==-1) {
 
         RDB_reportError(p, RDB_ERR_FAILED_GET_FD_FLAGS,
-            "RDBX_createReaderFileDesc(): Error getting file descriptor flags");
+            "RDBX_createReaderFileDesc(fd=%d): Failed to get fcntl(). Error:%s", fd, strerror(errno));
         return NULL;
     }
 
     if (flags & O_NONBLOCK) {
         RDB_reportError(p, RDB_ERR_NONBLOCKING_FD,
-            "RDBX_createReaderFileDesc(): fd must be set to blocking mode");
+            "RDBX_createReaderFileDesc(fd=%d): fd must be set to blocking mode", fd);
+        return NULL;
+    }
+
+    FILE *file = fdopen(fd, "r");
+
+    if (file == NULL) {
+        RDB_reportError(p, RDB_ERR_FAILED_OPEN_FILE,
+        "RDBX_createReaderFileDesc(fd=%d): failed on fdopen() with errno %d: %s\"",
+            fd, errno, strerror(errno));
         return NULL;
     }
 
     RdbxReaderFileDesc *ctx = (RdbxReaderFileDesc *) RDB_alloc(p, sizeof(RdbxReaderFileDesc));
     ctx->parser = p;
     ctx->fd = fd;
+    ctx->file = file;
     ctx->fdCloseWhenDone = fdCloseWhenDone;
     RDB_createReaderRdb(p, readFileDesc, ctx, deleteReaderFileDesc);
     return ctx;

src/ext/respToFileWriter.c

@@ -1,7 +1,7 @@
 #include <stdio.h>
 #include "common.h"
 #include <string.h>
-#include <unistd.h>
+#include <errno.h>
 
 struct RdbxRespToFileWriter {
     long cmdCount;
@@ -20,7 +20,7 @@ static int respFileWritev(void *context, struct iovec *iov, int count,
     /* not optimized code */
     for (int i = 0 ; i < count ; ++i) {
         if (unlikely(fwrite(iov[i].iov_base, sizeof(char), iov[i].iov_len, ctx->filePtr) != iov[i].iov_len)) {
-            RDB_reportError(ctx->p, (RdbRes) RDBX_ERR_RESP_WRITE, "Failed to write RESP to file");
+            RDB_reportError(ctx->p, (RdbRes) RDBX_ERR_RESP_WRITE, "Failed to write RESP to file: (errno=%d)", errno);
             return 1;
         }
     }
@@ -54,8 +54,8 @@ RdbxRespToFileWriter *RDBX_createRespToFileWriter(RdbParser *p, RdbxToResp *rdbT
     } else {
         filePtr = fopen(filePath, "wb");
         if (filePtr == NULL) {
-            RDB_reportError(p, (RdbRes) RDBX_ERR_FAILED_OPEN_FILE, "createRespWriter: Failed to open file: %s",
-                            filePath);
+            RDB_reportError(p, RDB_ERR_FAILED_OPEN_FILE, "createRespWriter: Failed to open file: %s. errno:%d",
+                            filePath, errno);
             return NULL;
         }
     }

src/lib/parser.c

@@ -345,7 +345,7 @@ _LIBRDB_API size_t RDB_bulkLen(RdbParser *p, RdbBulk b) {
     }
 
     RDB_reportError(p, RDB_ERR_INVALID_BULK_LENGTH_REQUEST,
-                    "Invalid RDB_bulkLen() request. Couldn't find application-bulk with address: %p", b);
+                    "Invalid RDB_bulkLen() request. Couldn't find application-bulk with address: %p", (void*)b);
 
     return 0;
 }
@@ -363,7 +363,7 @@ _LIBRDB_API int RDB_isRefBulk(RdbParser *p, RdbBulk b) {
     }
 
     RDB_reportError(p, RDB_ERR_INVALID_IS_REF_BULK,
-                    "Invalid RDB_isRefBulk() request. Couldn't find application-bulk with address: %p", b);
+                    "Invalid RDB_isRefBulk() request. Couldn't find application-bulk with address: %p", (void*)b);
     return 0;
 }
 
@@ -375,7 +375,7 @@ _LIBRDB_API RdbBulkCopy RDB_bulkClone(RdbParser *p, RdbBulk b) {
     }
 
     RDB_reportError(p, RDB_ERR_INVALID_BULK_CLONE_REQUEST,
-                    "Invalid RDB_bulkClone() request. Couldn't find application-bulk with address: %p", b);
+                    "Invalid RDB_bulkClone() request. Couldn't find application-bulk with address: %p", (void*)b);
 
     return NULL;
 }
@@ -1980,7 +1980,7 @@ RdbStatus elementEndOfFile(RdbParser *p) {
             if (cksum == 0) {
                 RDB_log(p, RDB_LOG_WRN, "RDB file was saved with checksum disabled: no check performed.");
             } else if (cksum != evaluated) {
-                RDB_reportError(p, RDB_ERR_CHECKSUM_FAILURE, "Wrong RDB checksum checksum=%lx, evaluated=%lx",
+                RDB_reportError(p, RDB_ERR_CHECKSUM_FAILURE, "Wrong RDB checksum checksum=%llx, evaluated=%llx",
                                 (unsigned long long) cksum,
                                 (unsigned long long) p->checksum);
                 return RDB_STATUS_ERROR;
@@ -2042,7 +2042,7 @@ RdbStatus elementModule(RdbParser *p) {
                     IF_NOT_OK_RETURN(rdbLoadLen(p, NULL, &when, NULL, NULL));
                     if (unlikely(when_opcode != RDB_MODULE_OPCODE_UINT)) {
                         RDB_reportError(p, RDB_ERR_MODULE_INVALID_WHEN_OPCODE,
-                            "elementModule() : Invalid when opcode: %d.", when_opcode);
+                            "elementModule() : Invalid when opcode: %ld.", when_opcode);
                         return RDB_STATUS_ERROR;
                     }
                 }
@@ -2588,7 +2588,7 @@ RdbStatus rdbLoadString(RdbParser *p, AllocTypeRq type, char *refBuf, BulkInfo *
                 return rdbLoadLzfString(p, type, refBuf, binfo);
             default:
                 RDB_reportError(p, RDB_ERR_STRING_UNKNOWN_ENCODING_TYPE,
-                                "rdbLoadString(): Unknown RDB string encoding type: %llu",len);
+                                "rdbLoadString(): Unknown RDB string encoding type: %lu",len);
                 return RDB_STATUS_ERROR;
         }
     }

src/lib/parserRaw.c

@@ -315,7 +315,7 @@ RdbStatus elementRawString(RdbParser *p) {
                         break;
                     default:
                         RDB_reportError(p, RDB_ERR_STRING_UNKNOWN_ENCODING_TYPE,
-                            "elementRawString(): Unknown RDB string encoding type: %llu", strCtx->len);
+                            "elementRawString(): Unknown RDB string encoding type: %lu", strCtx->len);
                         return RDB_STATUS_ERROR;
                 }
             }
@@ -419,7 +419,7 @@ RdbStatus elementRawString(RdbParser *p) {
             }
 
             RDB_reportError(p, RDB_ERR_STRING_UNKNOWN_ENCODING_TYPE,
-                           "elementRawString(): Unknown RDB string encoding type: %llu", strCtx->encoding);
+                           "elementRawString(): Unknown RDB string encoding type: %lu", strCtx->encoding);
             return RDB_STATUS_ERROR;
         }
 
@@ -694,7 +694,7 @@ RdbStatus elementRawModule(RdbParser *p) {
                 IF_NOT_OK_RETURN(rdbLoadLen(p, NULL, &ma->when, NULL, NULL));
                 if (unlikely(ma->when_opcode != RDB_MODULE_OPCODE_UINT)) {
                     RDB_reportError(p, RDB_ERR_MODULE_INVALID_WHEN_OPCODE,
-                                    "elementRawModule() : Invalid when opcode: %d.", ma->when_opcode);
+                                    "elementRawModule() : Invalid when opcode: %ld.", ma->when_opcode);
                     return RDB_STATUS_ERROR;
                 }
                 /*** ENTER SAFE STATE ***/