mapleFU commented on code in PR #1489:
URL: 
https://github.com/apache/incubator-kvrocks/pull/1489#discussion_r1235459411


##########
src/server/server.h:
##########
@@ -319,4 +359,8 @@ class Server {
   std::atomic<size_t> watched_key_size_ = 0;
   std::map<std::string, std::set<redis::Connection *>> watched_key_map_;
   std::shared_mutex watched_key_mutex_;
+
+  // SCAN ring buffer
+  std::atomic<uint16_t> cursor_counter_ = {0};
+  std::array<CursorDictElement, CURSOR_DICT_SIZE> cursor_dict_;

Review Comment:
   @PragmaTwice would a `sizeof(CursorDictElement) * CURSOR_DICT_SIZE` dict in 
`Server` makes it too large? And If `Server` is large, would it matters?



##########
src/server/server.h:
##########
@@ -84,6 +88,39 @@ struct ChannelSubscribeNum {
   size_t subscribe_num;
 };
 
+// CURSOR_DICT_SIZE must be 2^n where n <= 16
+constexpr const size_t CURSOR_DICT_SIZE = 1024 * 16;
+static_assert((CURSOR_DICT_SIZE & (CURSOR_DICT_SIZE - 1)) == 0, 
"CURSOR_DICT_SIZE must be 2^n");
+static_assert(CURSOR_DICT_SIZE <= (1 << 15), "CURSOR_DICT_SIZE must be less 
than or equal to 2^16");
+
+enum CursorType {

Review Comment:
   ```c++
   enum class CursorType: uint8_t {};
   ```
   
   would `enum class` helps?



##########
src/server/server.h:
##########
@@ -84,6 +88,39 @@ struct ChannelSubscribeNum {
   size_t subscribe_num;
 };
 
+// CURSOR_DICT_SIZE must be 2^n where n <= 16
+constexpr const size_t CURSOR_DICT_SIZE = 1024 * 16;
+static_assert((CURSOR_DICT_SIZE & (CURSOR_DICT_SIZE - 1)) == 0, 
"CURSOR_DICT_SIZE must be 2^n");
+static_assert(CURSOR_DICT_SIZE <= (1 << 15), "CURSOR_DICT_SIZE must be less 
than or equal to 2^16");
+
+enum CursorType {
+  kTypeNone = 0,  // none
+  kTypeBase = 1,  // cursor for SCAN
+  kTypeHash = 2,  // cursor for HSCAN
+  kTypeSet = 3,   // cursor for SSCAN
+  kTypeZSet = 4,  // cursor for ZSCAN
+};
+struct CursorDictElement;
+
+class NumberCursor {
+ public:
+  NumberCursor() = default;
+  explicit NumberCursor(CursorType cursor_type, uint16_t counter, const 
std::string &key_name);
+  explicit NumberCursor(uint64_t number_cursor) : cursor_(number_cursor) {}
+  size_t GetIndex() { return cursor_ % CURSOR_DICT_SIZE; }
+  bool IsMatch(const CursorDictElement &element, CursorType cursor_type);

Review Comment:
   better mark these as `const`?



##########
src/server/server.h:
##########
@@ -84,6 +88,39 @@ struct ChannelSubscribeNum {
   size_t subscribe_num;
 };
 
+// CURSOR_DICT_SIZE must be 2^n where n <= 16
+constexpr const size_t CURSOR_DICT_SIZE = 1024 * 16;
+static_assert((CURSOR_DICT_SIZE & (CURSOR_DICT_SIZE - 1)) == 0, 
"CURSOR_DICT_SIZE must be 2^n");
+static_assert(CURSOR_DICT_SIZE <= (1 << 15), "CURSOR_DICT_SIZE must be less 
than or equal to 2^16");

Review Comment:
   Emm is the comment mismatches the code?



-- 
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]

Reply via email to