From ff9898967d47552fd218083e8ebab34422f948f7 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Tue, 16 Apr 2024 16:05:12 -0400 Subject: [PATCH] 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 --- CMakeLists.txt | 55 +++++-------- deps/CRoaring | 2 +- deps/hiredis | 2 +- deps/redis | 2 +- src/data-structure.c | 18 ++--- src/redis-roaring.c | 2 +- tests/numbersfromtextfiles.h | 147 +++++++++++++++++++++++++++++++++++ tests/performance.c | 7 +- tests/unit.c | 30 +++---- 9 files changed, 199 insertions(+), 66 deletions(-) create mode 100644 tests/numbersfromtextfiles.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 3192983..3e1255a 100644 --- a/CMakeLists.txt +++ b/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) diff --git a/deps/CRoaring b/deps/CRoaring index fd6f6ff..c501a95 160000 --- a/deps/CRoaring +++ b/deps/CRoaring @@ -1 +1 @@ -Subproject commit fd6f6ff423bf1f9d240e23e9d2a900885f809c2c +Subproject commit c501a95206dec8187aa63a053b1f1a8acb943f7c diff --git a/deps/hiredis b/deps/hiredis index 3d8709d..60e5075 160000 --- a/deps/hiredis +++ b/deps/hiredis @@ -1 +1 @@ -Subproject commit 3d8709d19d7fa67d203a33c969e69f0f1a4eab02 +Subproject commit 60e5075d4ac77424809f855ba3e398df7aacefe8 diff --git a/deps/redis b/deps/redis index 58f79e2..d2c8a4b 160000 --- a/deps/redis +++ b/deps/redis @@ -1 +1 @@ -Subproject commit 58f79e2ff48bb20e48d5eb60ff6a87c9afae5fe9 +Subproject commit d2c8a4b91e8c0e6aefd1f5bc0bf582cddbe046b7 diff --git a/src/data-structure.c b/src/data-structure.c index 7c6037c..76e91dd 100644 --- a/src/data-structure.c +++ b/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; } diff --git a/src/redis-roaring.c b/src/redis-roaring.c index 7aa1ef3..be58af2 100644 --- a/src/redis-roaring.c +++ b/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; diff --git a/tests/numbersfromtextfiles.h b/tests/numbersfromtextfiles.h new file mode 100644 index 0000000..61bcd0a --- /dev/null +++ b/tests/numbersfromtextfiles.h @@ -0,0 +1,147 @@ +#ifndef BITMAPSFROMTEXTFILES_H_ +#define BITMAPSFROMTEXTFILES_H_ +#ifndef _GNU_SOURCE +#define _GNU_SOURCE +#endif +#include +#include +#include +#include +#include +#include +#include + +/*********************************/ +/******************************** + * 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_ */ \ No newline at end of file diff --git a/tests/performance.c b/tests/performance.c index 4a03c70..6035d9c 100644 --- a/tests/performance.c +++ b/tests/performance.c @@ -3,8 +3,9 @@ #include #include #include +#include #include "hiredis.h" -#include "benchmarks/numbersfromtextfiles.h" +#include "numbersfromtextfiles.h" #ifdef __MACH__ #include @@ -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); diff --git a/tests/unit.c b/tests/unit.c index 6151b34..7a7429e 100644 --- a/tests/unit.c +++ b/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);