Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add roaring_bitmap_rank_many(): get rank() values in Bulk #527

Merged
merged 3 commits into from
Dec 1, 2023

Conversation

longqimin
Copy link
Contributor

@longqimin longqimin commented Nov 29, 2023

this pr provide an api implementation for fast getting rank of many elements in bulk mode.
as well as test/benchmark.

$ microbenchmarks/bench --benchmark_filter="RankMany*"
2023-11-29T09:55:50+08:00
Running microbenchmarks/bench
Run on (36 X 4600 MHz CPU s)
CPU Caches:
  L1 Data 32 KiB (x18)
  L1 Instruction 32 KiB (x18)
  L2 Unified 1024 KiB (x18)
  L3 Unified 25344 KiB (x1)
Load Average: 0.57, 0.73, 0.61
AVX-2 hardware: yes
AVX-512: supported by compiler
AVX-512 hardware: no
In RAM volume in MiB (estimated): 1.802828
benchmarking other files: You may pass is a data directory as a parameter.
data source: CRoaring/benchmarks/realdata/census1881
number of bitmaps: 200
x64: detected
***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead.
-----------------------------------------------------------------------
Benchmark             Time             CPU   Iterations UserCounters...
-----------------------------------------------------------------------
RankManySlow      14070 ns        14030 ns        49497 GHz=4.60848 cycles=61.537k instructions=145.324k
RankMany           5616 ns         5601 ns       125617 GHz=4.42231 cycles=24.734k instructions=75.405k

@longqimin longqimin changed the title roaring_bitmap_rank_many(): get rank() values in Bulk add roaring_bitmap_rank_many(): get rank() values in Bulk Nov 30, 2023
@longqimin
Copy link
Contributor Author

@lemire Would you take a look ?

@lemire
Copy link
Member

lemire commented Nov 30, 2023

Of course I will review.

* it puts rank value of each element in `[begin .. end)` to `ans[]`
*
* the values in `[begin .. end)` must be sorted in Ascending order;
* the `ans` must have enough size.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be we can be more precise and state what is meant by size. It is size_t length = (end-begin). Correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. i borrowed a doc from roaring_bitmap_to_uint32_array().

 *     ans = malloc((end-begin) * sizeof(uint64_t));

@lemire
Copy link
Member

lemire commented Nov 30, 2023

@longqimin This looks very good to me. Can you check my comment above?

uint16_t xhigh = x >> 16;
if(xhigh != high) return iter - begin;// stop at next container

const int32_t idx = binarySearch(arr->array+pos, arr->cardinality-pos, x&0xFFFF);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const int32_t idx = binarySearch(arr->array+pos, arr->cardinality-pos, x&0xFFFF);
const int32_t idx = binarySearch(arr->array+pos, arr->cardinality-pos, (uint16_t)x);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -412,6 +412,29 @@ inline int array_container_rank(const array_container_t *arr, uint16_t x) {
}
}

// bulk version of array_container_rank(); return number of consumed elements
inline uint32_t array_container_rank_many(const array_container_t *arr, uint64_t start_rank, const uint32_t* begin, const uint32_t* end, uint64_t* ans){
const uint16_t high = (*begin) >> 16;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const uint16_t high = (*begin) >> 16;
const uint16_t high = (uint16_t)((*begin) >> 16);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

const uint32_t* iter = begin;
for(; iter != end; iter++) {
uint32_t x = *iter;
uint16_t xhigh = x >> 16;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uint16_t xhigh = x >> 16;
uint16_t xhigh = (uint16_t)(x >> 16);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -1232,6 +1232,29 @@ int bitset_container_rank(const bitset_container_t *container, uint16_t x) {
return sum;
}

uint32_t bitset_container_rank_many(const bitset_container_t *container, uint64_t start_rank, const uint32_t* begin, const uint32_t* end, uint64_t* ans){
const uint16_t high = (*begin) >> 16;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const uint16_t high = (*begin) >> 16;
const uint16_t high = (uint16_t)((*begin) >> 16);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

const uint32_t* iter = begin;
for(; iter != end; iter++) {
uint32_t x = *iter;
uint16_t xhigh = x >> 16;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uint16_t xhigh = x >> 16;
uint16_t xhigh = (uint16_t)(x >> 16);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

uint16_t xhigh = x >> 16;
if(xhigh != high) return iter - begin; // stop at next container

uint16_t xlow = x & 0xFFFF;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uint16_t xlow = x & 0xFFFF;
uint16_t xlow = (uint16_t)x;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

int i = 0;
for(;iter != end; iter++) {
uint32_t x = *iter;
uint16_t xhigh = x >> 16;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uint16_t xhigh = x >> 16;
uint16_t xhigh = (uint16_t)(x >> 16);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@lemire lemire merged commit 370199f into RoaringBitmap:master Dec 1, 2023
25 checks passed
@lemire
Copy link
Member

lemire commented Dec 1, 2023

Merged. Will be part of the next release. Thanks.

@Dr-Emann
Copy link
Member

Dr-Emann commented Dec 1, 2023

the values in [begin .. end) must be sorted in Ascending order

It might be worth documenting which of the following can occur if this is not upheld:

  • Incorrect values
  • Infinite looping
  • Full Undefined Behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants