BewareMyPower commented on code in PR #309:
URL: https://github.com/apache/pulsar-client-cpp/pull/309#discussion_r1312867199
##########
lib/MessageAndCallbackBatch.cc:
##########
@@ -22,59 +22,95 @@
#include "ClientConnection.h"
#include "Commands.h"
-#include "LogUtils.h"
-#include "MessageImpl.h"
-
-DECLARE_LOG_OBJECT()
+#include "CompressionCodec.h"
+#include "MessageCrypto.h"
+#include "OpSendMsg.h"
+#include "PulsarApi.pb.h"
namespace pulsar {
+MessageAndCallbackBatch::MessageAndCallbackBatch() {}
+
+MessageAndCallbackBatch::~MessageAndCallbackBatch() {}
+
void MessageAndCallbackBatch::add(const Message& msg, const SendCallback&
callback) {
- if (empty()) {
- msgImpl_.reset(new MessageImpl);
- Commands::initBatchMessageMetadata(msg, msgImpl_->metadata);
+ if (callbacks_.empty()) {
+ metadata_.reset(new proto::MessageMetadata);
+ Commands::initBatchMessageMetadata(msg, *metadata_);
+ sequenceId_ = metadata_->sequence_id();
}
- LOG_DEBUG(" Before serialization payload size in bytes = " <<
msgImpl_->payload.readableBytes());
- sequenceId_ = Commands::serializeSingleMessageInBatchWithPayload(msg,
msgImpl_->payload,
-
ClientConnection::getMaxMessageSize());
- LOG_DEBUG(" After serialization payload size in bytes = " <<
msgImpl_->payload.readableBytes());
+ messages_.emplace_back(msg);
callbacks_.emplace_back(callback);
-
- ++messagesCount_;
messagesSize_ += msg.getLength();
}
+std::unique_ptr<OpSendMsg> MessageAndCallbackBatch::createOpSendMsg(
+ uint64_t producerId, const ProducerConfiguration& producerConfig,
MessageCrypto* crypto) {
+ auto callback = createSendCallback();
+ if (empty()) {
+ return OpSendMsg::create(ResultOperationNotSupported,
std::move(callback));
+ }
+
+ // The magic number 64 is just an estimated size increment after setting
some fields of the
+ // SingleMessageMetadata. It does not have to be accurate because it's
only used to reduce the
+ // reallocation of the payload buffer.
+ static const size_t kEstimatedHeaderSize =
Review Comment:
I uses an estimated size just to avoid calling
`MessageMetadata::ByteSizeLong()` repeatedly in
`serializeSingleMessageInBatchWithPayload`.
I observed the overhead of `ByteSizeLong` cannot be ignored when I ran perf
tests:
<img width="581" alt="image"
src="https://github.com/apache/pulsar-client-cpp/assets/18204803/ea72f255-6e88-4675-84ea-d87e7530e028";>
However, the flamegraph is based on a debug library. After rechecking the
flamegraph for a release library, I found it does not take much time. So I
think we can simply use an accurate size here just like you suggested.
I will fix it ASAP.
--
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]
