From d922d45d7ff2a8ac4a766cba4537562551a40213 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Tue, 30 Apr 2024 14:22:39 -0400 Subject: [PATCH 1/6] Implement InputStream move --- cpp/include/Ice/InputStream.h | 12 +++++++ cpp/src/Ice/InputStream.cpp | 68 +++++++++++++++++++++++++++++++++-- 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/cpp/include/Ice/InputStream.h b/cpp/include/Ice/InputStream.h index 7524ae9b474..9d6ab7ccb2a 100644 --- a/cpp/include/Ice/InputStream.h +++ b/cpp/include/Ice/InputStream.h @@ -176,6 +176,18 @@ namespace Ice InputStream(const CommunicatorPtr&, const EncodingVersion&, IceInternal::Buffer&, bool = false); /// \endcond + /** + * Move constructor. + * @param other The output stream to move into this output stream. + */ + InputStream(InputStream&& other) noexcept; + + /** + * Move assignment operator. + * @param other The output stream to move into this output stream. + */ + InputStream& operator=(InputStream&& other) noexcept; + ~InputStream() { // Inlined for performance reasons. diff --git a/cpp/src/Ice/InputStream.cpp b/cpp/src/Ice/InputStream.cpp index 6f25a36a2a8..d94f19064e4 100644 --- a/cpp/src/Ice/InputStream.cpp +++ b/cpp/src/Ice/InputStream.cpp @@ -127,6 +127,68 @@ Ice::InputStream::InputStream(Instance* instance, const EncodingVersion& encodin initialize(instance, encoding); } +Ice::InputStream::InputStream(InputStream&& other) noexcept : + Buffer(std::move(other)), + _instance(other._instance), + _encoding(std::move(other._encoding)), + _traceSlicing(other._traceSlicing), + _classGraphDepthMax(other._classGraphDepthMax), + _closure(other._closure), + _sliceValues(other._sliceValues), + _startSeq(other._startSeq), + _minSeqSize(other._minSeqSize), + _valueFactoryManager(std::move(other._valueFactoryManager)), + _logger(std::move(other._logger)), + _compactIdResolver(std::move(other._compactIdResolver)) +{ + // Reset other to its default state. See initialize(). + other._instance = nullptr; + other._encoding = currentEncoding; + other._traceSlicing = false; + other._classGraphDepthMax = 0x7fffffff; + other._closure = nullptr; + other._sliceValues = true; + other._startSeq = -1; + other._minSeqSize = 0; + + resetEncapsulation(); + other.resetEncapsulation(); +} + +InputStream& +Ice::InputStream::operator=(InputStream&& other) noexcept +{ + if (this != &other) + { + Buffer::operator=(std::move(other)); + _instance = other._instance; + _encoding = std::move(other._encoding); + _traceSlicing = other._traceSlicing; + _classGraphDepthMax = other._classGraphDepthMax; + _closure = other._closure; + _sliceValues = other._sliceValues; + _startSeq = other._startSeq; + _minSeqSize = other._minSeqSize; + _valueFactoryManager = std::move(other._valueFactoryManager); + _logger = std::move(other._logger); + _compactIdResolver = std::move(other._compactIdResolver); + + // Reset other to its default state. See initialize(). + other._instance = nullptr; + other._encoding = currentEncoding; + other._traceSlicing = false; + other._classGraphDepthMax = 0x7fffffff; + other._closure = nullptr; + other._sliceValues = true; + other._startSeq = -1; + other._minSeqSize = 0; + + resetEncapsulation(); + other.resetEncapsulation(); + } + return *this; +} + void Ice::InputStream::initialize(const CommunicatorPtr& communicator) { @@ -153,12 +215,12 @@ Ice::InputStream::initialize(Instance* instance, const EncodingVersion& encoding void Ice::InputStream::initialize(const EncodingVersion& encoding) { - _instance = 0; + _instance = nullptr; _encoding = encoding; - _currentEncaps = 0; + _currentEncaps = nullptr; _traceSlicing = false; _classGraphDepthMax = 0x7fffffff; - _closure = 0; + _closure = nullptr; _sliceValues = true; _startSeq = -1; _minSeqSize = 0; From 011877b384ced09da34bce9dfc0cf6a354851bba Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Tue, 30 Apr 2024 14:41:16 -0400 Subject: [PATCH 2/6] Use move ctor/assignment --- cpp/src/Ice/CollocatedRequestHandler.cpp | 3 +-- cpp/src/Ice/ConnectionI.cpp | 5 ++--- cpp/src/Ice/InputStream.cpp | 8 ++++++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/cpp/src/Ice/CollocatedRequestHandler.cpp b/cpp/src/Ice/CollocatedRequestHandler.cpp index bd7f491ce8b..dd790ad95ee 100644 --- a/cpp/src/Ice/CollocatedRequestHandler.cpp +++ b/cpp/src/Ice/CollocatedRequestHandler.cpp @@ -28,12 +28,11 @@ namespace int32_t requestId, int32_t dispatchCount) : _outAsync(outAsync), - _stream(stream.instance(), currentProtocolEncoding), + _stream(std::move(stream)), _handler(handler), _requestId(requestId), _dispatchCount(dispatchCount) { - _stream.swap(stream); } void run() final diff --git a/cpp/src/Ice/ConnectionI.cpp b/cpp/src/Ice/ConnectionI.cpp index e52ebb04cbd..201082db3e9 100644 --- a/cpp/src/Ice/ConnectionI.cpp +++ b/cpp/src/Ice/ConnectionI.cpp @@ -86,9 +86,8 @@ namespace _connectionStartCompleted(std::move(connectionStartCompleted)), _sentCBs(sentCBs), _messageUpcall(std::move(messageUpcall)), - _messageStream(messageStream.instance(), currentProtocolEncoding) + _messageStream(std::move(messageStream)) { - _messageStream.swap(messageStream); } void run() final { _connection->upcall(_connectionStartCompleted, _sentCBs, _messageUpcall, _messageStream); } @@ -3190,7 +3189,7 @@ Ice::ConnectionI::parseMessage(int32_t& upcallCount, functiongetIs()); + *outAsync->getIs() = std::move(stream); #if defined(ICE_USE_IOCP) // diff --git a/cpp/src/Ice/InputStream.cpp b/cpp/src/Ice/InputStream.cpp index d94f19064e4..451e23f899f 100644 --- a/cpp/src/Ice/InputStream.cpp +++ b/cpp/src/Ice/InputStream.cpp @@ -142,7 +142,9 @@ Ice::InputStream::InputStream(InputStream&& other) noexcept : _compactIdResolver(std::move(other._compactIdResolver)) { // Reset other to its default state. See initialize(). - other._instance = nullptr; + + // TODO: reset instance + // other._instance = nullptr; other._encoding = currentEncoding; other._traceSlicing = false; other._classGraphDepthMax = 0x7fffffff; @@ -174,7 +176,9 @@ Ice::InputStream::operator=(InputStream&& other) noexcept _compactIdResolver = std::move(other._compactIdResolver); // Reset other to its default state. See initialize(). - other._instance = nullptr; + + // TODO: reset _instance. + // other._instance = nullptr; other._encoding = currentEncoding; other._traceSlicing = false; other._classGraphDepthMax = 0x7fffffff; From e9325c2c266024660c98a1bee9ea0162e2f0f775 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Tue, 30 Apr 2024 14:46:21 -0400 Subject: [PATCH 3/6] Fix doc comment --- cpp/include/Ice/InputStream.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/include/Ice/InputStream.h b/cpp/include/Ice/InputStream.h index 9d6ab7ccb2a..323bbcb918a 100644 --- a/cpp/include/Ice/InputStream.h +++ b/cpp/include/Ice/InputStream.h @@ -178,13 +178,13 @@ namespace Ice /** * Move constructor. - * @param other The output stream to move into this output stream. + * @param other The input stream to move into this input stream. */ InputStream(InputStream&& other) noexcept; /** * Move assignment operator. - * @param other The output stream to move into this output stream. + * @param other The input stream to move into this input stream. */ InputStream& operator=(InputStream&& other) noexcept; From c5e40f63bbb77406fdeca7f315a9c65890f6f0ab Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Tue, 30 Apr 2024 15:08:26 -0400 Subject: [PATCH 4/6] Reformat --- cpp/src/Ice/InputStream.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/cpp/src/Ice/InputStream.cpp b/cpp/src/Ice/InputStream.cpp index 451e23f899f..e7f80cca083 100644 --- a/cpp/src/Ice/InputStream.cpp +++ b/cpp/src/Ice/InputStream.cpp @@ -127,19 +127,19 @@ Ice::InputStream::InputStream(Instance* instance, const EncodingVersion& encodin initialize(instance, encoding); } -Ice::InputStream::InputStream(InputStream&& other) noexcept : - Buffer(std::move(other)), - _instance(other._instance), - _encoding(std::move(other._encoding)), - _traceSlicing(other._traceSlicing), - _classGraphDepthMax(other._classGraphDepthMax), - _closure(other._closure), - _sliceValues(other._sliceValues), - _startSeq(other._startSeq), - _minSeqSize(other._minSeqSize), - _valueFactoryManager(std::move(other._valueFactoryManager)), - _logger(std::move(other._logger)), - _compactIdResolver(std::move(other._compactIdResolver)) +Ice::InputStream::InputStream(InputStream&& other) noexcept + : Buffer(std::move(other)), + _instance(other._instance), + _encoding(std::move(other._encoding)), + _traceSlicing(other._traceSlicing), + _classGraphDepthMax(other._classGraphDepthMax), + _closure(other._closure), + _sliceValues(other._sliceValues), + _startSeq(other._startSeq), + _minSeqSize(other._minSeqSize), + _valueFactoryManager(std::move(other._valueFactoryManager)), + _logger(std::move(other._logger)), + _compactIdResolver(std::move(other._compactIdResolver)) { // Reset other to its default state. See initialize(). From 9bdba44c5c160f495452f20e04436ddb20961825 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Wed, 1 May 2024 09:35:40 -0400 Subject: [PATCH 5/6] Clear _instance --- cpp/src/Ice/InputStream.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cpp/src/Ice/InputStream.cpp b/cpp/src/Ice/InputStream.cpp index e7f80cca083..d63ad3e4f1c 100644 --- a/cpp/src/Ice/InputStream.cpp +++ b/cpp/src/Ice/InputStream.cpp @@ -143,8 +143,7 @@ Ice::InputStream::InputStream(InputStream&& other) noexcept { // Reset other to its default state. See initialize(). - // TODO: reset instance - // other._instance = nullptr; + other._instance = nullptr; other._encoding = currentEncoding; other._traceSlicing = false; other._classGraphDepthMax = 0x7fffffff; @@ -177,8 +176,7 @@ Ice::InputStream::operator=(InputStream&& other) noexcept // Reset other to its default state. See initialize(). - // TODO: reset _instance. - // other._instance = nullptr; + other._instance = nullptr; other._encoding = currentEncoding; other._traceSlicing = false; other._classGraphDepthMax = 0x7fffffff; From 0679238c773ef4463aafa3918243f7962de1dce7 Mon Sep 17 00:00:00 2001 From: Bernard Normier Date: Wed, 1 May 2024 10:11:04 -0400 Subject: [PATCH 6/6] Cleanup --- cpp/src/Ice/InputStream.cpp | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/cpp/src/Ice/InputStream.cpp b/cpp/src/Ice/InputStream.cpp index d63ad3e4f1c..63aab1ba3d2 100644 --- a/cpp/src/Ice/InputStream.cpp +++ b/cpp/src/Ice/InputStream.cpp @@ -141,19 +141,11 @@ Ice::InputStream::InputStream(InputStream&& other) noexcept _logger(std::move(other._logger)), _compactIdResolver(std::move(other._compactIdResolver)) { - // Reset other to its default state. See initialize(). - - other._instance = nullptr; - other._encoding = currentEncoding; - other._traceSlicing = false; - other._classGraphDepthMax = 0x7fffffff; - other._closure = nullptr; - other._sliceValues = true; - other._startSeq = -1; - other._minSeqSize = 0; - resetEncapsulation(); + + // Reset other to its default state other.resetEncapsulation(); + other.initialize(currentEncoding); } InputStream& @@ -174,19 +166,11 @@ Ice::InputStream::operator=(InputStream&& other) noexcept _logger = std::move(other._logger); _compactIdResolver = std::move(other._compactIdResolver); - // Reset other to its default state. See initialize(). - - other._instance = nullptr; - other._encoding = currentEncoding; - other._traceSlicing = false; - other._classGraphDepthMax = 0x7fffffff; - other._closure = nullptr; - other._sliceValues = true; - other._startSeq = -1; - other._minSeqSize = 0; - resetEncapsulation(); + + // Reset other to its default state. other.resetEncapsulation(); + other.initialize(currentEncoding); } return *this; }