From 0a14a7a1daadaba5361b80458a248065acfb465d Mon Sep 17 00:00:00 2001 From: Anton Angelov <16073265+antonangeloff@users.noreply.github.com> Date: Thu, 4 Dec 2025 08:44:19 +0200 Subject: [PATCH] Improve performance of `rz_bv_set_range()` (#5562) * Add microbenchmarks * New implementation for rz_bv_set_range() * Remove rz_ prefix from static function names --- librz/util/bitvector.c | 50 ++++++++++++++++++++++++++++-------- test/bench/bench_bitvector.c | 22 ++++++++++++++++ test/unit/test_bitvector.c | 46 +++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 11 deletions(-) diff --git a/librz/util/bitvector.c b/librz/util/bitvector.c index f4983df4a3..523c695412 100644 --- a/librz/util/bitvector.c +++ b/librz/util/bitvector.c @@ -203,7 +203,7 @@ RZ_API ut32 rz_bv_copy(RZ_NONNULL const RzBitVector *src, RZ_NONNULL RzBitVector /** * \brief Optimized version of rz_bv_copy_nbits() for large bitvectors (more than 64 bits) with bit positions aligned to BV_ELEM_SIZE */ -static ut32 rz_bv_copy_nbits_large_aligned(const RzBitVector *src, ut32 src_start_pos, RzBitVector *dst, ut32 dst_start_pos, ut32 nbit) { +static ut32 bv_copy_nbits_large_aligned(const RzBitVector *src, ut32 src_start_pos, RzBitVector *dst, ut32 dst_start_pos, ut32 nbit) { // Sanity check performed by caller ut8 start_bits = RZ_MIN((BV_ELEM_SIZE - dst_start_pos) % BV_ELEM_SIZE, nbit); ut8 trailing_bits = RZ_MIN((src_start_pos + nbit) % BV_ELEM_SIZE, nbit - start_bits); @@ -243,7 +243,7 @@ static ut32 rz_bv_copy_nbits_large_aligned(const RzBitVector *src, ut32 src_star /** * \brief Optimized version of rz_bv_copy_nbits() for copying bit range from a large bitvector to a small one */ -static ut32 rz_bv_copy_nbits_large_to_small(const RzBitVector *src, ut32 src_start_pos, RzBitVector *dst, ut32 dst_start_pos, ut32 nbit) { +static ut32 bv_copy_nbits_large_to_small(const RzBitVector *src, ut32 src_start_pos, RzBitVector *dst, ut32 dst_start_pos, ut32 nbit) { ut64 buffer = 0; ut8 start_bits = RZ_MIN((BV_ELEM_SIZE - src_start_pos) % BV_ELEM_SIZE, nbit); ut32 byte_index = (src_start_pos + start_bits) / BV_ELEM_SIZE; @@ -292,7 +292,7 @@ static ut32 rz_bv_copy_nbits_large_to_small(const RzBitVector *src, ut32 src_sta /** * \brief Optimized version of rz_bv_copy_nbits() for copying bit range from a small bitvector to a large one */ -static ut32 rz_bv_copy_nbits_small_to_large(const RzBitVector *src, ut32 src_start_pos, RzBitVector *dst, ut32 dst_start_pos, ut32 nbit) { +static ut32 bv_copy_nbits_small_to_large(const RzBitVector *src, ut32 src_start_pos, RzBitVector *dst, ut32 dst_start_pos, ut32 nbit) { ut64 byte_index = dst_start_pos / BV_ELEM_SIZE; ut8 start_bits = RZ_MIN((BV_ELEM_SIZE - dst_start_pos) % BV_ELEM_SIZE, nbit); ut8 trailing_bits = RZ_MIN((dst_start_pos + nbit) % BV_ELEM_SIZE, nbit - start_bits); @@ -351,7 +351,7 @@ static ut32 rz_bv_copy_nbits_small_to_large(const RzBitVector *src, ut32 src_sta /** * \brief Optimized version of rz_bv_copy_nbits() for large bitvectors (more than 64 bits) with unaligned bit positions */ -static ut32 rz_bv_copy_nbits_large_unaligned(const RzBitVector *src, ut32 src_start_pos, RzBitVector *dst, ut32 dst_start_pos, ut32 nbit) { +static ut32 bv_copy_nbits_large_unaligned(const RzBitVector *src, ut32 src_start_pos, RzBitVector *dst, ut32 dst_start_pos, ut32 nbit) { // Sanity check performed by caller ut64 bits_remaining = nbit; @@ -412,17 +412,17 @@ RZ_API ut32 rz_bv_copy_nbits(RZ_NONNULL const RzBitVector *src, ut32 src_start_p if (src->len > 64 && dst->len > 64) { // Both src and dst are larger than 64 bits if (src_start_pos % BV_ELEM_SIZE == dst_start_pos % BV_ELEM_SIZE) { - return rz_bv_copy_nbits_large_aligned(src, src_start_pos, dst, dst_start_pos, nbit); + return bv_copy_nbits_large_aligned(src, src_start_pos, dst, dst_start_pos, nbit); } if (src->bits.large_a != dst->bits.large_a) { - return rz_bv_copy_nbits_large_unaligned(src, src_start_pos, dst, dst_start_pos, nbit); + return bv_copy_nbits_large_unaligned(src, src_start_pos, dst, dst_start_pos, nbit); } // Use a temporary bitvector for same-vector copies RzBitVector *temp = rz_bv_new(rz_bv_len(dst)); rz_bv_copy(dst, temp); - ut32 bits_copied = rz_bv_copy_nbits_large_unaligned(src, src_start_pos, temp, dst_start_pos, nbit); + ut32 bits_copied = bv_copy_nbits_large_unaligned(src, src_start_pos, temp, dst_start_pos, nbit); rz_bv_copy(temp, dst); rz_bv_free(temp); return bits_copied; @@ -430,11 +430,11 @@ RZ_API ut32 rz_bv_copy_nbits(RZ_NONNULL const RzBitVector *src, ut32 src_start_p if (src->len > 64) { // Large to small copy - return rz_bv_copy_nbits_large_to_small(src, src_start_pos, dst, dst_start_pos, nbit); + return bv_copy_nbits_large_to_small(src, src_start_pos, dst, dst_start_pos, nbit); } // Small to large - return rz_bv_copy_nbits_small_to_large(src, src_start_pos, dst, dst_start_pos, nbit); + return bv_copy_nbits_small_to_large(src, src_start_pos, dst, dst_start_pos, nbit); } /** @@ -1689,8 +1689,36 @@ RZ_API bool rz_bv_set_range(RZ_NONNULL RzBitVector *bv, ut32 pos_start, ut32 pos return false; } - for (ut32 i = pos_start; i <= pos_end; ++i) { - rz_bv_set(bv, i, b); + if (pos_start > pos_end) { + return false; + } + + ut32 nbit = pos_end - pos_start + 1; + + if (bv->len <= 64) { + ut64 value = b ? UT64_MAX : 0; + bv->bits.small_u = rz_bits_copy_ut64(value, 0, bv->bits.small_u, pos_start, nbit); + return true; + } + + ut8 value = b ? UT8_MAX : 0; + ut8 start_bits = RZ_MIN((BV_ELEM_SIZE - pos_start) % BV_ELEM_SIZE, nbit); + ut8 trailing_bits = RZ_MIN((pos_start + nbit) % BV_ELEM_SIZE, nbit - start_bits); + ut64 middle_bytes = (nbit - start_bits - trailing_bits) / BV_ELEM_SIZE; + ut64 byte_index = pos_start / BV_ELEM_SIZE; + + if (start_bits > 0) { + bv->bits.large_a[byte_index] = rz_bits_copy_ut8(value, 0, bv->bits.large_a[byte_index], pos_start % BV_ELEM_SIZE, start_bits); + byte_index++; + } + + if (middle_bytes > 0) { + memset(&bv->bits.large_a[byte_index], value, middle_bytes); + byte_index += middle_bytes; + } + + if (trailing_bits > 0) { + bv->bits.large_a[byte_index] = rz_bits_copy_ut8(value, 0, bv->bits.large_a[byte_index], 0, trailing_bits); } return true; diff --git a/test/bench/bench_bitvector.c b/test/bench/bench_bitvector.c index d8e2e710f5..1b5b51d97d 100644 --- a/test/bench/bench_bitvector.c +++ b/test/bench/bench_bitvector.c @@ -79,6 +79,26 @@ static void bench_bv_copy_large_to_small_60_bit(RzTable *t_out) { rz_bv_free(dst); } +static void bench_bv_set_range_60_bit(RzTable *t_out) { + RzBitVector *a = rz_bv_new(64); + + RZ_BENCH_RUN("rz_bv_set_range (60 bit)", t_out, 1000000, { + rz_bv_set_range(a, 1, 60, true); + }); + + rz_bv_free(a); +} + +static void bench_bv_set_range_100_bit(RzTable *t_out) { + RzBitVector *a = rz_bv_new(128); + + RZ_BENCH_RUN("rz_bv_set_range (100 bit)", t_out, 1000000, { + rz_bv_set_range(a, 1, 100, true); + }); + + rz_bv_free(a); +} + int main(RzTable *t_out) { RzTable *t = rz_table_new(); rz_table_set_columnsf(t, "snnnn", "Benchmark", "Iterations", "Total time [ms]", "Average time [us/op]", "Throughput [ops/sec]"); @@ -89,6 +109,8 @@ int main(RzTable *t_out) { bench_bv_copy_large_100_bit_unaligned(t); bench_bv_copy_large_to_small_60_bit(t); bench_bv_copy_small_to_large_60_bit(t); + bench_bv_set_range_60_bit(t); + bench_bv_set_range_100_bit(t); // Print results const char *out = rz_table_tostring(t); diff --git a/test/unit/test_bitvector.c b/test/unit/test_bitvector.c index 9baa6aa856..9889aea171 100644 --- a/test/unit/test_bitvector.c +++ b/test/unit/test_bitvector.c @@ -1064,6 +1064,51 @@ bool test_rz_bv_set_operations(void) { mu_end; } +bool test_rz_bv_set_range_large(void) { + RzBitVector *bv = rz_bv_new(128); + + // Expect failure on inverted range + mu_assert_false(rz_bv_set_range(bv, 20, 10, true), "expected failure for inverse range"); + + // Bitrange with unalign prefix bits + mu_assert_true(rz_bv_set_range(bv, 5, 7, true), "expect rz_bv_set_range() success"); + mu_assert_streq_free(rz_bv_as_hex_string(bv, false), "0xe0", "range set 5~7 to 1"); + + mu_assert_true(rz_bv_set_range(bv, 5, 7, false), "expect rz_bv_set_range() success"); + mu_assert_streq_free(rz_bv_as_hex_string(bv, false), "0x0", "range set 5~7 to 0"); + + // Bitrange with unalign prefix and suffix bits + mu_assert_true(rz_bv_set_range(bv, 5, 8, true), "expect rz_bv_set_range() success"); + mu_assert_streq_free(rz_bv_as_hex_string(bv, false), "0x1e0", "range set 5~8 to 1"); + + mu_assert_true(rz_bv_set_range(bv, 5, 8, false), "expect rz_bv_set_range() success"); + mu_assert_streq_free(rz_bv_as_hex_string(bv, false), "0x0", "range set 5~8 to 0"); + + // Only suffix bit + mu_assert_true(rz_bv_set_range(bv, 8, 8, true), "expect rz_bv_set_range() success"); + mu_assert_streq_free(rz_bv_as_hex_string(bv, false), "0x100", "range set 8~8 to 1"); + + mu_assert_true(rz_bv_set_range(bv, 8, 8, false), "expect rz_bv_set_range() success"); + mu_assert_streq_free(rz_bv_as_hex_string(bv, false), "0x0", "range set 8~8 to 0"); + + // Bitrange with unaligned prefix, suffix bits and aligned middle bytes + mu_assert_true(rz_bv_set_range(bv, 5, 24, true), "expect rz_bv_set_range() success"); + mu_assert_streq_free(rz_bv_as_hex_string(bv, false), "0x1ffffe0", "range set 5~24 to 1"); + + mu_assert_true(rz_bv_set_range(bv, 5, 24, false), "expect rz_bv_set_range() success"); + mu_assert_streq_free(rz_bv_as_hex_string(bv, false), "0x0", "range set 5~24 to 0"); + + // Aligned + mu_assert_true(rz_bv_set_range(bv, 16, 31, true), "expect rz_bv_set_range() success"); + mu_assert_streq_free(rz_bv_as_hex_string(bv, false), "0xffff0000", "range set 16~31 to 1"); + + mu_assert_true(rz_bv_set_range(bv, 16, 31, false), "expect rz_bv_set_range() success"); + mu_assert_streq_free(rz_bv_as_hex_string(bv, false), "0x0", "range set 16~31 to 0"); + + rz_bv_free(bv); + mu_end; +} + static bool test_rz_bv_set_to_bytes_le(void) { { ut8 buf8[8] = { 0 }; @@ -1517,6 +1562,7 @@ bool all_tests() { mu_run_test(test_rz_bv_mod); mu_run_test(test_rz_bv_len_bytes); mu_run_test(test_rz_bv_set_operations); + mu_run_test(test_rz_bv_set_range_large); mu_run_test(test_rz_bv_set_to_bytes_le); mu_run_test(test_rz_bv_copy_nbits); mu_run_test(test_rz_bv_copy_nbits_small);