Modernize the build setup (#107)

* fix: modernize the project

* fixes

* fixes

* no need for verbose

* adding shared library

* simplifying

* fix: don't change the lib name and remove deprecated functions

---------

Co-authored-by: Daniel Lemire <dlemire@lemire.me>

CMakeLists.txt

@@ -1,52 +1,35 @@
-cmake_minimum_required(VERSION 3.0)
+cmake_minimum_required(VERSION 3.5)
 project(redis-roaring)
+set (CMAKE_C_STANDARD 11)
+set(CMAKE_POSITION_INDEPENDENT_CODE ON)
 
-set(CMAKE_VERBOSE_MAKEFILE on)
-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=gnu99 -Wall -Werror")
 set(SRC_PATH ${CMAKE_CURRENT_SOURCE_DIR}/src)
 set(DEPS_PATH ${CMAKE_CURRENT_SOURCE_DIR}/deps)
 set(TEST_PATH ${CMAKE_CURRENT_SOURCE_DIR}/tests)
+set(ENABLE_ROARING_TESTS OFF)
 set(CROARING_PATH ${DEPS_PATH}/CRoaring)
+add_subdirectory(${CROARING_PATH} EXCLUDE_FROM_ALL)
+option(DISABLE_TESTS "disables tests in hiredis" ON)
+option(BUILD_SHARED_LIBS "use shared libraries" OFF)
 set(HIREDIS_PATH ${DEPS_PATH}/hiredis)
-set(SDS_PATH ${DEPS_PATH}/sds)
-#set(CMAKE_C_FLAGS_RELEASE "") # removes -NDEBUG
-set(CMAKE_BUILD_TYPE Debug)
-
-link_directories(${HIREDIS_PATH})
-include_directories(${CROARING_PATH})
-include_directories(${HIREDIS_PATH})
-include_directories(${SDS_PATH})
+add_subdirectory(${HIREDIS_PATH} EXCLUDE_FROM_ALL)
 include_directories(${SRC_PATH})
 enable_testing()
 
-####### unit tests #######
-message(STATUS "Compiling unit tests")
 
-set(UNIT_FILES
-        ${SRC_PATH}/data-structure.c
-        ${SRC_PATH}/roaring.c
-        ${TEST_PATH}/unit.c)
-
-add_executable(unit ${UNIT_FILES})
-target_link_libraries(unit)
+add_executable(unit ${SRC_PATH}/data-structure.c ${TEST_PATH}/unit.c)
+target_link_libraries(unit roaring::roaring)
 add_test(NAME unit_tests COMMAND unit)
 
-####### performance tests #######
-message(STATUS "Compiling performance tests")
-
-set(PERFORMANCE_FILES ${TEST_PATH}/performance.c)
-
-add_executable(performance ${PERFORMANCE_FILES})
-target_link_libraries(performance libhiredis.a)
-target_link_libraries(performance m)
+add_executable(performance ${TEST_PATH}/performance.c)
+set (CROARING_BENCHMARK_DATA_DIR "${CROARING_PATH}/benchmarks/realdata/")
+target_compile_definitions(performance PRIVATE CROARING_BENCHMARK_DATA_DIR="${CROARING_BENCHMARK_DATA_DIR}")
+target_link_libraries(performance  m roaring::roaring hiredis::hiredis)
 add_test(NAME performance_tests COMMAND performance)
 
-####### redis module #######
-message(STATUS "Compiling redis module")
-set(CMAKE_POSITION_INDEPENDENT_CODE ON)
 
-set(SOURCE_FILES ${SRC_PATH}/redis-roaring.c
-        ${SRC_PATH}/data-structure.c
-        ${SDS_PATH}/sds.c)
-add_library(redis-roaring SHARED ${SOURCE_FILES})
-target_link_libraries(redis-roaring)
+set(REDIS_ROARING_SOURCE_FILES ${SRC_PATH}/redis-roaring.c
+        ${SRC_PATH}/data-structure.c)
+add_library(redis-roaring SHARED ${REDIS_ROARING_SOURCE_FILES})
+
+target_link_libraries(redis-roaring roaring::roaring hiredis::hiredis)

src/data-structure.c

@@ -29,21 +29,21 @@ bool bitmap_getbit(const Bitmap* bitmap, uint32_t offset) {
 }
 
 int64_t bitmap_get_nth_element_present(const Bitmap* bitmap, uint64_t n) {
-  roaring_uint32_iterator_t* iterator = roaring_create_iterator(bitmap);
+  roaring_uint32_iterator_t* iterator = roaring_iterator_create(bitmap);
   int64_t element = -1;
   for (uint64_t i = 1; iterator->has_value; i++) {
     if (i == n) {
       element = iterator->current_value;
       break;
     }
-    roaring_advance_uint32_iterator(iterator);
+    roaring_uint32_iterator_advance(iterator);
   }
-  roaring_free_uint32_iterator(iterator);
+  roaring_uint32_iterator_free(iterator);
   return element;
 }
 
 int64_t bitmap_get_nth_element_not_present(const Bitmap* bitmap, uint64_t n) {
-  roaring_uint32_iterator_t* iterator = roaring_create_iterator(bitmap);
+  roaring_uint32_iterator_t* iterator = roaring_iterator_create(bitmap);
   int64_t element = -1;
   int64_t last = -1;
   for (uint64_t i = 1; iterator->has_value; i++) {
@@ -56,9 +56,9 @@ int64_t bitmap_get_nth_element_not_present(const Bitmap* bitmap, uint64_t n) {
       n -= (step - 1);
     }
     last = current;
-    roaring_advance_uint32_iterator(iterator);
+    roaring_uint32_iterator_advance(iterator);
   }
-  roaring_free_uint32_iterator(iterator);
+  roaring_uint32_iterator_free(iterator);
   return element;
 }
 
@@ -138,12 +138,12 @@ char* bitmap_get_bit_array(const Bitmap* bitmap, size_t* size) {
   memset(ans, '0', *size);
   ans[*size] = '\0';
 
-  roaring_uint32_iterator_t* iterator = roaring_create_iterator(bitmap);
+  roaring_uint32_iterator_t* iterator = roaring_iterator_create(bitmap);
   while (iterator->has_value) {
     ans[iterator->current_value] = '1';
-    roaring_advance_uint32_iterator(iterator);
+    roaring_uint32_iterator_advance(iterator);
   }
-  roaring_free_uint32_iterator(iterator);
+  roaring_uint32_iterator_free(iterator);
 
   return ans;
 }

src/redis-roaring.c

@@ -7,7 +7,7 @@
 #define realloc(p, sz) RedisModule_Realloc(p, sz)
 #define free(p) RedisModule_Free(p)
 
-#include "roaring.c"
+#include "roaring.h"
 
 static RedisModuleType* BitmapType;
 

tests/numbersfromtextfiles.h

@@ -0,0 +1,147 @@
+#ifndef BITMAPSFROMTEXTFILES_H_
+#define BITMAPSFROMTEXTFILES_H_
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+#include <dirent.h>
+#include <inttypes.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+/*********************************/
+/********************************
+ * General functions to load up bitmaps from text files.
+ * Except format: comma-separated integers.
+ *******************************/
+/*********************************/
+
+/**
+ * Read the content of a file to a char array. Caller is
+ * responsible for memory de-allocation.
+ * Returns NULL on error.
+ *
+ * (If the individual files are small, this function is
+ * a good idea.)
+ */
+static char *read_file(const char *filename) {
+    FILE *fp = fopen(filename, "r");
+    if (!fp) {
+        printf("Could not open file %s\n", filename);
+        return NULL;
+    }
+
+    fseek(fp, 0, SEEK_END);
+    size_t size = (size_t)ftell(fp);
+    rewind(fp);
+    char *answer = (char *)malloc(size + 1);
+    if (!answer) {
+        fclose(fp);
+        return NULL;
+    }
+    if (fread(answer, size, 1, fp) != 1) {
+        free(answer);
+        return NULL;
+    }
+    answer[size] = '\0';
+    fclose(fp);
+    return answer;
+}
+
+/**
+ * Given a file made of comma-separated integers,
+ * read it all and generate an array of integers.
+ * The caller is responsible for memory de-allocation.
+ */
+static uint32_t *read_integer_file(const char *filename, size_t *howmany) {
+    char *buffer = read_file(filename);
+    if (buffer == NULL) return NULL;
+
+    size_t howmanyints = 1;
+    size_t i1 = 0;
+    for (; buffer[i1] != '\0'; i1++) {
+        if (buffer[i1] == ',') ++howmanyints;
+    }
+
+    uint32_t *answer = (uint32_t *)malloc(howmanyints * sizeof(uint32_t));
+    if (answer == NULL) return NULL;
+    size_t pos = 0;
+    for (size_t i = 0; (i < i1) && (buffer[i] != '\0'); i++) {
+        uint32_t currentint;
+        while ((buffer[i] < '0') || (buffer[i] > '9')) {
+            i++;
+            if (buffer[i] == '\0') goto END;
+        }
+        currentint = (uint32_t)(buffer[i] - '0');
+        i++;
+        for (; (buffer[i] >= '0') && (buffer[i] <= '9'); i++)
+            currentint = currentint * 10 + (uint32_t)(buffer[i] - '0');
+        answer[pos++] = currentint;
+    }
+END:
+    if (pos != howmanyints) {
+        printf("unexpected number of integers! %d %d \n", (int)pos,
+               (int)howmanyints);
+    }
+    *howmany = pos;
+    free(buffer);
+    return answer;
+}
+
+/**
+ * Does the file filename ends with the given extension.
+ */
+static bool hasExtension(const char *filename, const char *extension) {
+    const char *ext = strrchr(filename, '.');
+    return (ext && !strcmp(ext, extension));
+}
+
+/**
+ * read all (count) integer files in a directory. Caller is responsible
+ * for memory de-allocation. In case of error, a NULL is returned.
+ */
+static uint32_t **read_all_integer_files(const char *dirname,
+                                         const char *extension,
+                                         size_t **howmany, size_t *count) {
+    struct dirent **entry_list;
+
+    int c = scandir(dirname, &entry_list, 0, alphasort);
+    if (c < 0) return NULL;
+    size_t truec = 0;
+    for (int i = 0; i < c; i++) {
+        if (hasExtension(entry_list[i]->d_name, extension)) ++truec;
+    }
+    *count = truec;
+    *howmany = (size_t *)malloc(sizeof(size_t) * (*count));
+    uint32_t **answer = (uint32_t **)malloc(sizeof(uint32_t *) * (*count));
+    size_t dirlen = strlen(dirname);
+    char *modifdirname = (char *)dirname;
+    if (modifdirname[dirlen - 1] != '/') {
+        modifdirname = (char *)malloc(dirlen + 2);
+        strcpy(modifdirname, dirname);
+        modifdirname[dirlen] = '/';
+        modifdirname[dirlen + 1] = '\0';
+        dirlen++;
+    }
+    for (size_t i = 0, pos = 0; i < (size_t)c;
+         i++) { /* formerly looped while i < *count */
+        if (!hasExtension(entry_list[i]->d_name, extension)) continue;
+        size_t filelen = strlen(entry_list[i]->d_name);
+        char *fullpath = (char *)malloc(dirlen + filelen + 1);
+        strcpy(fullpath, modifdirname);
+        strcpy(fullpath + dirlen, entry_list[i]->d_name);
+        answer[pos] = read_integer_file(fullpath, &((*howmany)[pos]));
+        pos++;
+        free(fullpath);
+    }
+    if (modifdirname != dirname) {
+        free(modifdirname);
+    }
+    for (int i = 0; i < c; ++i) free(entry_list[i]);
+    free(entry_list);
+    return answer;
+}
+
+#endif /* BITMAPSFROMTEXTFILES_H_ */

tests/performance.c

@@ -3,8 +3,9 @@
 #include <time.h>
 #include <math.h>
 #include <assert.h>
+#include <stdbool.h>
 #include "hiredis.h"
-#include "benchmarks/numbersfromtextfiles.h"
+#include "numbersfromtextfiles.h"
 
 #ifdef __MACH__
 #include <mach/clock.h>
@@ -147,7 +148,9 @@ void statistics_print(Statistics* statistics) {
 int main(int argc, char* argv[]) {
   size_t count;
   size_t* howmany = NULL;
-  uint32_t** numbers = read_all_integer_files("./deps/CRoaring/benchmarks/realdata/census1881",
+  const char* dirbuffer = CROARING_BENCHMARK_DATA_DIR "census1881";
+  printf("dirbuffer %s\n", dirbuffer);
+  uint32_t** numbers = read_all_integer_files(dirbuffer,
                                               ".txt", &howmany, &count);
 
   assert(numbers != NULL);

tests/unit.c

@@ -35,13 +35,13 @@ int main(int argc, char* argv[]) {
 
     Bitmap* bitmaps[] = {sixteen, four, nine};
     Bitmap* or = bitmap_or(sizeof(bitmaps) / sizeof(*bitmaps), (const Bitmap**) bitmaps);
-    roaring_uint32_iterator_t* iterator = roaring_create_iterator(or);
+    roaring_uint32_iterator_t* iterator = roaring_iterator_create(or);
     int expected[] = {0, 2, 3, 4};
     for (int i = 0; iterator->has_value; i++) {
       assert(iterator->current_value == expected[i]);
-      roaring_advance_uint32_iterator(iterator);
+      roaring_uint32_iterator_advance(iterator);
     }
-    roaring_free_uint32_iterator(iterator);
+    roaring_uint32_iterator_free(iterator);
 
     bitmap_free(or);
 
@@ -63,13 +63,13 @@ int main(int argc, char* argv[]) {
 
     Bitmap* bitmaps[] = {twelve, four, six};
     Bitmap* and = bitmap_and(sizeof(bitmaps) / sizeof(*bitmaps), (const Bitmap**) bitmaps);
-    roaring_uint32_iterator_t* iterator = roaring_create_iterator(and);
+    roaring_uint32_iterator_t* iterator = roaring_iterator_create(and);
     int expected[] = {2};
     for (int i = 0; iterator->has_value; i++) {
       assert(iterator->current_value == expected[i]);
-      roaring_advance_uint32_iterator(iterator);
+      roaring_uint32_iterator_advance(iterator);
     }
-    roaring_free_uint32_iterator(iterator);
+    roaring_uint32_iterator_free(iterator);
 
     bitmap_free(and);
 
@@ -91,13 +91,13 @@ int main(int argc, char* argv[]) {
 
     Bitmap* bitmaps[] = {twelve, four, six};
     Bitmap* xor = bitmap_xor(sizeof(bitmaps) / sizeof(*bitmaps), (const Bitmap**) bitmaps);
-    roaring_uint32_iterator_t* iterator = roaring_create_iterator(xor);
+    roaring_uint32_iterator_t* iterator = roaring_iterator_create(xor);
     int expected[] = {1, 2, 3};
     for (int i = 0; iterator->has_value; i++) {
       assert(iterator->current_value == expected[i]);
-      roaring_advance_uint32_iterator(iterator);
+      roaring_uint32_iterator_advance(iterator);
     }
-    roaring_free_uint32_iterator(iterator);
+    roaring_uint32_iterator_free(iterator);
 
     bitmap_free(xor);
 
@@ -118,20 +118,20 @@ int main(int argc, char* argv[]) {
     int expected[] = {0, 1};
 
     {
-      roaring_uint32_iterator_t* iterator = roaring_create_iterator(not_array);
+      roaring_uint32_iterator_t* iterator = roaring_iterator_create(not_array);
       for (int i = 0; iterator->has_value; i++) {
         assert(iterator->current_value == expected[i]);
-        roaring_advance_uint32_iterator(iterator);
+        roaring_uint32_iterator_advance(iterator);
       }
-      roaring_free_uint32_iterator(iterator);
+      roaring_uint32_iterator_free(iterator);
     }
     {
-      roaring_uint32_iterator_t* iterator = roaring_create_iterator(not);
+      roaring_uint32_iterator_t* iterator = roaring_iterator_create(not);
       for (int i = 0; iterator->has_value; i++) {
         assert(iterator->current_value == expected[i]);
-        roaring_advance_uint32_iterator(iterator);
+        roaring_uint32_iterator_advance(iterator);
       }
-      roaring_free_uint32_iterator(iterator);
+      roaring_uint32_iterator_free(iterator);
     }
 
     bitmap_free(not);