mapleFU commented on code in PR #2170:
URL: https://github.com/apache/kvrocks/pull/2170#discussion_r1527940671
##########
src/types/redis_bitmap_string.cc:
##########
@@ -100,31 +100,80 @@ rocksdb::Status BitmapString::BitCount(const std::string
&raw_value, int64_t sta
}
rocksdb::Status BitmapString::BitPos(const std::string &raw_value, bool bit,
int64_t start, int64_t stop,
- bool stop_given, int64_t *pos) {
+ bool stop_given, int64_t *pos, bool
is_bit_index) {
std::string_view string_value =
std::string_view{raw_value}.substr(Metadata::GetOffsetAfterExpire(raw_value[0]));
auto strlen = static_cast<int64_t>(string_value.size());
/* Convert negative and out-of-bound indexes */
- std::tie(start, stop) = NormalizeRange(start, stop, strlen);
+
+ int64_t length = is_bit_index ? strlen * 8 : strlen;
+ std::tie(start, stop) = NormalizeRange(start, stop, length);
if (start > stop) {
*pos = -1;
- } else {
- int64_t bytes = stop - start + 1;
- *pos = util::msb::RawBitpos(reinterpret_cast<const uint8_t
*>(string_value.data()) + start, bytes, bit);
-
- /* If we are looking for clear bits, and the user specified an exact
- * range with start-end, we can't consider the right of the range as
- * zero padded (as we do when no explicit end is given).
- *
- * So if redisBitpos() returns the first bit outside the range,
- * we return -1 to the caller, to mean, in the specified range there
- * is not a single "0" bit. */
- if (stop_given && bit == 0 && *pos == bytes * 8) {
+ return rocksdb::Status::OK();
+ }
+
+ int64_t byte_start = is_bit_index ? start / 8 : start;
+ int64_t byte_stop = is_bit_index ? stop / 8 : stop;
+ int64_t bit_in_start_byte = is_bit_index ? start % 8 : 0;
+ int64_t bit_in_stop_byte = is_bit_index ? stop % 8 : 7;
+ int64_t bytes_cnt = byte_stop - byte_start + 1;
+
+ auto bit_pos_in_byte_startstop = [](char byte, bool bit, uint32_t start,
uint32_t stop) -> int {
Review Comment:
ditto
##########
src/types/redis_bitmap.cc:
##########
@@ -335,14 +341,39 @@ rocksdb::Status Bitmap::BitPos(const Slice &user_key,
bool bit, int64_t start, i
return -1;
};
+ auto bit_pos_in_byte_startstop = [](char byte, bool bit, uint32_t start,
uint32_t stop) -> int {
+ for (uint32_t i = start; i <= stop; i++) {
+ if (bit && (byte & (1 << i)) != 0) return (int)i; // typecast to int
since the value ranges from 0 to 7
Review Comment:
`// typecast to int since the value ranges from 0 to 7`
What does this mean? Isn't something like uint8_t or other is proper?
uint32_t and int ( usally int32_t ) doesn't have significant different here?
##########
src/types/redis_bitmap.cc:
##########
@@ -335,14 +341,39 @@ rocksdb::Status Bitmap::BitPos(const Slice &user_key,
bool bit, int64_t start, i
return -1;
};
+ auto bit_pos_in_byte_startstop = [](char byte, bool bit, uint32_t start,
uint32_t stop) -> int {
+ for (uint32_t i = start; i <= stop; i++) {
+ if (bit && (byte & (1 << i)) != 0) return (int)i; // typecast to int
since the value ranges from 0 to 7
+ if (!bit && (byte & (1 << i)) == 0) return (int)i;
+ }
+ return -1;
+ };
+
+ auto range_in_byte = [](uint32_t byte_start, uint32_t byte_end, uint32_t
curr_byte, uint32_t start_bit,
+ uint32_t end_bit) -> std::pair<uint32_t, uint32_t> {
+ if (curr_byte == byte_start && curr_byte == byte_end) return {start_bit,
end_bit};
+ if (curr_byte == byte_start) return {start_bit, 7};
+ if (curr_byte == byte_end) return {0, end_bit};
+ return {0, 7};
+ };
Review Comment:
```
auto range_in_byte = [start_bit_pos_in_byte,
stop_bit_pos_in_byte](uint32_t byte_start, uint32_t byte_end, uint32_t
curr_byte) -> std::pair<uint32_t, uint32_t> {
if (curr_byte == byte_start && curr_byte == byte_end) return
{start_bit_pos_in_byte, stop_bit_pos_in_byte};
if (curr_byte == byte_start) return {start_bit, 7};
if (curr_byte == byte_end) return {0, end_bit};
return {0, 7};
};
```
Would this be more clear?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
