From 2460988260d26ea15614b8782ceac608b1c370e8 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Wed, 5 Jun 2024 10:29:13 -0400 Subject: [PATCH 01/12] fix: introduce macro to initialize to zero structs portably (#634) * fix: introduce macro to initialize to zero structs portably * lint --- include/roaring/portability.h | 10 ++++++++++ src/art/art.c | 6 +++--- src/roaring.c | 6 +++--- src/roaring64.c | 6 +++--- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/include/roaring/portability.h b/include/roaring/portability.h index 74b0870ab..48e469d3d 100644 --- a/include/roaring/portability.h +++ b/include/roaring/portability.h @@ -589,6 +589,16 @@ static inline uint32_t croaring_refcount_get(const croaring_refcount_t *val) { #define CROARING_DEPRECATED #endif // defined(__GNUC__) || defined(__clang__) +// We want to initialize structs to zero portably (C and C++), without +// warnings. We can do mystruct s = CROARING_ZERO_INITIALIZER; +#if __cplusplus +#define CROARING_ZERO_INITIALIZER \ + {} +#else +#define CROARING_ZERO_INITIALIZER \ + { 0 } +#endif + // We need portability.h to be included first, // but we also always want isadetection.h to be // included (right after). diff --git a/src/art/art.c b/src/art/art.c index 135cee498..835699146 100644 --- a/src/art/art.c +++ b/src/art/art.c @@ -1670,7 +1670,7 @@ static bool art_node_iterator_lower_bound(const art_node_t *node, } art_iterator_t art_init_iterator(const art_t *art, bool first) { - art_iterator_t iterator = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + art_iterator_t iterator = CROARING_ZERO_INITIALIZER; if (art->root == NULL) { return iterator; } @@ -1727,7 +1727,7 @@ bool art_iterator_lower_bound(art_iterator_t *iterator, } art_iterator_t art_lower_bound(const art_t *art, const art_key_chunk_t *key) { - art_iterator_t iterator = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + art_iterator_t iterator = CROARING_ZERO_INITIALIZER; if (art->root != NULL) { art_node_iterator_lower_bound(art->root, &iterator, key); } @@ -1735,7 +1735,7 @@ art_iterator_t art_lower_bound(const art_t *art, const art_key_chunk_t *key) { } art_iterator_t art_upper_bound(const art_t *art, const art_key_chunk_t *key) { - art_iterator_t iterator = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + art_iterator_t iterator = CROARING_ZERO_INITIALIZER; if (art->root != NULL) { if (art_node_iterator_lower_bound(art->root, &iterator, key) && art_compare_keys(iterator.key, key) == 0) { diff --git a/src/roaring.c b/src/roaring.c index 9b7ad8002..e3847bae9 100644 --- a/src/roaring.c +++ b/src/roaring.c @@ -199,7 +199,7 @@ roaring_bitmap_t *roaring_bitmap_of(size_t n_args, ...) { // todo: could be greatly optimized but we do not expect this call to ever // include long lists roaring_bitmap_t *answer = roaring_bitmap_create(); - roaring_bulk_context_t context = {0, 0, 0, 0}; + roaring_bulk_context_t context = CROARING_ZERO_INITIALIZER; va_list ap; va_start(ap, n_args); for (size_t i = 0; i < n_args; i++) { @@ -1541,7 +1541,7 @@ roaring_bitmap_t *roaring_bitmap_deserialize(const void *buf) { if (bitmap == NULL) { return NULL; } - roaring_bulk_context_t context = {0, 0, 0, 0}; + roaring_bulk_context_t context = CROARING_ZERO_INITIALIZER; for (uint32_t i = 0; i < card; i++) { // elems may not be aligned, read with memcpy uint32_t elem; @@ -1584,7 +1584,7 @@ roaring_bitmap_t *roaring_bitmap_deserialize_safe(const void *buf, if (bitmap == NULL) { return NULL; } - roaring_bulk_context_t context = {0, 0, 0, 0}; + roaring_bulk_context_t context = CROARING_ZERO_INITIALIZER; for (uint32_t i = 0; i < card; i++) { // elems may not be aligned, read with memcpy uint32_t elem; diff --git a/src/roaring64.c b/src/roaring64.c index 328b0eacb..d41507b3c 100644 --- a/src/roaring64.c +++ b/src/roaring64.c @@ -227,7 +227,7 @@ roaring64_bitmap_t *roaring64_bitmap_of_ptr(size_t n_args, roaring64_bitmap_t *roaring64_bitmap_of(size_t n_args, ...) { roaring64_bitmap_t *r = roaring64_bitmap_create(); - roaring64_bulk_context_t context = {0, 0, 0, 0, 0, 0, 0}; + roaring64_bulk_context_t context = CROARING_ZERO_INITIALIZER; va_list ap; va_start(ap, n_args); for (size_t i = 0; i < n_args; i++) { @@ -320,7 +320,7 @@ void roaring64_bitmap_add_many(roaring64_bitmap_t *r, size_t n_args, return; } const uint64_t *end = vals + n_args; - roaring64_bulk_context_t context = {0, 0, 0, 0, 0, 0, 0}; + roaring64_bulk_context_t context = CROARING_ZERO_INITIALIZER; for (const uint64_t *current_val = vals; current_val != end; current_val++) { roaring64_bitmap_add_bulk(r, &context, *current_val); @@ -644,7 +644,7 @@ void roaring64_bitmap_remove_many(roaring64_bitmap_t *r, size_t n_args, return; } const uint64_t *end = vals + n_args; - roaring64_bulk_context_t context = {0, 0, 0, 0, 0, 0, 0}; + roaring64_bulk_context_t context = CROARING_ZERO_INITIALIZER; for (const uint64_t *current_val = vals; current_val != end; current_val++) { roaring64_bitmap_remove_bulk(r, &context, *current_val); From 5da0593dc07f8ebf47879c11e51dca4a980a8f81 Mon Sep 17 00:00:00 2001 From: Zachary Dremann Date: Thu, 27 Jun 2024 09:17:12 -0400 Subject: [PATCH 02/12] Mark `sum_value` in roaring_statistics_t as deprecated (#639) --- amalgamation.sh | 2 +- include/roaring/roaring_types.h | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/amalgamation.sh b/amalgamation.sh index c17524aaf..bee927ab1 100755 --- a/amalgamation.sh +++ b/amalgamation.sh @@ -36,8 +36,8 @@ DEMOCPP="amalgamation_demo.cpp" # ALL_PUBLIC_H=" $SCRIPTPATH/include/roaring/roaring_version.h -$SCRIPTPATH/include/roaring/roaring_types.h $SCRIPTPATH/include/roaring/portability.h +$SCRIPTPATH/include/roaring/roaring_types.h $SCRIPTPATH/include/roaring/bitset/bitset.h $SCRIPTPATH/include/roaring/roaring.h $SCRIPTPATH/include/roaring/memory.h diff --git a/include/roaring/roaring_types.h b/include/roaring/roaring_types.h index 88f354a6b..3a8a61894 100644 --- a/include/roaring/roaring_types.h +++ b/include/roaring/roaring_types.h @@ -8,6 +8,8 @@ #include #include +#include + #ifdef __cplusplus extern "C" { namespace roaring { @@ -89,6 +91,8 @@ typedef struct roaring_statistics_s { max_value; /* the maximal value, undefined if cardinality is zero */ uint32_t min_value; /* the minimal value, undefined if cardinality is zero */ + + CROARING_DEPRECATED uint64_t sum_value; /* deprecated always zero */ uint64_t cardinality; /* total number of values stored in the bitmap */ From 9526ecf2edafdf5b61635763e84e7ef30eaff62b Mon Sep 17 00:00:00 2001 From: Zachary Dremann Date: Wed, 3 Jul 2024 17:29:14 -0400 Subject: [PATCH 03/12] Fix segfault with roaring64 intersect_with_range and empty bitmap (#636) * Add unit tests for lower bound of an empty bitmap64 * art_iterator_lower_bound works on empty bitmap64 When we reset the iterator to the root, check if the root is null --- src/art/art.c | 7 +++++-- tests/art_unit.cpp | 10 ++++++++++ tests/roaring64_unit.cpp | 18 +++++++++++++++++- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/art/art.c b/src/art/art.c index 835699146..dc2f17a5d 100644 --- a/src/art/art.c +++ b/src/art/art.c @@ -1693,8 +1693,11 @@ bool art_iterator_lower_bound(art_iterator_t *iterator, // a valid key. Start from the root. iterator->frame = 0; iterator->depth = 0; - return art_node_iterator_lower_bound(art_iterator_node(iterator), - iterator, key); + art_node_t *root = art_iterator_node(iterator); + if (root == NULL) { + return false; + } + return art_node_iterator_lower_bound(root, iterator, key); } int compare_result = art_compare_prefix(iterator->key, 0, key, 0, ART_KEY_BYTES); diff --git a/tests/art_unit.cpp b/tests/art_unit.cpp index f5fdcc6a4..06d647492 100644 --- a/tests/art_unit.cpp +++ b/tests/art_unit.cpp @@ -319,6 +319,16 @@ DEFINE_TEST(test_art_iterator_prev) { } DEFINE_TEST(test_art_iterator_lower_bound) { + { + art_t art{NULL}; + art_iterator_t iterator = art_init_iterator(&art, true); + assert_null(iterator.value); + assert_false( + art_iterator_lower_bound(&iterator, (art_key_chunk_t*)"000000")); + assert_false( + art_iterator_lower_bound(&iterator, (art_key_chunk_t*)"000001")); + art_free(&art); + } { std::vector keys = { "000001", "000002", "000003", "000004", "001005", diff --git a/tests/roaring64_unit.cpp b/tests/roaring64_unit.cpp index cad17d46a..f6129abc3 100644 --- a/tests/roaring64_unit.cpp +++ b/tests/roaring64_unit.cpp @@ -1053,6 +1053,11 @@ DEFINE_TEST(test_intersect) { DEFINE_TEST(test_intersect_with_range) { roaring64_bitmap_t* r = roaring64_bitmap_create(); + // Empty bitmap never intersects + assert_false(roaring64_bitmap_intersect_with_range(r, 0, 0)); + assert_false(roaring64_bitmap_intersect_with_range(r, 0, 50000)); + assert_false(roaring64_bitmap_intersect_with_range(r, 0, UINT64_MAX)); + roaring64_bitmap_add(r, 50000); roaring64_bitmap_add(r, 100000); roaring64_bitmap_add(r, 100001); @@ -1065,6 +1070,13 @@ DEFINE_TEST(test_intersect_with_range) { assert_true(roaring64_bitmap_intersect_with_range(r, 50001, 100001)); assert_false(roaring64_bitmap_intersect_with_range(r, 300001, UINT64_MAX)); + // Empty ranges never intersect + assert_false(roaring64_bitmap_intersect_with_range(r, 0, 0)); + assert_false( + roaring64_bitmap_intersect_with_range(r, UINT64_MAX, UINT64_MAX)); + assert_false(roaring64_bitmap_intersect_with_range(r, UINT64_MAX, 0)); + assert_false(roaring64_bitmap_intersect_with_range(r, 50000, 50000)); + roaring64_bitmap_free(r); } @@ -1674,13 +1686,17 @@ DEFINE_TEST(test_iterator_previous) { DEFINE_TEST(test_iterator_move_equalorlarger) { roaring64_bitmap_t* r = roaring64_bitmap_create(); + roaring64_iterator_t* it = roaring64_iterator_create(r); + assert_false(roaring64_iterator_move_equalorlarger(it, 0)); + assert_false(roaring64_iterator_move_equalorlarger(it, UINT64_MAX)); + roaring64_bitmap_add(r, 0); roaring64_bitmap_add(r, 1ULL << 35); roaring64_bitmap_add(r, (1ULL << 35) + 1); roaring64_bitmap_add(r, (1ULL << 35) + 2); roaring64_bitmap_add(r, (1ULL << 36)); - roaring64_iterator_t* it = roaring64_iterator_create(r); + roaring64_iterator_reinit(r, it); assert_true(roaring64_iterator_move_equalorlarger(it, 0)); assert_true(roaring64_iterator_has_value(it)); assert_int_equal(roaring64_iterator_value(it), 0); From 76a2cbb2fc2ebfb5998820605d023d6b6709926b Mon Sep 17 00:00:00 2001 From: PerseoGI Date: Thu, 4 Jul 2024 14:38:51 +0200 Subject: [PATCH 04/12] Update README.md with up2date conan instructions (#640) --- README.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 4e8afc646..c884c438a 100644 --- a/README.md +++ b/README.md @@ -749,13 +749,17 @@ We have optimizations specific to AVX2 and AVX-512 in the code, and they are tur ## Usage (Using `conan`) -You can install the library using the conan package manager: +You can install pre-built binaries for `roaring` or build it from source using [Conan](https://conan.io/). Use the following command to install latest version: ``` -$ echo -e "[requires]\nroaring/0.3.3" > conanfile.txt -$ conan install . +conan install --requires="roaring/[*]" --build=missing ``` +For detailed instructions on how to use Conan, please refer to the [Conan documentation](https://docs.conan.io/2/). + +The `roaring` Conan recipe is kept up to date by Conan maintainers and community contributors. +If the version is out of date, please [create an issue or pull request](https://github.com/conan-io/conan-center-index) on the ConanCenterIndex repository. + ## Usage (Using `vcpkg` on Windows, Linux and macOS) From 8bcc0effd9dbbe8537d712810715243b2d904613 Mon Sep 17 00:00:00 2001 From: Zachary Dremann Date: Mon, 8 Jul 2024 08:48:57 -0400 Subject: [PATCH 05/12] Only ignore amalgamation files at the root (#641) Currently, the gitignore tries to ignore src/roaring.c This actually only works in gitignore because git only considers the gitignore when it would _add_ a file to be tracked, changes to existing files already in git are tracked regardless of what's in .gitignore. Fixes #637 --- .gitignore | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index e68e3fbb6..9f3ef4756 100644 --- a/.gitignore +++ b/.gitignore @@ -321,11 +321,12 @@ paket-files/ __pycache__/ *.pyc -amalgamation_demo -amalgamation_demo.c -amalgamation_demo.cpp -roaring.c -roaring.h -roaring.hh +# Exclude amalgamation created files +/amalgamation_demo +/amalgamation_demo.c +/amalgamation_demo.cpp +/roaring.c +/roaring.h +/roaring.hh #############END VISUAL STUDIO############## From 065f4878e1d677fb213f4355766b0099d3a20803 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Mon, 22 Jul 2024 16:35:57 -0400 Subject: [PATCH 06/12] adding more run container benchmarking --- benchmarks/run_container_benchmark.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/benchmarks/run_container_benchmark.c b/benchmarks/run_container_benchmark.c index 662da59e3..aa3cb7e65 100644 --- a/benchmarks/run_container_benchmark.c +++ b/benchmarks/run_container_benchmark.c @@ -109,6 +109,31 @@ int main() { free(testvalues); run_container_free(Bt); } + + printf("==dense range test \n"); + for (int howmany = 32; howmany <= (1 << 16); howmany *= 8) { + run_container_t* Bt = run_container_create(); + for (int j = 0; j < howmany; ++j) { + uint16_t min = (uint16_t)pcg32_random() % 4096; + uint16_t max = min + 4096; + int32_t nruns_greater = + rle16_count_greater(Bt->runs, Bt->n_runs, max); + int32_t nruns_less = + rle16_count_less(Bt->runs, Bt->n_runs - nruns_greater, min); + run_container_add_range_nruns(Bt, min, max, nruns_less, + nruns_greater); + } + printf("\n number of values in container = %d\n", + run_container_cardinality(Bt)); + int card = run_container_cardinality(Bt); + uint32_t* out = malloc(sizeof(uint32_t) * (unsigned long)card); + BEST_TIME(run_container_to_uint32_array(out, Bt, 1234), card, repeat, + card); + free(out); + + run_container_free(Bt); + } + printf("\n"); run_container_t* B1 = run_container_create(); @@ -170,4 +195,4 @@ int main() { run_container_free(B2); run_container_free(BO); return 0; -} +} \ No newline at end of file From e326af3b1af3b7655cde0650630f1d3c73e74b54 Mon Sep 17 00:00:00 2001 From: stdpain <34912776+stdpain@users.noreply.github.com> Date: Tue, 23 Jul 2024 04:48:55 +0800 Subject: [PATCH 07/12] support AVX2 for run_container_to_uint32_array (#642) * support AVX2 for run_container_to_uint32_array 1. support AVX for run_container_to_uint32_array 2. add dense range for run container baseline ``` number of values in container = 256 run_container_to_uint32_array(out, Bt, 1234): 3.64 cycles per operation number of values in container = 2018 run_container_to_uint32_array(out, Bt, 1234): 3.07 cycles per operation number of values in container = 14498 run_container_to_uint32_array(out, Bt, 1234): 3.47 cycles per operation number of values in container = 7826 run_container_to_uint32_array(out, Bt, 1234): 0.18 cycles per operation number of values in container = 8152 run_container_to_uint32_array(out, Bt, 1234): 0.18 cycles per operation number of values in container = 8189 run_container_to_uint32_array(out, Bt, 1234): 0.18 cycles per operation number of values in container = 8191 run_container_to_uint32_array(out, Bt, 1234): 0.18 cycles per operation ``` AVX2 version: ``` number of values in container = 256 run_container_to_uint32_array(out, Bt, 1234): 4.38 cycles per operation number of values in container = 2018 run_container_to_uint32_array(out, Bt, 1234): 3.77 cycles per operation number of values in container = 14498 run_container_to_uint32_array(out, Bt, 1234): 4.19 cycles per operation number of values in container = 7826 run_container_to_uint32_array(out, Bt, 1234): 0.10 cycles per operation number of values in container = 8152 run_container_to_uint32_array(out, Bt, 1234): 0.10 cycles per operation number of values in container = 8189 run_container_to_uint32_array(out, Bt, 1234): 0.10 cycles per operation number of values in container = 8191 run_container_to_uint32_array(out, Bt, 1234): 0.10 cycles per operation ``` SIMD version works well on dense case. However, if the length of each runs is small, a single operation will have an if additional overhead. * avoid regression when run length is small --- src/containers/run.c | 106 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 88 insertions(+), 18 deletions(-) diff --git a/src/containers/run.c b/src/containers/run.c index 0d6472643..1da4fc140 100644 --- a/src/containers/run.c +++ b/src/containers/run.c @@ -636,24 +636,6 @@ void run_container_andnot(const run_container_t *src_1, } } -ALLOW_UNALIGNED -int run_container_to_uint32_array(void *vout, const run_container_t *cont, - uint32_t base) { - int outpos = 0; - uint32_t *out = (uint32_t *)vout; - for (int i = 0; i < cont->n_runs; ++i) { - uint32_t run_start = base + cont->runs[i].value; - uint16_t le = cont->runs[i].length; - for (int j = 0; j <= le; ++j) { - uint32_t val = run_start + j; - memcpy(out + outpos, &val, - sizeof(uint32_t)); // should be compiled as a MOV on x64 - outpos++; - } - } - return outpos; -} - /* * Print this container using printf (useful for debugging). */ @@ -1026,6 +1008,47 @@ static inline int _avx2_run_container_cardinality(const run_container_t *run) { return sum; } +ALLOW_UNALIGNED +int _avx2_run_container_to_uint32_array(void *vout, const run_container_t *cont, + uint32_t base) { + int outpos = 0; + uint32_t *out = (uint32_t *)vout; + + for (int i = 0; i < cont->n_runs; ++i) { + uint32_t run_start = base + cont->runs[i].value; + uint16_t le = cont->runs[i].length; + if (le < 8) { + for (int j = 0; j <= le; ++j) { + uint32_t val = run_start + j; + memcpy(out + outpos, &val, + sizeof(uint32_t)); // should be compiled as a MOV on x64 + outpos++; + } + } else { + int j = 0; + __m256i run_start_v = _mm256_set1_epi32(run_start); + // [8,8,8,8....] + __m256i inc = _mm256_set1_epi32(8); + // used for generate sequence: + // [0, 1, 2, 3...], [8, 9, 10,...] + __m256i delta = _mm256_setr_epi32(0, 1, 2, 3, 4, 5, 6, 7); + for (j = 0; j + 8 <= le; j += 8) { + __m256i val_v = _mm256_add_epi32(run_start_v, delta); + _mm256_storeu_si256((__m256i *)(out + outpos), val_v); + delta = _mm256_add_epi32(inc, delta); + outpos += 8; + } + for (; j <= le; ++j) { + uint32_t val = run_start + j; + memcpy(out + outpos, &val, + sizeof(uint32_t)); // should be compiled as a MOV on x64 + outpos++; + } + } + } + return outpos; +} + CROARING_UNTARGET_AVX2 /* Get the cardinality of `run'. Requires an actual computation. */ @@ -1055,6 +1078,34 @@ int run_container_cardinality(const run_container_t *run) { return _scalar_run_container_cardinality(run); } } + +int _scalar_run_container_to_uint32_array(void *vout, + const run_container_t *cont, + uint32_t base) { + int outpos = 0; + uint32_t *out = (uint32_t *)vout; + for (int i = 0; i < cont->n_runs; ++i) { + uint32_t run_start = base + cont->runs[i].value; + uint16_t le = cont->runs[i].length; + for (int j = 0; j <= le; ++j) { + uint32_t val = run_start + j; + memcpy(out + outpos, &val, + sizeof(uint32_t)); // should be compiled as a MOV on x64 + outpos++; + } + } + return outpos; +} + +int run_container_to_uint32_array(void *vout, const run_container_t *cont, + uint32_t base) { + if (croaring_hardware_support() & ROARING_SUPPORTS_AVX2) { + return _avx2_run_container_to_uint32_array(vout, cont, base); + } else { + return _scalar_run_container_to_uint32_array(vout, cont, base); + } +} + #else /* Get the cardinality of `run'. Requires an actual computation. */ @@ -1071,6 +1122,25 @@ int run_container_cardinality(const run_container_t *run) { return sum; } + +ALLOW_UNALIGNED +int run_container_to_uint32_array(void *vout, const run_container_t *cont, + uint32_t base) { + int outpos = 0; + uint32_t *out = (uint32_t *)vout; + for (int i = 0; i < cont->n_runs; ++i) { + uint32_t run_start = base + cont->runs[i].value; + uint16_t le = cont->runs[i].length; + for (int j = 0; j <= le; ++j) { + uint32_t val = run_start + j; + memcpy(out + outpos, &val, + sizeof(uint32_t)); // should be compiled as a MOV on x64 + outpos++; + } + } + return outpos; +} + #endif #ifdef __cplusplus From ac8639c5c088e01dfbb94c0278d19e852b6c3fcb Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Mon, 29 Jul 2024 21:14:30 -0400 Subject: [PATCH 08/12] reenabling avx under vs (#644) --- src/isadetection.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/isadetection.c b/src/isadetection.c index d7904149b..4cc43197c 100644 --- a/src/isadetection.c +++ b/src/isadetection.c @@ -48,15 +48,15 @@ POSSIBILITY OF SUCH DAMAGE. #include #include -// Binaries produced by Visual Studio with solely AVX2 routines +// Binaries produced by Visual Studio 19.38 with solely AVX2 routines // can compile to AVX-512 thus causing crashes on non-AVX-512 systems. // This appears to affect VS 17.8 and 17.9. We disable AVX-512 and AVX2 // on these systems. It seems that ClangCL is not affected. // https://github.com/RoaringBitmap/CRoaring/pull/603 #ifndef __clang__ -#if _MSC_VER >= 1938 +#if _MSC_VER == 1938 #define ROARING_DISABLE_AVX 1 -#endif // _MSC_VER >= 1938 +#endif // _MSC_VER == 1938 #endif // __clang__ // We need portability.h to be included first, see From 8cc603f9df391c488949c1247c540767ed7bb243 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Tue, 30 Jul 2024 09:28:07 -0400 Subject: [PATCH 09/12] fix: remove null checks before free. Reverses commit eb5395b (#643) * fix: remove null checks before free. Reverses commit eb5395b * completing the null removal * updating readme * adding back some checks. * lint --- README.md | 15 +++++++++++++++ src/containers/array.c | 12 +++--------- src/containers/bitset.c | 7 ++----- src/containers/run.c | 12 +++--------- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index c884c438a..f582d3115 100644 --- a/README.md +++ b/README.md @@ -303,6 +303,21 @@ int main(){ } ``` +By default we use: +```C +static roaring_memory_t global_memory_hook = { + .malloc = malloc, + .realloc = realloc, + .calloc = calloc, + .free = free, + .aligned_malloc = roaring_bitmap_aligned_malloc, + .aligned_free = roaring_bitmap_aligned_free, +}; +``` + +We require that the `free`/`aligned_free` functions follow the C +convention where `free(NULL)`/`aligned_free(NULL)` have no effect. + # Example (C) diff --git a/src/containers/array.c b/src/containers/array.c index 0a24482a3..4b5e5bc53 100644 --- a/src/containers/array.c +++ b/src/containers/array.c @@ -145,11 +145,8 @@ int array_container_shrink_to_fit(array_container_t *src) { /* Free memory. */ void array_container_free(array_container_t *arr) { - if (arr->array != - NULL) { // Jon Strabala reports that some tools complain otherwise - roaring_free(arr->array); - arr->array = NULL; // pedantic - } + if (arr == NULL) return; + roaring_free(arr->array); roaring_free(arr); } @@ -177,10 +174,7 @@ void array_container_grow(array_container_t *container, int32_t min, (uint16_t *)roaring_realloc(array, new_capacity * sizeof(uint16_t)); if (container->array == NULL) roaring_free(array); } else { - // Jon Strabala reports that some tools complain otherwise - if (array != NULL) { - roaring_free(array); - } + roaring_free(array); container->array = (uint16_t *)roaring_malloc(new_capacity * sizeof(uint16_t)); } diff --git a/src/containers/bitset.c b/src/containers/bitset.c index f49739594..7b84af82e 100644 --- a/src/containers/bitset.c +++ b/src/containers/bitset.c @@ -130,11 +130,8 @@ void bitset_container_add_from_range(bitset_container_t *bitset, uint32_t min, /* Free memory. */ void bitset_container_free(bitset_container_t *bitset) { - if (bitset->words != - NULL) { // Jon Strabala reports that some tools complain otherwise - roaring_aligned_free(bitset->words); - bitset->words = NULL; // pedantic - } + if (bitset == NULL) return; + roaring_aligned_free(bitset->words); roaring_free(bitset); } diff --git a/src/containers/run.c b/src/containers/run.c index 1da4fc140..986db6d1a 100644 --- a/src/containers/run.c +++ b/src/containers/run.c @@ -189,11 +189,8 @@ void run_container_offset(const run_container_t *c, container_t **loc, /* Free memory. */ void run_container_free(run_container_t *run) { - if (run->runs != - NULL) { // Jon Strabala reports that some tools complain otherwise - roaring_free(run->runs); - run->runs = NULL; // pedantic - } + if (run == NULL) return; + roaring_free(run->runs); roaring_free(run); } @@ -211,10 +208,7 @@ void run_container_grow(run_container_t *run, int32_t min, bool copy) { run->capacity * sizeof(rle16_t)); if (run->runs == NULL) roaring_free(oldruns); } else { - // Jon Strabala reports that some tools complain otherwise - if (run->runs != NULL) { - roaring_free(run->runs); - } + roaring_free(run->runs); run->runs = (rle16_t *)roaring_malloc(run->capacity * sizeof(rle16_t)); } // We may have run->runs == NULL. From 11c1198c02e246e9d57eaaeed022920a431dc645 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Tue, 30 Jul 2024 09:31:00 -0400 Subject: [PATCH 10/12] release --- CMakeLists.txt | 6 +++--- doxygen | 2 +- include/roaring/roaring_version.h | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 32a6127f3..1203e5fd7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,9 +19,9 @@ if(CMAKE_C_COMPILER_ID STREQUAL "AppleClang" AND CMAKE_C_COMPILER_VERSION VERSIO endif() set(ROARING_LIB_NAME roaring) set(PROJECT_VERSION_MAJOR 4) -set(PROJECT_VERSION_MINOR 0) -set(PROJECT_VERSION_PATCH 0) -set(ROARING_LIB_VERSION "4.0.0" CACHE STRING "Roaring library version") +set(PROJECT_VERSION_MINOR 1) +set(PROJECT_VERSION_PATCH 1) +set(ROARING_LIB_VERSION "4.1.1" CACHE STRING "Roaring library version") set(ROARING_LIB_SOVERSION "16" CACHE STRING "Roaring library soversion") option(ROARING_EXCEPTIONS "Enable exception-throwing interface" ON) diff --git a/doxygen b/doxygen index 446212313..444ad8afb 100644 --- a/doxygen +++ b/doxygen @@ -48,7 +48,7 @@ PROJECT_NAME = "CRoaring" # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = "4.0.0" +PROJECT_NUMBER = "4.1.1" # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a diff --git a/include/roaring/roaring_version.h b/include/roaring/roaring_version.h index 719630048..3f7519449 100644 --- a/include/roaring/roaring_version.h +++ b/include/roaring/roaring_version.h @@ -2,11 +2,11 @@ // /include/roaring/roaring_version.h automatically generated by release.py, do not change by hand #ifndef ROARING_INCLUDE_ROARING_VERSION #define ROARING_INCLUDE_ROARING_VERSION -#define ROARING_VERSION "4.0.0" +#define ROARING_VERSION "4.1.1" enum { ROARING_VERSION_MAJOR = 4, - ROARING_VERSION_MINOR = 0, - ROARING_VERSION_REVISION = 0 + ROARING_VERSION_MINOR = 1, + ROARING_VERSION_REVISION = 1 }; #endif // ROARING_INCLUDE_ROARING_VERSION // clang-format on \ No newline at end of file From 6c39369251e59b1c79cbfc338e10c231f1221494 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Tue, 30 Jul 2024 15:44:27 -0400 Subject: [PATCH 11/12] automating amal --- tools/release.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/release.py b/tools/release.py index b9e054f94..0fa36031e 100755 --- a/tools/release.py +++ b/tools/release.py @@ -98,8 +98,10 @@ def topaddedversionstring(major, minor, rev): print(versionfile + " modified") - +from pathlib import Path scriptlocation = os.path.dirname(os.path.abspath(__file__)) +projectlocation = Path(os.path.abspath(__file__)).parent.absolute() +amal_path = os.path.join(projectlocation, "amalgamation.sh") import fileinput @@ -142,6 +144,8 @@ def topaddedversionstring(major, minor, rev): for line in fileinput.input(doxygenfile, inplace=1, backup='.bak'): line = re.sub(r'PROJECT_NUMBER = "\d+\.\d+\.\d+','PROJECT_NUMBER = "'+newversionstring, line.rstrip()) print(line) - +ret = subprocess.call(["bash", amal_path]) +if(ret != 0): + print("failed to update amalgamation") print("Please run the tests before issuing a release: "+scriptlocation + "/prereleasetests.sh \n") print("to issue release, enter \n git commit -a \n git push \n git tag -a v"+toversionstring(*newversion)+" -m \"version "+toversionstring(*newversion)+"\"\n git push --tags \n") From 0268464d60467e6595026bd758e67ed03bb30cc2 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Tue, 30 Jul 2024 15:47:20 -0400 Subject: [PATCH 12/12] amal. automation --- tools/release.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/release.py b/tools/release.py index 0fa36031e..2db22eb30 100755 --- a/tools/release.py +++ b/tools/release.py @@ -100,7 +100,7 @@ def topaddedversionstring(major, minor, rev): print(versionfile + " modified") from pathlib import Path scriptlocation = os.path.dirname(os.path.abspath(__file__)) -projectlocation = Path(os.path.abspath(__file__)).parent.absolute() +projectlocation = Path(os.path.abspath(__file__)).parent.parent.absolute() amal_path = os.path.join(projectlocation, "amalgamation.sh") import fileinput @@ -144,8 +144,10 @@ def topaddedversionstring(major, minor, rev): for line in fileinput.input(doxygenfile, inplace=1, backup='.bak'): line = re.sub(r'PROJECT_NUMBER = "\d+\.\d+\.\d+','PROJECT_NUMBER = "'+newversionstring, line.rstrip()) print(line) -ret = subprocess.call(["bash", amal_path]) -if(ret != 0): - print("failed to update amalgamation") + + + +pipe = subprocess.Popen(["bash", amal_path], stdout=subprocess.PIPE, stderr=subprocess.STDOUT) +print( pipe.communicate()[0].decode().strip() ) print("Please run the tests before issuing a release: "+scriptlocation + "/prereleasetests.sh \n") print("to issue release, enter \n git commit -a \n git push \n git tag -a v"+toversionstring(*newversion)+" -m \"version "+toversionstring(*newversion)+"\"\n git push --tags \n")