mapleFU commented on code in PR #1901: URL: https://github.com/apache/kvrocks/pull/1901#discussion_r1400572408
########## src/common/bitfield_util.h: ########## @@ -0,0 +1,180 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ + +#pragma once + +#include <cstdint> +#include <cstring> +#include <limits> +#include <stdexcept> +#include <string> + +enum class BitfieldOverflowBehavior : uint8_t { kWrap, kSat, kFail }; + +class BitfieldBitsLengthNotFitExecption : public std::runtime_error { + public: + BitfieldBitsLengthNotFitExecption() : std::runtime_error("bits length is not fitted to bitfield.") {} +}; + +class BitfieldEncoding { + public: + enum class Type : uint8_t { kSigned, kUnsigned }; + + // check whether the bit length is fit to given number sign. + // Redis has bits length limitation to bitfield. + // Quote: + // The supported encodings are up to 64 bits for signed integers, and up to 63 bits for unsigned integers. This + // limitation with unsigned integers is due to the fact that currently the Redis protocol is unable to return 64 bit + // unsigned integers as replies. + // see also https://redis.io/commands/bitfield/ + static bool IsSupportedBitLengths(Type type, uint8_t bits) { + uint8_t max_bits = 64; + if (type == Type::kUnsigned) { + max_bits = 63; + } + return 1 <= bits && bits <= max_bits; + } + + BitfieldEncoding(Type type, uint8_t bits) : type_(type), bits_(bits) { + if (!IsSupportedBitLengths(type, bits)) { + throw BitfieldBitsLengthNotFitExecption(); + } + } + + bool IsSigned() const noexcept { return type_ == Type::kSigned; } + + uint8_t Bits() const noexcept { return bits_; } + + void SetBits(uint8_t new_bits) { + if (!IsSupportedBitLengths(type_, new_bits)) { + throw BitfieldBitsLengthNotFitExecption(); + } + bits_ = new_bits; + } + + void SetType(Type new_type) { + if (!IsSupportedBitLengths(new_type, bits_)) { + throw BitfieldBitsLengthNotFitExecption(); + } + type_ = new_type; + } + + std::string ToString() const noexcept { return (IsSigned() ? "i" : "u") + std::to_string(static_cast<int>(bits_)); } + + private: + Type type_; + uint8_t bits_; +}; + +struct BitfieldOperation { + // see https://redis.io/commands/bitfield/ to get more details. + enum class Type : uint8_t { kGet, kSet, kIncrBy }; + + Type type; + BitfieldOverflowBehavior overflow{BitfieldOverflowBehavior::kWrap}; + BitfieldEncoding encoding{BitfieldEncoding::Type::kSigned, 32}; + uint32_t offset; + // INCRBY amount or SET value + int64_t value; +}; + +namespace detail { +// return true if overflow +bool SignedBitfieldPlus(uint64_t value, int64_t incr, uint8_t bits, BitfieldOverflowBehavior overflow, uint64_t *dst); + +// return true if overflow. +bool UnsignedBitfieldPlus(uint64_t value, int64_t incr, uint8_t bits, BitfieldOverflowBehavior overflow, uint64_t *dst); +} // namespace detail + +// safe cast from unsigned to signed, without any bit changes. +// see also "Integral conversions" on https://en.cppreference.com/w/cpp/language/implicit_conversion +// If the destination type is signed, the result when overflow is implementation-defined until C++20 +inline int64_t CastToSignedWithoutBitChanges(uint64_t x) { + int64_t res = 0; + memcpy(&res, &x, sizeof(res)); + return res; +} + +// return true if overflow. +inline auto BitfieldPlus(uint64_t value, int64_t incr, BitfieldEncoding enc, BitfieldOverflowBehavior overflow, Review Comment: Better using `bool` as response explicit? ########## src/common/bitfield_util.h: ########## @@ -0,0 +1,180 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ + +#pragma once + +#include <cstdint> +#include <cstring> +#include <limits> +#include <stdexcept> +#include <string> + +enum class BitfieldOverflowBehavior : uint8_t { kWrap, kSat, kFail }; + +class BitfieldBitsLengthNotFitExecption : public std::runtime_error { Review Comment: Hmmm for Style kvrocks uses `Status` rather than Exception, would you mind using status to wrap this? ########## src/common/bitfield_util.h: ########## @@ -0,0 +1,180 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ + +#pragma once + +#include <cstdint> +#include <cstring> +#include <limits> +#include <stdexcept> +#include <string> + +enum class BitfieldOverflowBehavior : uint8_t { kWrap, kSat, kFail }; + +class BitfieldBitsLengthNotFitExecption : public std::runtime_error { + public: + BitfieldBitsLengthNotFitExecption() : std::runtime_error("bits length is not fitted to bitfield.") {} +}; + +class BitfieldEncoding { + public: + enum class Type : uint8_t { kSigned, kUnsigned }; + + // check whether the bit length is fit to given number sign. + // Redis has bits length limitation to bitfield. + // Quote: + // The supported encodings are up to 64 bits for signed integers, and up to 63 bits for unsigned integers. This + // limitation with unsigned integers is due to the fact that currently the Redis protocol is unable to return 64 bit + // unsigned integers as replies. + // see also https://redis.io/commands/bitfield/ + static bool IsSupportedBitLengths(Type type, uint8_t bits) { + uint8_t max_bits = 64; + if (type == Type::kUnsigned) { + max_bits = 63; + } + return 1 <= bits && bits <= max_bits; + } + + BitfieldEncoding(Type type, uint8_t bits) : type_(type), bits_(bits) { Review Comment: This can be wrapped as a `Result<BitfieldEncoding>` builder, and `BitfieldEncoding` as a private ctor. ########## src/common/bitfield_util.h: ########## @@ -0,0 +1,180 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ + +#pragma once + +#include <cstdint> +#include <cstring> +#include <limits> +#include <stdexcept> +#include <string> + +enum class BitfieldOverflowBehavior : uint8_t { kWrap, kSat, kFail }; + +class BitfieldBitsLengthNotFitExecption : public std::runtime_error { + public: + BitfieldBitsLengthNotFitExecption() : std::runtime_error("bits length is not fitted to bitfield.") {} +}; + +class BitfieldEncoding { + public: + enum class Type : uint8_t { kSigned, kUnsigned }; + + // check whether the bit length is fit to given number sign. + // Redis has bits length limitation to bitfield. + // Quote: + // The supported encodings are up to 64 bits for signed integers, and up to 63 bits for unsigned integers. This + // limitation with unsigned integers is due to the fact that currently the Redis protocol is unable to return 64 bit + // unsigned integers as replies. + // see also https://redis.io/commands/bitfield/ + static bool IsSupportedBitLengths(Type type, uint8_t bits) { + uint8_t max_bits = 64; + if (type == Type::kUnsigned) { + max_bits = 63; + } + return 1 <= bits && bits <= max_bits; + } + + BitfieldEncoding(Type type, uint8_t bits) : type_(type), bits_(bits) { + if (!IsSupportedBitLengths(type, bits)) { + throw BitfieldBitsLengthNotFitExecption(); + } + } + + bool IsSigned() const noexcept { return type_ == Type::kSigned; } + + uint8_t Bits() const noexcept { return bits_; } + + void SetBits(uint8_t new_bits) { + if (!IsSupportedBitLengths(type_, new_bits)) { + throw BitfieldBitsLengthNotFitExecption(); + } + bits_ = new_bits; + } + + void SetType(Type new_type) { + if (!IsSupportedBitLengths(new_type, bits_)) { + throw BitfieldBitsLengthNotFitExecption(); + } + type_ = new_type; + } + + std::string ToString() const noexcept { return (IsSigned() ? "i" : "u") + std::to_string(static_cast<int>(bits_)); } + + private: + Type type_; + uint8_t bits_; +}; + +struct BitfieldOperation { + // see https://redis.io/commands/bitfield/ to get more details. + enum class Type : uint8_t { kGet, kSet, kIncrBy }; + + Type type; + BitfieldOverflowBehavior overflow{BitfieldOverflowBehavior::kWrap}; + BitfieldEncoding encoding{BitfieldEncoding::Type::kSigned, 32}; + uint32_t offset; + // INCRBY amount or SET value + int64_t value; +}; + +namespace detail { +// return true if overflow +bool SignedBitfieldPlus(uint64_t value, int64_t incr, uint8_t bits, BitfieldOverflowBehavior overflow, uint64_t *dst); + +// return true if overflow. +bool UnsignedBitfieldPlus(uint64_t value, int64_t incr, uint8_t bits, BitfieldOverflowBehavior overflow, uint64_t *dst); +} // namespace detail + +// safe cast from unsigned to signed, without any bit changes. +// see also "Integral conversions" on https://en.cppreference.com/w/cpp/language/implicit_conversion +// If the destination type is signed, the result when overflow is implementation-defined until C++20 +inline int64_t CastToSignedWithoutBitChanges(uint64_t x) { + int64_t res = 0; + memcpy(&res, &x, sizeof(res)); + return res; +} + +// return true if overflow. +inline auto BitfieldPlus(uint64_t value, int64_t incr, BitfieldEncoding enc, BitfieldOverflowBehavior overflow, + uint64_t *dst) { + if (enc.IsSigned()) { + return detail::SignedBitfieldPlus(value, incr, enc.Bits(), overflow, dst); + } + return detail::UnsignedBitfieldPlus(value, incr, enc.Bits(), overflow, dst); +} + +// return true if successful +inline bool BitfieldOp(BitfieldOperation op, uint64_t old_value, uint64_t *new_value) { + if (op.type == BitfieldOperation::Type::kGet) { + *new_value = old_value; + return true; + } + + bool overflow = false; + if (op.type == BitfieldOperation::Type::kSet) { + overflow = BitfieldPlus(op.value, 0, op.encoding, op.overflow, new_value); + } else { + overflow = BitfieldPlus(old_value, op.value, op.encoding, op.overflow, new_value); + } + + return op.overflow != BitfieldOverflowBehavior::kFail || !overflow; +} + +template <class BitmapView> Review Comment: We can leave a todo here for optimizations. I didn't try it in `-O3` but it looks slow if bits isn't small ########## src/common/bitfield_util.h: ########## @@ -0,0 +1,180 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ + +#pragma once + +#include <cstdint> +#include <cstring> +#include <limits> +#include <stdexcept> +#include <string> + +enum class BitfieldOverflowBehavior : uint8_t { kWrap, kSat, kFail }; + +class BitfieldBitsLengthNotFitExecption : public std::runtime_error { + public: + BitfieldBitsLengthNotFitExecption() : std::runtime_error("bits length is not fitted to bitfield.") {} +}; + +class BitfieldEncoding { + public: + enum class Type : uint8_t { kSigned, kUnsigned }; + + // check whether the bit length is fit to given number sign. + // Redis has bits length limitation to bitfield. + // Quote: + // The supported encodings are up to 64 bits for signed integers, and up to 63 bits for unsigned integers. This + // limitation with unsigned integers is due to the fact that currently the Redis protocol is unable to return 64 bit + // unsigned integers as replies. + // see also https://redis.io/commands/bitfield/ + static bool IsSupportedBitLengths(Type type, uint8_t bits) { + uint8_t max_bits = 64; + if (type == Type::kUnsigned) { + max_bits = 63; + } + return 1 <= bits && bits <= max_bits; + } + + BitfieldEncoding(Type type, uint8_t bits) : type_(type), bits_(bits) { + if (!IsSupportedBitLengths(type, bits)) { + throw BitfieldBitsLengthNotFitExecption(); + } + } + + bool IsSigned() const noexcept { return type_ == Type::kSigned; } + + uint8_t Bits() const noexcept { return bits_; } + + void SetBits(uint8_t new_bits) { + if (!IsSupportedBitLengths(type_, new_bits)) { + throw BitfieldBitsLengthNotFitExecption(); + } + bits_ = new_bits; + } + + void SetType(Type new_type) { + if (!IsSupportedBitLengths(new_type, bits_)) { + throw BitfieldBitsLengthNotFitExecption(); + } + type_ = new_type; + } + + std::string ToString() const noexcept { return (IsSigned() ? "i" : "u") + std::to_string(static_cast<int>(bits_)); } + + private: + Type type_; + uint8_t bits_; +}; + +struct BitfieldOperation { + // see https://redis.io/commands/bitfield/ to get more details. + enum class Type : uint8_t { kGet, kSet, kIncrBy }; + + Type type; + BitfieldOverflowBehavior overflow{BitfieldOverflowBehavior::kWrap}; + BitfieldEncoding encoding{BitfieldEncoding::Type::kSigned, 32}; + uint32_t offset; + // INCRBY amount or SET value + int64_t value; +}; + +namespace detail { +// return true if overflow +bool SignedBitfieldPlus(uint64_t value, int64_t incr, uint8_t bits, BitfieldOverflowBehavior overflow, uint64_t *dst); + +// return true if overflow. +bool UnsignedBitfieldPlus(uint64_t value, int64_t incr, uint8_t bits, BitfieldOverflowBehavior overflow, uint64_t *dst); +} // namespace detail + +// safe cast from unsigned to signed, without any bit changes. +// see also "Integral conversions" on https://en.cppreference.com/w/cpp/language/implicit_conversion +// If the destination type is signed, the result when overflow is implementation-defined until C++20 +inline int64_t CastToSignedWithoutBitChanges(uint64_t x) { + int64_t res = 0; + memcpy(&res, &x, sizeof(res)); + return res; +} + +// return true if overflow. +inline auto BitfieldPlus(uint64_t value, int64_t incr, BitfieldEncoding enc, BitfieldOverflowBehavior overflow, + uint64_t *dst) { + if (enc.IsSigned()) { + return detail::SignedBitfieldPlus(value, incr, enc.Bits(), overflow, dst); + } + return detail::UnsignedBitfieldPlus(value, incr, enc.Bits(), overflow, dst); +} + +// return true if successful +inline bool BitfieldOp(BitfieldOperation op, uint64_t old_value, uint64_t *new_value) { + if (op.type == BitfieldOperation::Type::kGet) { + *new_value = old_value; + return true; + } + + bool overflow = false; + if (op.type == BitfieldOperation::Type::kSet) { + overflow = BitfieldPlus(op.value, 0, op.encoding, op.overflow, new_value); + } else { + overflow = BitfieldPlus(old_value, op.value, op.encoding, op.overflow, new_value); + } + + return op.overflow != BitfieldOverflowBehavior::kFail || !overflow; +} + +template <class BitmapView> +inline void SetBitfield(BitmapView &view, uint32_t offset, uint32_t bits, uint64_t value) { + while (bits--) { + bool v = (value & (uint64_t(1) << bits)) != 0; + uint32_t byte = offset >> 3; + uint32_t bit = 7 - (offset & 7); + uint32_t byteval = view[byte]; + byteval &= ~(1 << bit); + byteval |= int(v) << bit; + view[byte] = char(byteval); + offset++; + } +} + +template <class BitmapView> +inline uint64_t GetUnsignedBitfield(const BitmapView &view, uint64_t offset, uint64_t bits) { Review Comment: ditto, we can leave a todo here? ########## src/types/redis_bitmap.cc: ########## @@ -38,6 +39,22 @@ const char kErrBitmapStringOutOfRange[] = "The size of the bitmap string exceeds the " "configuration item max-bitmap-to-string-mb"; +// min_bytes can not more than kBitmapSegmentBytes +void ExpandBitmapSegment(std::string *segment, size_t min_bytes) { + auto old_size = segment->size(); + if (min_bytes > old_size) { + size_t expand_size = 0; + if (min_bytes > old_size * 2) { + expand_size = min_bytes - old_size; + } else if (old_size * 2 > kBitmapSegmentBytes) { + expand_size = kBitmapSegmentBytes - old_size; Review Comment: I think we need to check or dcheck `kBitmapSegmentBytes` and size here (as an separate helper function, more checks seems better) ########## src/common/bitfield_util.h: ########## @@ -0,0 +1,180 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ + +#pragma once + +#include <cstdint> +#include <cstring> +#include <limits> +#include <stdexcept> +#include <string> + +enum class BitfieldOverflowBehavior : uint8_t { kWrap, kSat, kFail }; + +class BitfieldBitsLengthNotFitExecption : public std::runtime_error { + public: + BitfieldBitsLengthNotFitExecption() : std::runtime_error("bits length is not fitted to bitfield.") {} +}; + +class BitfieldEncoding { + public: + enum class Type : uint8_t { kSigned, kUnsigned }; + + // check whether the bit length is fit to given number sign. + // Redis has bits length limitation to bitfield. + // Quote: + // The supported encodings are up to 64 bits for signed integers, and up to 63 bits for unsigned integers. This + // limitation with unsigned integers is due to the fact that currently the Redis protocol is unable to return 64 bit + // unsigned integers as replies. + // see also https://redis.io/commands/bitfield/ + static bool IsSupportedBitLengths(Type type, uint8_t bits) { + uint8_t max_bits = 64; + if (type == Type::kUnsigned) { + max_bits = 63; + } + return 1 <= bits && bits <= max_bits; + } + + BitfieldEncoding(Type type, uint8_t bits) : type_(type), bits_(bits) { + if (!IsSupportedBitLengths(type, bits)) { + throw BitfieldBitsLengthNotFitExecption(); + } + } + + bool IsSigned() const noexcept { return type_ == Type::kSigned; } + + uint8_t Bits() const noexcept { return bits_; } + + void SetBits(uint8_t new_bits) { + if (!IsSupportedBitLengths(type_, new_bits)) { + throw BitfieldBitsLengthNotFitExecption(); + } + bits_ = new_bits; + } + + void SetType(Type new_type) { + if (!IsSupportedBitLengths(new_type, bits_)) { + throw BitfieldBitsLengthNotFitExecption(); + } + type_ = new_type; + } + + std::string ToString() const noexcept { return (IsSigned() ? "i" : "u") + std::to_string(static_cast<int>(bits_)); } + + private: + Type type_; + uint8_t bits_; +}; + +struct BitfieldOperation { + // see https://redis.io/commands/bitfield/ to get more details. + enum class Type : uint8_t { kGet, kSet, kIncrBy }; + + Type type; + BitfieldOverflowBehavior overflow{BitfieldOverflowBehavior::kWrap}; + BitfieldEncoding encoding{BitfieldEncoding::Type::kSigned, 32}; + uint32_t offset; + // INCRBY amount or SET value + int64_t value; +}; + +namespace detail { +// return true if overflow +bool SignedBitfieldPlus(uint64_t value, int64_t incr, uint8_t bits, BitfieldOverflowBehavior overflow, uint64_t *dst); + +// return true if overflow. +bool UnsignedBitfieldPlus(uint64_t value, int64_t incr, uint8_t bits, BitfieldOverflowBehavior overflow, uint64_t *dst); +} // namespace detail + +// safe cast from unsigned to signed, without any bit changes. +// see also "Integral conversions" on https://en.cppreference.com/w/cpp/language/implicit_conversion +// If the destination type is signed, the result when overflow is implementation-defined until C++20 +inline int64_t CastToSignedWithoutBitChanges(uint64_t x) { + int64_t res = 0; + memcpy(&res, &x, sizeof(res)); + return res; +} + +// return true if overflow. +inline auto BitfieldPlus(uint64_t value, int64_t incr, BitfieldEncoding enc, BitfieldOverflowBehavior overflow, + uint64_t *dst) { + if (enc.IsSigned()) { + return detail::SignedBitfieldPlus(value, incr, enc.Bits(), overflow, dst); + } + return detail::UnsignedBitfieldPlus(value, incr, enc.Bits(), overflow, dst); +} + +// return true if successful +inline bool BitfieldOp(BitfieldOperation op, uint64_t old_value, uint64_t *new_value) { + if (op.type == BitfieldOperation::Type::kGet) { + *new_value = old_value; + return true; + } + + bool overflow = false; + if (op.type == BitfieldOperation::Type::kSet) { + overflow = BitfieldPlus(op.value, 0, op.encoding, op.overflow, new_value); + } else { + overflow = BitfieldPlus(old_value, op.value, op.encoding, op.overflow, new_value); + } + + return op.overflow != BitfieldOverflowBehavior::kFail || !overflow; +} + +template <class BitmapView> +inline void SetBitfield(BitmapView &view, uint32_t offset, uint32_t bits, uint64_t value) { + while (bits--) { + bool v = (value & (uint64_t(1) << bits)) != 0; + uint32_t byte = offset >> 3; + uint32_t bit = 7 - (offset & 7); + uint32_t byteval = view[byte]; + byteval &= ~(1 << bit); + byteval |= int(v) << bit; + view[byte] = char(byteval); + offset++; + } +} + +template <class BitmapView> +inline uint64_t GetUnsignedBitfield(const BitmapView &view, uint64_t offset, uint64_t bits) { + uint64_t value = 0; + while (bits--) { + uint32_t byte = offset >> 3; + uint32_t bit = 7 - (offset & 0x7); + uint32_t byteval = view[byte]; + uint32_t bitval = (byteval >> bit) & 1; + value = (value << 1) | bitval; + offset++; + } + return value; +} + +template <class BitmapView> +inline int64_t GetSignedBitfield(const BitmapView &view, uint64_t offset, uint64_t bits) { + if (!BitfieldEncoding::IsSupportedBitLengths(BitfieldEncoding::Type::kSigned, bits)) { + throw BitfieldBitsLengthNotFitExecption(); + } + int64_t value = CastToSignedWithoutBitChanges(GetUnsignedBitfield(view, offset, bits)); + + if ((value & (static_cast<uint64_t>(1) << (bits - 1))) != 0) { + value |= CastToSignedWithoutBitChanges(std::numeric_limits<uint64_t>::max() << bits); + } Review Comment: Hmmm this is a bit tricky here I think...Can we add some comment here? ########## src/storage/batch_extractor.cc: ########## @@ -232,6 +232,10 @@ rocksdb::Status WriteBatchExtractor::PutCF(uint32_t column_family_id, const Slic first_seen_ = false; } break; + case kRedisCmdBitfield: Review Comment: Would you mind testing the slot-migration case here? -- 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]
