feat: add command object pool

[lqxhub]

使用对象池来解决命令对象复用, 现在只是一个实现方案的思路验证,还请帮忙多检查

对象复用池在 cmd_object_pool 文件, 这个会代替 cmd_table_manager , 后续相关的文件会删除.

[AlexStocks]

@CodiumAI-Agent /improve

[CodiumAI-Agent]

PR Code Suggestions ✨

Category	Suggestion	Score
Robustness	Implement error handling in object pool methods to enhance robustness ___ **Add error handling for the GetObject and PutObject methods in the CmdObjectPool class to manage exceptions or failures in object retrieval and storage, enhancing robustness.** [src/cmd_object_pool.h [44-47]](https://github.com/OpenAtomFoundation/pikiwidb/pull/260/files#diff-feaf22d6220b833fa557f82d9a5b693d27e238f8bfcd690a7e750a3fd54c611aR44-R47) ```diff -std::unique_ptr GetObject(const std::string &key); -void PutObject(std::string &&key, std::unique_ptr &&v); +std::unique_ptr GetObject(const std::string &key) { + try { + // existing implementation + } catch (const std::exception& e) { + // handle error + } +} +void PutObject(std::string &&key, std::unique_ptr &&v) { + try { + // existing implementation + } catch (const std::exception& e) { + // handle error + } +} ``` Suggestion importance[1-10]: 9 Why: Adding error handling to these methods significantly enhances the robustness of the code by managing exceptions and failures, which is crucial for reliable software.	9
Enhancement	Add validation for flag values before applying bitwise operations ___ **Consider checking the result of bitwise operations for flags to ensure they are valid before applying them. This can prevent unintended behavior due to incorrect flag values.** [src/base_cmd.cc [54-55]](https://github.com/OpenAtomFoundation/pikiwidb/pull/260/files#diff-23c5847b03356e8dd665fe88eaea18e5a2ba197586895ecc56426ea9186f7fafR54-R55) ```diff -void BaseCmd::SetFlag(uint32_t flag) { flag_ |= flag; } -void BaseCmd::ResetFlag(uint32_t flag) { flag_ &= ~flag; } +void BaseCmd::SetFlag(uint32_t flag) { + if (isValidFlag(flag)) { + flag_ |= flag; + } +} +void BaseCmd::ResetFlag(uint32_t flag) { + if (isValidFlag(flag)) { + flag_ &= ~flag; + } +} ``` Suggestion importance[1-10]: 8 Why: Adding validation for flag values before applying bitwise operations can prevent unintended behavior due to incorrect flag values. This is a significant improvement for ensuring the robustness of the code.	8
	Improve error handling by adding logging when a command is not found ___ **It is recommended to handle the case where cmdPtr is nullptr more robustly by logging the error or handling the specific case inside the if (!cmdPtr) block.** [src/cmd_thread_pool_worker.cc [21-30]](https://github.com/OpenAtomFoundation/pikiwidb/pull/260/files#diff-24a4de80190fd7ed32279732bdadf550a2f7d5f3144f9dfe36ecc2e53d981f8aR21-R30) ```diff if (!cmdPtr) { if (ret == CmdRes::kInvalidParameter) { task->Client()->SetRes(CmdRes::kInvalidParameter); } else { + LOG_ERROR("Command not found: " + task->CmdName()); task->Client()->SetRes(CmdRes::kErrOther, "unknown command '" + task->CmdName() + "'"); } g_pikiwidb->PushWriteTask(task->Client()); continue; } ``` Suggestion importance[1-10]: 7 Why: Adding logging when a command is not found improves error handling and makes debugging easier. However, it is a minor enhancement and not critical to the functionality.	7
	Replace placeholder implementation with actual logic for configuration commands ___ **Implement actual command logic in ConfigGetCmd::DoCmd and ConfigSetCmd::DoCmd instead of placeholder text to ensure the commands perform their intended functions.** [src/cmd_admin.cc [23-30]](https://github.com/OpenAtomFoundation/pikiwidb/pull/260/files#diff-a81e422e42fb63b4c1e2d4e05b959e57d0a483e48b2197734ec50a2aab5171c8R23-R30) ```diff -void ConfigGetCmd::DoCmd(PClient* client) { client->AppendString("config cmd in development"); } -void ConfigSetCmd::DoCmd(PClient* client) { client->AppendString("config cmd in development"); } +void ConfigGetCmd::DoCmd(PClient* client) { + // Example implementation + std::string configValue = getConfigValue(client->argv_[1]); + client->AppendString(configValue); +} +void ConfigSetCmd::DoCmd(PClient* client) { + // Example implementation + bool success = setConfigValue(client->argv_[1], client->argv_[2]); + client->AppendString(success ? "Success" : "Failure"); +} ``` Suggestion importance[1-10]: 6 Why: Implementing actual command logic instead of placeholder text is important for the functionality of the commands. However, the suggestion does not provide the actual logic, only a placeholder for it.	6
Best practice	Initialize the command object pool in the PikiwiDB constructor to ensure it is ready for use ___ **Consider initializing the cmd_object_pool_ in the constructor of PikiwiDB to ensure it is properly set up before use. This change ensures that the object pool is always ready when the database instance is created, avoiding potential null pointer dereferences.** [src/pikiwidb.cc [318]](https://github.com/OpenAtomFoundation/pikiwidb/pull/260/files#diff-9dd96a0dfc5c615939f9657275524d8b17baf5e1944bd7435e25c230513dfe15R318-R318) ```diff g_pikiwidb = std::make_unique(); +g_pikiwidb->InitCmdObjectPool(); ``` Suggestion importance[1-10]: 8 Why: Initializing `cmd_object_pool_` in the constructor is a good practice to ensure it is set up before use, preventing potential null pointer dereferences. This suggestion improves the robustness of the code.	8
Maintainability	Remove commented-out code for clarity and maintainability ___ **Remove the commented-out initialization of g_cmd_object_pool in the main function, as it seems to be handled elsewhere in the code. This cleanup will improve code readability and maintainability.** [src/pikiwidb.cc [319]](https://github.com/OpenAtomFoundation/pikiwidb/pull/260/files#diff-9dd96a0dfc5c615939f9657275524d8b17baf5e1944bd7435e25c230513dfe15R319-R319) ```diff -// g_cmd_object_pool = std::make_unique(); +// Initialization of g_cmd_object_pool is handled in the PikiwiDB class. ``` Suggestion importance[1-10]: 7 Why: Removing commented-out code improves readability and maintainability. This suggestion is correct and helps keep the codebase clean.	7
Performance	Optimize the global pool management for better performance in concurrent environments ___ **Consider using a lock-free data structure or fine-grained locking mechanism for managing the global pool in CmdObjectPool to improve performance under high concurrency.** [src/cmd_object_pool.h [70-74]](https://github.com/OpenAtomFoundation/pikiwidb/pull/260/files#diff-feaf22d6220b833fa557f82d9a5b693d27e238f8bfcd690a7e750a3fd54c611aR70-R74) ```diff -std::unordered_map> pool_; -std::mutex mutex_; +// Use concurrent data structure or fine-grained locks +ConcurrentMap> pool_; ``` Suggestion importance[1-10]: 6 Why: While this suggestion could improve performance under high concurrency, it requires careful consideration and testing. It is a good suggestion but not immediately crucial.	6
	Optimize the cleanup logic in the object pool by using a min-heap for better performance ___ **Optimize the object pool cleanup logic by introducing a more efficient algorithm for managing rarely used objects, potentially using a min-heap for tracking object usage.** [src/cmd_object_pool.cc [213-223]](https://github.com/OpenAtomFoundation/pikiwidb/pull/260/files#diff-22a6183190e993fc8f034baf4d877a372bf7454445deb08fe4789dc7ac620067R213-R223) ```diff -for (auto it = tl_local_pool->rbegin(); it != tl_local_pool->rend();) { - if (reclaim_rarely_used && it->count_ > 1) { - break; +// Using min-heap to manage object usage efficiently +if (reclaim_rarely_used) { + std::make_heap(tl_local_pool->begin(), tl_local_pool->end(), [](const SmallObject &a, const SmallObject &b) { + return a.count_ < b.count_; + }); + while (tl_local_pool->size() > local_max_ && tl_local_pool->front().count_ <= 1) { + PutObjectBackGlobal(tl_local_pool->front().key_, tl_local_pool->front().object_); + std::pop_heap(tl_local_pool->begin(), tl_local_pool->end(), [](const SmallObject &a, the SmallObject &b) { + return a.count_ < b.count_; + }); + tl_local_pool->pop_back(); } - PutObjectBackGlobal(it->key_, it->object_); - it = std::vector::reverse_iterator(tl_local_pool->erase((++it).base())); } ``` Suggestion importance[1-10]: 5 Why: Using a min-heap to manage object usage can improve performance, but the current implementation may already be sufficient for the intended use case. This is a performance optimization rather than a critical fix.	5

[AlexStocks]

@CodiumAI-Agent /review

[CodiumAI-Agent]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	4, because the PR introduces a significant amount of new code across multiple files, involving complex logic related to command object pooling and thread-local storage. The changes impact core functionality and require careful review to ensure thread safety, memory management, and correct behavior across different command types.
🧪 Relevant tests	No
⚡ Possible issues	Thread Safety: The use of `thread_local` for `tl_local_pool` and `tl_counter` in `CmdObjectPool` requires careful handling to ensure that objects are managed correctly across different threads.
	Memory Management: The manual management of memory using `std::unique_ptr` in multiple places, especially in `PutObject` and `GetObjectByGlobal`, could lead to potential memory leaks or double frees if not handled correctly.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/cmd_object_pool.cc
suggestion	Consider using a more robust locking mechanism or lock-free data structures for managing the command object pool to enhance performance and avoid potential deadlocks or race conditions. [important]
relevant line	std::lock_guard l(mutex_);

---

relevant file	src/cmd_object_pool.cc
suggestion	Implement logging for error states and unusual behavior, especially in the `GetObject` and `PutObject` methods, to aid in debugging and monitoring the system's behavior. [important]
relevant line	return std::move(obj.object_);

---

relevant file	src/cmd_object_pool.cc
suggestion	Optimize the `GetObjectByGlobal` method by checking if the object can be reused without removal from the global pool, which might reduce the overhead of object creation and destruction. [medium]
relevant line	pool_.erase(it);

---

relevant file	src/cmd_object_pool.cc
suggestion	Refactor the `PutObject` method to encapsulate the logic for reclaiming rarely used objects into a separate function to improve modularity and readability. [medium]
relevant line	bool reclaim_rarely_used = tl_counter % (check_rate_ * less_use_check_rate_) == 0;

---