diff --git a/librz/util/vector.c b/librz/util/vector.c index 778265aa48..77f406393b 100644 --- a/librz/util/vector.c +++ b/librz/util/vector.c @@ -562,30 +562,23 @@ RZ_API RZ_OWN void *rz_vector_take_array(RZ_BORROW RzVector *vec) { // CLRS Quicksort. It is slow, but simple. #define VEC_INDEX(a, i) (char *)a + elem_size *(i) -static void vector_quick_sort(void *a, size_t elem_size, size_t len, RzVectorComparator cmp, bool reverse, void *user) { - rz_return_if_fail(a); + +// Recursive quicksort. \p t and \p pivot are caller-provided scratch buffers of +// elem_size bytes each; they are reused across the whole recursion so the sort +// performs no per-call allocation. +static void vector_quick_sort_rec(void *a, size_t elem_size, size_t len, RzVectorComparator cmp, bool reverse, void *user, void *t, void *pivot) { if (len <= 1) { return; } size_t i = rand() % len, j = 0; - void *t, *pivot; - - t = (void *)malloc(elem_size); - pivot = (void *)malloc(elem_size); - if (!t || !pivot) { - free(t); - free(pivot); - RZ_LOG_ERROR("Failed to allocate memory\n"); - return; - } memcpy(pivot, VEC_INDEX(a, i), elem_size); if (i != len - 1) { memcpy(VEC_INDEX(a, i), VEC_INDEX(a, len - 1), elem_size); } for (i = 0; i < len - 1; i++) { - if ((cmp(VEC_INDEX(a, i), pivot, user) < 0 && !reverse) || - (cmp(VEC_INDEX(a, i), pivot, user) > 0 && reverse)) { + int c = cmp(VEC_INDEX(a, i), pivot, user); + if ((c < 0 && !reverse) || (c > 0 && reverse)) { if (j != i) { memcpy(t, VEC_INDEX(a, i), elem_size); memcpy(VEC_INDEX(a, i), VEC_INDEX(a, j), elem_size); @@ -598,10 +591,42 @@ static void vector_quick_sort(void *a, size_t elem_size, size_t len, RzVectorCom memcpy(VEC_INDEX(a, len - 1), VEC_INDEX(a, j), elem_size); } memcpy(VEC_INDEX(a, j), pivot, elem_size); - RZ_FREE(t); - RZ_FREE(pivot); - vector_quick_sort(a, elem_size, j, cmp, reverse, user); - vector_quick_sort(VEC_INDEX(a, j + 1), elem_size, len - j - 1, cmp, reverse, user); + vector_quick_sort_rec(a, elem_size, j, cmp, reverse, user, t, pivot); + vector_quick_sort_rec(VEC_INDEX(a, j + 1), elem_size, len - j - 1, cmp, reverse, user, t, pivot); +} + +#define RZ_VECTOR_SORT_TMP_SIZE 256 + +static void vector_quick_sort(void *a, size_t elem_size, size_t len, RzVectorComparator cmp, bool reverse, void *user) { + rz_return_if_fail(a); + if (len <= 1) { + return; + } + // Allocate the two scratch buffers once for the whole sort instead of on + // every recursive call. Small elements (the common case) use the stack. + ut8 t_buf[RZ_VECTOR_SORT_TMP_SIZE]; + ut8 pivot_buf[RZ_VECTOR_SORT_TMP_SIZE]; + void *t = elem_size <= RZ_VECTOR_SORT_TMP_SIZE ? (void *)t_buf : malloc(elem_size); + void *pivot = elem_size <= RZ_VECTOR_SORT_TMP_SIZE ? (void *)pivot_buf : malloc(elem_size); + if (!t || !pivot) { + if (t != (void *)t_buf) { + free(t); + } + if (pivot != (void *)pivot_buf) { + free(pivot); + } + RZ_LOG_ERROR("Failed to allocate memory\n"); + return; + } + + vector_quick_sort_rec(a, elem_size, len, cmp, reverse, user, t, pivot); + + if (t != (void *)t_buf) { + free(t); + } + if (pivot != (void *)pivot_buf) { + free(pivot); + } } #undef VEC_INDEX @@ -822,7 +847,7 @@ RZ_API void rz_pvector_remove_data(RzPVector *vec, void *x) { return; } - size_t index = (el - (void **)vec->v.a) * sizeof(void **) / vec->v.elem_size; + size_t index = el - (void **)vec->v.a; rz_vector_remove_at(&vec->v, index, NULL); } diff --git a/test/bench/bench_vector.c b/test/bench/bench_vector.c index c4957f1fbd..81f13e314b 100644 --- a/test/bench/bench_vector.c +++ b/test/bench/bench_vector.c @@ -61,6 +61,80 @@ static void bench_rz_vector_swap(RzTable *t_out) { rz_vector_free(v); } +#define SORT_N 4096 + +static int bench_cmp_u64(const void *a, const void *b, void *user) { + (void)user; + ut64 x = *(const ut64 *)a, y = *(const ut64 *)b; + return (x > y) - (x < y); +} + +// A deliberately non-trivial comparator, representative of comparing real +// struct elements; makes the per-element comparator-call count matter. +static int bench_cmp_u64_expensive(const void *a, const void *b, void *user) { + (void)user; + volatile int acc = 0; + for (int k = 0; k < 24; k++) { + acc += k * (k ^ 5); + } + ut64 x = *(const ut64 *)a, y = *(const ut64 *)b; + return ((x > y) - (x < y)) + (acc & 0); +} + +static int bench_cmp_pvoid(const void *a, const void *b, void *user) { + (void)user; + return (a > b) - (a < b); +} + +static void bench_rz_vector_sort(RzTable *t_out) { + RzVector *v = rz_vector_new(sizeof(ut64), NULL, NULL); + rz_vector_reserve(v, SORT_N); + ut64 *master = malloc(sizeof(ut64) * SORT_N); + for (size_t i = 0; i < SORT_N; i++) { + master[i] = rz_num_rand32(UT32_MAX); + } + + { + RZ_BENCH_RUN("[RzVector] rz_vector_sort ut64 4k", t_out, 2000, { + memcpy(v->a, master, sizeof(ut64) * SORT_N); + v->len = SORT_N; + v->reverse_sorted = false; + rz_vector_sort(v, bench_cmp_u64, false, NULL); + }); + } + { + RZ_BENCH_RUN("[RzVector] rz_vector_sort expensive cmp 4k", t_out, 500, { + memcpy(v->a, master, sizeof(ut64) * SORT_N); + v->len = SORT_N; + v->reverse_sorted = false; + rz_vector_sort(v, bench_cmp_u64_expensive, false, NULL); + }); + } + + free(master); + rz_vector_free(v); +} + +static void bench_rz_pvector_sort(RzTable *t_out) { + RzPVector *v = rz_pvector_new(NULL); + rz_pvector_reserve(v, SORT_N); + void **master = malloc(sizeof(void *) * SORT_N); + for (size_t i = 0; i < SORT_N; i++) { + master[i] = (void *)(size_t)(rz_num_rand32(UT32_MAX) + 1); + } + + { + RZ_BENCH_RUN("[RzPVector] rz_pvector_sort 4k", t_out, 2000, { + memcpy(v->v.a, master, sizeof(void *) * SORT_N); + v->v.len = SORT_N; + rz_pvector_sort(v, bench_cmp_pvoid, NULL); + }); + } + + free(master); + rz_pvector_free(v); +} + int main() { RzTable *t = rz_table_new(); RZ_BENCH_TABLE_INIT(t); @@ -68,6 +142,8 @@ int main() { // Micro benchmarks bench_rz_vector_remove_at(t); bench_rz_vector_swap(t); + bench_rz_vector_sort(t); + bench_rz_pvector_sort(t); // Print results RZ_BENCH_TABLE_PRINT_AND_FREE(t); diff --git a/test/unit/test_vector.c b/test/unit/test_vector.c index 538acc08d6..363e12a731 100644 --- a/test/unit/test_vector.c +++ b/test/unit/test_vector.c @@ -373,6 +373,120 @@ static bool test_vector_find_sorted(void) { mu_end; } +static int cmp_u32(const void *a, const void *b, void *user) { + (void)user; + ut32 x = *(const ut32 *)a, y = *(const ut32 *)b; + return (x > y) - (x < y); +} +static int qsort_u32_asc(const void *a, const void *b) { + ut32 x = *(const ut32 *)a, y = *(const ut32 *)b; + return (x > y) - (x < y); +} +static int qsort_u32_desc(const void *a, const void *b) { + ut32 x = *(const ut32 *)a, y = *(const ut32 *)b; + return (y > x) - (y < x); +} + +// Sort a large vector with many duplicates, ascending and descending, and check +// the result is fully ordered and a permutation of the input (verified against +// a reference qsort). Exercises the recursion deeply and the shared scratch +// buffers, which the small existing sort tests do not. +static bool test_vector_sort_large(void) { + const size_t n = 2000; + ut32 *ref = malloc(sizeof(ut32) * n); + mu_assert_notnull(ref, "ref alloc"); + RzVector v; + rz_vector_init(&v, sizeof(ut32), NULL, NULL); + srand(0xC0FFEE); + for (size_t i = 0; i < n; i++) { + ut32 x = (ut32)(rand() % 100); // heavy duplication + ref[i] = x; + rz_vector_push(&v, &x); + } + + rz_vector_sort(&v, cmp_u32, false, NULL); + mu_assert_eq(v.len, n, "len after sort"); + bool ok = true; + for (size_t i = 1; i < v.len; i++) { + if (*(ut32 *)rz_vector_index_ptr(&v, i - 1) > *(ut32 *)rz_vector_index_ptr(&v, i)) { + ok = false; + } + } + mu_assert_true(ok, "ascending order"); + qsort(ref, n, sizeof(ut32), qsort_u32_asc); + bool perm = true; + for (size_t i = 0; i < n; i++) { + if (*(ut32 *)rz_vector_index_ptr(&v, i) != ref[i]) { + perm = false; + } + } + mu_assert_true(perm, "ascending is a permutation of the input"); + + rz_vector_sort(&v, cmp_u32, true, NULL); + ok = true; + for (size_t i = 1; i < v.len; i++) { + if (*(ut32 *)rz_vector_index_ptr(&v, i - 1) < *(ut32 *)rz_vector_index_ptr(&v, i)) { + ok = false; + } + } + mu_assert_true(ok, "descending order"); + qsort(ref, n, sizeof(ut32), qsort_u32_desc); + perm = true; + for (size_t i = 0; i < n; i++) { + if (*(ut32 *)rz_vector_index_ptr(&v, i) != ref[i]) { + perm = false; + } + } + mu_assert_true(perm, "descending is a permutation of the input"); + + rz_vector_fini(&v); + free(ref); + mu_end; +} + +typedef struct { + ut32 key; + ut8 pad[300]; +} SortBlob304; // > 256 bytes: exercises the heap-fallback scratch path in the sort + +static int cmp_blob304(const void *a, const void *b, void *user) { + (void)user; + ut32 x = ((const SortBlob304 *)a)->key, y = ((const SortBlob304 *)b)->key; + return (x > y) - (x < y); +} + +// Sort elements larger than the on-stack scratch threshold, so the sort takes +// the heap-allocated scratch fallback. Also checks the whole element (not just +// the key) is moved consistently. +static bool test_vector_sort_large_elem(void) { + const size_t n = 400; + RzVector v; + rz_vector_init(&v, sizeof(SortBlob304), NULL, NULL); + srand(0xBEEF); + for (size_t i = 0; i < n; i++) { + SortBlob304 b; + b.key = (ut32)(rand() % 1000); + memset(b.pad, (int)(b.key & 0xff), sizeof(b.pad)); // pad tied to key + rz_vector_push(&v, &b); + } + rz_vector_sort(&v, cmp_blob304, false, NULL); + mu_assert_eq(v.len, n, "len after large-elem sort"); + bool ok = true; + for (size_t i = 0; i < v.len; i++) { + SortBlob304 *b = rz_vector_index_ptr(&v, i); + if (i > 0 && ((SortBlob304 *)rz_vector_index_ptr(&v, i - 1))->key > b->key) { + ok = false; + } + // the payload must still match its key after all the memcpy shuffling + if (b->pad[0] != (ut8)(b->key & 0xff) || b->pad[299] != (ut8)(b->key & 0xff)) { + ok = false; + } + } + mu_assert_true(ok, "large-element sort ordered with intact payloads"); + rz_vector_fini(&v); + mu_end; +} + static bool test_vector_empty(void) { RzVector v; rz_vector_init(&v, 1, NULL, NULL); @@ -1519,6 +1633,40 @@ static bool test_pvector_sort(void) { mu_end; } +// rz_pvector_remove_data finds the slot whose stored pointer equals x and +// removes it while preserving order. Covers the simplified index computation. +static bool test_pvector_remove_data(void) { + RzPVector v; + rz_pvector_init(&v, NULL); + for (size_t i = 1; i <= 6; i++) { + rz_pvector_push(&v, (void *)i); + } + rz_pvector_remove_data(&v, (void *)4); // expect 1,2,3,5,6 + mu_assert_eq(rz_pvector_len(&v), 5UL, "len after remove_data"); + void *exp[] = { (void *)1, (void *)2, (void *)3, (void *)5, (void *)6 }; + bool ok = true; + for (size_t i = 0; i < 5; i++) { + if (rz_pvector_at(&v, i) != exp[i]) { + ok = false; + } + } + mu_assert_true(ok, "remove_data removes the right element and keeps order"); + + // removing the first and last elements + rz_pvector_remove_data(&v, (void *)1); // 2,3,5,6 + rz_pvector_remove_data(&v, (void *)6); // 2,3,5 + mu_assert_eq(rz_pvector_len(&v), 3UL, "len after removing ends"); + mu_assert_ptreq(rz_pvector_at(&v, 0), (void *)2, "head after removing ends"); + mu_assert_ptreq(rz_pvector_at(&v, 2), (void *)5, "tail after removing ends"); + + // removing an absent pointer is a no-op + rz_pvector_remove_data(&v, (void *)999); + mu_assert_eq(rz_pvector_len(&v), 3UL, "remove_data of absent value is a no-op"); + + rz_pvector_clear(&v); + mu_end; +} + static bool test_pvector_foreach(void) { RzPVector v; init_test_pvector2(&v, 5, 5); @@ -1710,6 +1858,8 @@ static int all_tests(void) { mu_run_test(test_vector_remove_at); mu_run_test(test_vector_remove_at_unsorted); mu_run_test(test_vector_sort); + mu_run_test(test_vector_sort_large); + mu_run_test(test_vector_sort_large_elem); mu_run_test(test_vector_remove_range); mu_run_test(test_vector_insert); mu_run_test(test_vector_insert_range); @@ -1752,6 +1902,7 @@ static int all_tests(void) { mu_run_test(test_pvector_bounds); mu_run_test(test_pvector_tips); mu_run_test(test_pvector_uniq); + mu_run_test(test_pvector_remove_data); mu_run_test(test_array_bounds_fuzz);