fix: clear recycled CC message payloads #563
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| /** | ||
| * Copyright (C) 2025 EloqData Inc. | ||
| * | ||
| * This program is free software: you can redistribute it and/or modify | ||
| * it under either of the following two licenses: | ||
| * 1. GNU Affero General Public License, version 3, as published by the Free | ||
| * Software Foundation. | ||
| * 2. GNU General Public License as published by the Free Software | ||
| * Foundation; version 2 of the License. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| * GNU Affero General Public License or GNU General Public License for more | ||
| * details. | ||
| * | ||
| * You should have received a copy of the GNU Affero General Public License | ||
| * and GNU General Public License V2 along with this program. If not, see | ||
| * <http://www.gnu.org/licenses/>. | ||
| */ | ||
| #pragma once | ||
|
|
||
| #include <bthread/moodycamelqueue.h> | ||
|
|
||
| #include <memory> | ||
| #include <utility> | ||
|
|
||
| #include "proto/cc_request.pb.h" | ||
|
|
||
| namespace txservice::remote | ||
| { | ||
| /** | ||
| * @brief Thread-safe reuse pool for CC protobuf messages. | ||
| * | ||
| * Recycle clears a message before making it available to another thread. This | ||
| * keeps the idle pool from retaining payload allocations that ParseFrom would | ||
| * discard before the next use anyway. | ||
| */ | ||
| class CcMessagePool | ||
| { | ||
| public: | ||
| CcMessagePool() = default; | ||
| CcMessagePool(const CcMessagePool &) = delete; | ||
| CcMessagePool &operator=(const CcMessagePool &) = delete; | ||
| CcMessagePool(CcMessagePool &&) = delete; | ||
| CcMessagePool &operator=(CcMessagePool &&) = delete; | ||
|
|
||
| /** | ||
| * @brief Returns an empty pooled message or allocates a new one. | ||
| * | ||
| * The caller owns the returned message until passing it to Recycle(). | ||
| */ | ||
| std::unique_ptr<CcMessage> Acquire() | ||
| { | ||
| std::unique_ptr<CcMessage> msg; | ||
| if (!pool_.try_dequeue(msg)) | ||
| { | ||
| msg = std::make_unique<CcMessage>(); | ||
| } | ||
| return msg; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Clears and transfers a message back to the shared pool. | ||
| * | ||
| * @param msg A non-null message exclusively owned by the caller. | ||
| */ | ||
| void Recycle(std::unique_ptr<CcMessage> msg) | ||
| { | ||
| msg->Clear(); | ||
|
There was a problem hiding this comment. 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift 🔎 Supported by static analysis🏁 Script executed: printf '%s\n' '--- scoped conventions and learnings ---'
find /tmp/coderabbit-repo-knowledge/eloqdata-tx-service-e445869e -type f -name '*.md' -maxdepth 3 -print
printf '%s\n' '--- target file ---'
cat -n tx_service/include/remote/cc_message_pool.h
printf '%s\n' '--- directly bound message and pool definitions ---'
rg -n --glob '*.{h,hpp,cc,cpp}' 'class CcMessage|struct CcMessage|CcMessagePool|void Clear|Clear\(\)' tx_service/include tx_service/src 2>/dev/null | head -160Repository: eloqdata/tx_service Length of output: 13114 🏁 Script executed: printf '%s\n' '--- repository-wide conventions ---'
cat /tmp/coderabbit-repo-knowledge/eloqdata-tx-service-e445869e/conventions/repo-wide.md
printf '%s\n' '--- tx-service conventions ---'
cat /tmp/coderabbit-repo-knowledge/eloqdata-tx-service-e445869e/conventions/tx-service.md
printf '%s\n' '--- generated message declaration and proto source ---'
rg -n --glob '*.{h,hpp,cc,cpp,proto}' 'CcMessage|cc_request\.pb|protobuf|Protobuf' tx_service proto CMakeLists.txt cmake Makefile* 2>/dev/null | head -220
printf '%s\n' '--- pool callers and lifecycle ---'
rg -n -A18 -B8 'Acquire\(\)|Recycle\(' tx_service/include/remote tx_service/src/remote tx_service/include/sharder.h
printf '%s\n' '--- dependency declarations ---'
rg -n -i 'protobuf|protobuf-cpp|protobuf_VERSION' --glob '!*build*' --glob '!*.pb.*' . | head -160Repository: eloqdata/tx_service Length of output: 50376 🏁 Script executed: printf '%s\n' '--- CcMessage proto and generated-file locations ---'
fd -i 'cc_request(\.proto|\.pb\.(h|cc))$' .
printf '%s\n' '--- CcMessage proto definition ---'
proto_file=$(fd -i -t f 'cc_request.proto' . | head -1)
if [ -n "$proto_file" ]; then
rg -n -A80 -B10 'message CcMessage' "$proto_file"
fi
printf '%s\n' '--- focused pool test ---'
cat -n tx_service/tests/CcMessagePool-Test.cpp
printf '%s\n' '--- Protobuf dependency pins ---'
rg -n -i --glob '!*build*' --glob '!*.pb.*' --glob 'CMakeLists.txt' --glob '*.cmake' --glob '*.yml' --glob '*.yaml' --glob '*.toml' --glob '*.json' --glob 'Makefile*' 'protobuf|PROTOBUF' . | head -100Repository: eloqdata/tx_service Length of output: 10024 🌐 Web query:
💡 Result: In Protocol Buffers C++, the Clear method is designed to clear all fields of a message and reset them to their default states, but it typically does not free the allocated memory [1][2]. Instead, Protobuf message objects are designed to retain allocated memory to improve performance through reuse [1]. When you clear a message, it holds onto the memory previously used for its fields, assuming that this memory will be needed again for future messages [1][2]. This behavior helps avoid frequent and expensive memory allocations and deallocations when processing sequences of messages with similar structures [1]. If your goal is to genuinely release the memory held by a Protobuf message, you should destroy the message object itself [3][2]. If you need to repeatedly clear a message while controlling memory usage, consider storing the message object in a smart pointer (e.g., std::unique_ptr) and resetting the pointer as needed [3]. Additionally, if you are using Arena allocation, keep in mind that memory is only released when the entire Arena object is destroyed [4][5]. Regarding SpaceUsedLong: - SpaceUsedLong computes an estimate of the total number of bytes currently used by the message in memory [6][7]. - It is implemented using reflection, making it significantly slower than ByteSizeLong (which measures the size of the serialized binary message) [6][7]. - Because it is an estimate, the precise value returned should not be depended upon, and internal details or debug build configurations can cause the returned size to change [6]. - You can use this method to monitor when a reused message object becomes too bloated—if it exceeds a certain threshold, you might choose to delete and recreate the object to reclaim memory [1]. top_results: [3][1][6][7][2] Citations:
Do not rely on
🤖 Prompt for AI Agents |
||
| pool_.enqueue(std::move(msg)); | ||
| } | ||
|
|
||
| private: | ||
| moodycamel::ConcurrentQueue<std::unique_ptr<CcMessage>> pool_; | ||
| }; | ||
| } // namespace txservice::remote | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Document the unbounded pool constraint.
Acquire()can allocate indefinitely, andRecycle()retains every accepted message. Document that this pool has no capacity limit or eviction policy, and state the expected lifetime and memory tradeoff.As per coding guidelines, “Document non-obvious invariants and operational constraints, including ... hot-path tradeoffs.”
🤖 Prompt for AI Agents
Source: Coding guidelines