Change FrameWriter::addFrame() to move its argument.

This can prevent copy frame data since c++23
silicon-heaven · Nov 15, 2024 · b6e3fc1 · b6e3fc1
1 parent cd5a61a
commit b6e3fc1
Show file tree

Hide file tree

Showing 13 changed files with 51 additions and 26 deletions.
diff --git a/libshvchainpack/include/shv/chainpack/rpcdriver.h b/libshvchainpack/include/shv/chainpack/rpcdriver.h
@@ -26,7 +26,7 @@ class SHVCHAINPACK_DECL_EXPORT RpcDriver
 protected:
 	virtual bool isOpen() = 0;
 
-	virtual void writeFrameData(const std::string &frame_data) = 0;
+	virtual void writeFrame(RpcFrame &&frame) = 0;
 
 	void processRpcFrame(RpcFrame &&frame);
 	virtual void onRpcFrameReceived(RpcFrame &&frame);

diff --git a/libshvchainpack/include/shv/chainpack/rpcmessage.h b/libshvchainpack/include/shv/chainpack/rpcmessage.h
@@ -26,6 +26,7 @@ struct SHVCHAINPACK_DECL_EXPORT RpcFrame
 	RpcFrame(Rpc::ProtocolType protocol, RpcValue::MetaData &&meta, std::string &&data) : protocol(protocol), meta(std::move(meta)), data(std::move(data)) {}
 	bool isValid() const { return !meta.isEmpty() && !data.empty(); }
 	RpcMessage toRpcMessage(std::string *errmsg = nullptr) const;
+	std::string toFrameHead() const;
 	std::string toFrameData() const;
 	static RpcFrame fromFrameData(const std::string &frame_data);
 };

diff --git a/libshvchainpack/include/shv/chainpack/socketrpcdriver.h b/libshvchainpack/include/shv/chainpack/socketrpcdriver.h
@@ -20,7 +20,7 @@ class SHVCHAINPACK_DECL_EXPORT SocketRpcDriver : public RpcDriver
 	void sendNotify(std::string &&method, const RpcValue &result);
 protected:
 	bool isOpen() override;
-	void writeFrameData(const std::string &frame_data) override;
+	void writeFrame(RpcFrame &&frame) override;
 
 	virtual void onFrameDataRead(const std::string &frame_data);
 	virtual void idleTaskOnSelectTimeout();

diff --git a/libshvchainpack/src/rpcdriver.cpp b/libshvchainpack/src/rpcdriver.cpp
@@ -46,11 +46,13 @@ void RpcDriver::sendRpcFrame(RpcFrame &&frame)
 			if (!errmsg.empty()) {
 				throw std::runtime_error("Cannot convert RPC frame to message: " + errmsg);
 			}
-			frame = msg.toRpcFrame(m_clientProtocolType);
+			auto frame2 = msg.toRpcFrame(m_clientProtocolType);
+			writeFrame(std::move(frame2));
+		}
+		else {
+			//logRpcData().nospace() << "FRAME DATA WRITE " << frame_data.size() << " bytes of data:\n" << shv::chainpack::utils::hexDump(frame_data);
+			writeFrame(std::move(frame));
 		}
-		auto frame_data = frame.toFrameData();
-		//logRpcData().nospace() << "FRAME DATA WRITE " << frame_data.size() << " bytes of data:\n" << shv::chainpack::utils::hexDump(frame_data);
-		writeFrameData(frame_data);
 	}
 	catch (const std::exception &e) {
 		nError() << "ERROR send frame:" << e.what();

diff --git a/libshvchainpack/src/rpcmessage.cpp b/libshvchainpack/src/rpcmessage.cpp
@@ -96,7 +96,7 @@ RpcMessage RpcFrame::toRpcMessage(std::string *errmsg) const
 	return {};
 }
 
-std::string RpcFrame::toFrameData() const
+std::string RpcFrame::toFrameHead() const
 {
 	switch (protocol) {
 	case Rpc::ProtocolType::ChainPack: {
@@ -118,7 +118,6 @@ std::string RpcFrame::toFrameData() const
 		}
 		auto ret = out.str();
 		ret = static_cast<char>(protocol) + ret;
-		ret += data;
 		return ret;
 	}
 	default: {
@@ -127,6 +126,13 @@ std::string RpcFrame::toFrameData() const
 	}
 }
 
+std::string RpcFrame::toFrameData() const
+{
+	auto ret = toFrameHead();
+	ret += data;
+	return ret;
+}
+
 RpcFrame RpcFrame::fromFrameData(const std::string &frame_data)
 {
 	std::istringstream in(frame_data);

diff --git a/libshvchainpack/src/socketrpcdriver.cpp b/libshvchainpack/src/socketrpcdriver.cpp
@@ -56,14 +56,16 @@ void SocketRpcDriver::idleTaskOnSelectTimeout()
 {
 }
 
-void SocketRpcDriver::writeFrameData(const std::string &frame_data)
+void SocketRpcDriver::writeFrame(RpcFrame &&frame)
 {
 	if(!isOpen()) {
 		nInfo() << "Write to closed socket";
 		return;
 	}
-	if (m_writeBuffer.size() + frame_data.size() < m_maxWriteBufferLength) {
-		m_writeBuffer += frame_data;
+	auto frame_head = frame.toFrameHead();
+	if (m_writeBuffer.size() + frame_head.size() + frame.data.size() < m_maxWriteBufferLength) {
+		m_writeBuffer += frame_head;
+		m_writeBuffer += frame.data;
 		flush();
 	}
 }

diff --git a/libshviotqt/include/shv/iotqt/rpc/serialportsocket.h b/libshviotqt/include/shv/iotqt/rpc/serialportsocket.h
@@ -43,8 +43,9 @@ class SHVIOTQT_DECL_EXPORT SerialFrameWriter : public FrameWriter
 	SerialFrameWriter(CrcCheck crc);
 	~SerialFrameWriter() override = default;
 
-	void addFrame(const std::string &frame_data) override;
 	void resetCommunication() override;
+protected:
+	void addFrameData(std::string &&frame_data) override;
 private:
 	bool m_withCrcCheck = true;
 };

diff --git a/libshviotqt/include/shv/iotqt/rpc/socket.h b/libshviotqt/include/shv/iotqt/rpc/socket.h
@@ -45,13 +45,15 @@ class SHVIOTQT_DECL_EXPORT FrameWriter
 {
 public:
 	virtual ~FrameWriter() = default;
-	virtual void addFrame(const std::string &frame_data) = 0;
+	void addFrame(shv::chainpack::RpcFrame &&frame);
 	virtual void resetCommunication() {}
 	void flushToDevice(QIODevice *device);
 	void clear();
 #ifdef WITH_SHV_WEBSOCKETS
 	void flushToWebSocket(QWebSocket *socket);
 #endif
+protected:
+	virtual void addFrameData(std::string &&frame_data) = 0;
 protected:
 	QList<QByteArray> m_messageDataToWrite;
 };
@@ -70,8 +72,8 @@ class SHVIOTQT_DECL_EXPORT StreamFrameWriter : public FrameWriter
 {
 public:
 	~StreamFrameWriter() override = default;
-
-	void addFrame(const std::string &frame_data) override;
+protected:
+	void addFrameData(std::string &&frame_data) override;
 };
 
 /// wrapper class for QTcpSocket and QWebSocket
@@ -103,7 +105,7 @@ class SHVIOTQT_DECL_EXPORT Socket : public QObject
 	virtual quint16 peerPort() const = 0;
 
 	std::vector<chainpack::RpcFrame> takeFrames();
-	void writeFrameData(const std::string &frame_data);
+	void writeFrame(chainpack::RpcFrame &&frame);
 
 	virtual void ignoreSslErrors() = 0;
 

diff --git a/libshviotqt/include/shv/iotqt/rpc/socketrpcconnection.h b/libshviotqt/include/shv/iotqt/rpc/socketrpcconnection.h
@@ -50,7 +50,7 @@ class SHVIOTQT_DECL_EXPORT SocketRpcConnection : public QObject, public shv::cha
 protected:
 	// RpcDriver interface
 	bool isOpen() Q_DECL_OVERRIDE;
-	void writeFrameData(const std::string &frame_data) override;
+	void writeFrame(shv::chainpack::RpcFrame &&frame) override;
 
 	Socket* socket();
 	void onReadyRead();

diff --git a/libshviotqt/src/rpc/serialportsocket.cpp b/libshviotqt/src/rpc/serialportsocket.cpp
@@ -177,7 +177,7 @@ SerialFrameWriter::SerialFrameWriter(CrcCheck crc)
 {
 }
 
-void SerialFrameWriter::addFrame(const std::string &frame_data)
+void SerialFrameWriter::addFrameData(string &&frame_data)
 {
 	QByteArray data_to_write;
 	auto write_escaped = [&data_to_write](uint8_t b) {
@@ -213,7 +213,7 @@ void SerialFrameWriter::resetCommunication()
 
 	QByteArray data_to_write;
 	data_to_write.append(static_cast<char>(00));
-	addFrame(data_to_write.toStdString());
+	addFrameData(data_to_write.toStdString());
 }
 
 //======================================================

diff --git a/libshviotqt/src/rpc/socket.cpp b/libshviotqt/src/rpc/socket.cpp
@@ -24,6 +24,17 @@ namespace shv::iotqt::rpc {
 //======================================================
 // FrameWriter
 //======================================================
+void FrameWriter::addFrame(chainpack::RpcFrame &&frame)
+{
+	try {
+		auto frame_data = frame.toFrameHead();
+		frame_data += frame.data;
+		addFrameData(std::move(frame_data));
+	} catch (const std::runtime_error &e) {
+		shvWarning() << "Error converting frame to data:" << e.what();
+	}
+}
+
 void FrameWriter::flushToDevice(QIODevice *device)
 {
 	while (!m_messageDataToWrite.isEmpty()) {
@@ -160,7 +171,7 @@ QList<int> StreamFrameReader::addData(std::string_view data)
 //======================================================
 // StreamFrameWriter
 //======================================================
-void StreamFrameWriter::addFrame(const std::string &frame_data)
+void StreamFrameWriter::addFrameData(std::string &&frame_data)
 {
 	using namespace shv::chainpack;
 	std::ostringstream out;
@@ -245,10 +256,10 @@ std::vector<chainpack::RpcFrame> Socket::takeFrames()
 	return m_frameReader->takeFrames();
 }
 
-void Socket::writeFrameData(const std::string &frame_data)
+void Socket::writeFrame(shv::chainpack::RpcFrame &&frame)
 {
 	Q_ASSERT(m_frameWriter);
-	m_frameWriter->addFrame(frame_data);
+	m_frameWriter->addFrame(std::move(frame));
 	flushWriteBuffer();
 }
 

diff --git a/libshviotqt/src/rpc/socketrpcconnection.cpp b/libshviotqt/src/rpc/socketrpcconnection.cpp
@@ -111,9 +111,9 @@ bool SocketRpcConnection::isOpen()
 	return isSocketConnected();
 }
 
-void SocketRpcConnection::writeFrameData(const std::string &frame_data)
+void SocketRpcConnection::writeFrame(chainpack::RpcFrame &&frame)
 {
-	socket()->writeFrameData(frame_data);
+	socket()->writeFrame(std::move(frame));
 }
 
 void SocketRpcConnection::sendRpcMessage(const shv::chainpack::RpcMessage &rpc_msg)

diff --git a/libshviotqt/tests/test_frame_reader.cpp b/libshviotqt/tests/test_frame_reader.cpp
@@ -36,7 +36,7 @@ vector<string> msg_to_raw_data(const vector<string> &cpons)
 		auto rv = RpcValue::fromCpon(cpon);
 		auto msg = RpcMessage(rv);
 		StreamFrameWriter wr;
-		wr.addFrame(msg.toRpcFrame().toFrameData());
+		wr.addFrame(msg.toRpcFrame());
 		QByteArray ba;
 		{
 			QBuffer buffer(&ba);
@@ -54,7 +54,7 @@ vector<string> msg_to_raw_data_serial(const vector<string> &cpons, SerialFrameWr
 		auto rv = RpcValue::fromCpon(cpon);
 		auto msg = RpcMessage(rv);
 		SerialFrameWriter wr(crc_check);
-		wr.addFrame(msg.toRpcFrame().toFrameData());
+		wr.addFrame(msg.toRpcFrame());
 		QByteArray ba;
 		{
 			QBuffer buffer(&ba);