Bitvector in-place refactor

[Rot127]

Your checklist for this pull request

• I've read the guidelines for contributing to this repository.
• I made sure to follow the project's coding style.
• I've documented every RZ_API function and struct this PR changes.
• I've added tests that prove my changes are effective (required for changes to RZ_API).
• I've updated the Rizin book with the relevant information (if needed).
• I've used AI tools to generate fully or partially these code changes and I'm sure the changes are not copyrighted by somebody else.

Detailed description

Refactors multiple bit vector operations to work in place.

This is helpful for hot paths which don't want to allocate a new bit vector with each operation on them.

Test plan

Added

Closing issues

...

[Rot127]

fyi @antonangeloff

[antonangeloff]

Looks good! BTW doesn't rz_bv_copy_nbits() already work in place? Or rz_bv_copy_nbits_inplace() is added for API/naming consistency?

[Rot127]

Looks good! BTW doesn't rz_bv_copy_nbits() already work in place? Or rz_bv_copy_nbits_inplace() is added for API/naming consistency?

Yes, I wanted API consistency. As you see it uses the "not-inplace" function. So both are tested nonetheless.

[notxvilka]

While you are on it, could you please extend the bench to test also inplace functions? Would be interesting to compare the actual results.

[wargio]

make it const if doesn't change.

[wargio]

overall seems a nice improvement.

[antonangeloff]

Yes, I wanted API consistency. As you see it uses the "not-inplace" function. So both are tested nonetheless.

For what it's worth - from API perspective there seems to be 2 functions doing the same thing, and also the function rz_bv_copy_nbits_inplace() implies that it's counterpart (rz_bv_copy_nbits) doesn't do an in-place copy

If you think it makes sense rz_bv_copy_nbits() can be changed to do an actual out-of-place copy by allocating and returning a new bit vector

[Rot127]

@notxvilka Here are the benchmark results of the add and sub (with debug build):

rz_bv_add(a, b, NULL)                                   1000000     1336.091000             1.336091        748452.014122
rz_bv_add_inplace(a, b, NULL)                           1000000     1011.513000             1.011513        988618.040500
rz_bv_sub(a, b, NULL)                                   1000000     1730.198000             1.730198        577968.533081
rz_bv_sub_inplace(a, b, NULL)                           1000000     1475.617000             1.475617        677682.623608

So something around 20-25%.
Which easily justifies it, if we assume the analysis is for some automated scanning service.

Generally I'd like to see the analysis of a BB happen mostly in cache. Accessing RAM only for IO reads/writes or lifting of rarely used instructions.

I did run some perf mem record already, and IO access had most of the cache misses.
But it is too early to say. Because we need the interpreter be in a state to analyze a whole binary.