diff options
author | Adam Midvidy <amidvidy@gmail.com> | 2015-10-22 10:34:51 -0400 |
---|---|---|
committer | Adam Midvidy <amidvidy@gmail.com> | 2015-10-30 15:47:26 -0400 |
commit | 791c412874ce87d9cc1eac75edce8116a9d40640 (patch) | |
tree | cdaaeaefd1d093d88ff9a5332a68a8e440b124fa /src | |
parent | e62e2e71eff397caf22a0da13ac4669a8546b298 (diff) | |
download | mongo-791c412874ce87d9cc1eac75edce8116a9d40640.tar.gz |
SERVER-20609 do not heap allocate Message
Diffstat (limited to 'src')
35 files changed, 163 insertions, 177 deletions
diff --git a/src/mongo/client/dbclient.cpp b/src/mongo/client/dbclient.cpp index 7e0196d7ff5..6f7407355da 100644 --- a/src/mongo/client/dbclient.cpp +++ b/src/mongo/client/dbclient.cpp @@ -309,7 +309,7 @@ rpc::UniqueReply DBClientWithCommands::runCommandWithMetadata(StringData databas requestBuilder->setCommandArgs(commandArgs); auto requestMsg = requestBuilder->done(); - auto replyMsg = stdx::make_unique<Message>(); + Message replyMsg; // We always want to throw if there was a network error, we do it here // instead of passing 'true' for the 'assertOk' parameter so we can construct a @@ -318,14 +318,14 @@ rpc::UniqueReply DBClientWithCommands::runCommandWithMetadata(StringData databas str::stream() << "network error while attempting to run " << "command '" << command << "' " << "on host '" << host << "' ", - call(*requestMsg, *replyMsg, false, &host)); + call(requestMsg, replyMsg, false, &host)); - auto commandReply = rpc::makeReply(replyMsg.get()); + auto commandReply = rpc::makeReply(&replyMsg); uassert(ErrorCodes::RPCProtocolNegotiationFailed, str::stream() << "Mismatched RPC protocols - request was '" - << opToString(requestMsg->operation()) << "' '" - << " but reply was '" << opToString(replyMsg->operation()) << "' ", + << opToString(requestMsg.operation()) << "' '" + << " but reply was '" << opToString(replyMsg.operation()) << "' ", requestBuilder->getProtocol() == commandReply->getProtocol()); if (ErrorCodes::SendStaleConfig == diff --git a/src/mongo/client/dbclientcursor.cpp b/src/mongo/client/dbclientcursor.cpp index b5100b98bcc..00eb939da30 100644 --- a/src/mongo/client/dbclientcursor.cpp +++ b/src/mongo/client/dbclientcursor.cpp @@ -59,10 +59,10 @@ namespace { * This code is mostly duplicated from DBClientWithCommands::runCommand. It may not * be worth de-duplicating as this codepath will eventually be removed anyway. */ -std::unique_ptr<Message> assembleCommandRequest(DBClientWithCommands* cli, - StringData database, - int legacyQueryOptions, - BSONObj legacyQuery) { +Message assembleCommandRequest(DBClientWithCommands* cli, + StringData database, + int legacyQueryOptions, + BSONObj legacyQuery) { // TODO: Rewrite this to a common utility shared between this and DBClientMultiCommand. // Can be an OP_COMMAND or OP_QUERY message. @@ -117,8 +117,7 @@ void DBClientCursor::_assembleInit(Message& toSend) { bool hasValidFlagsForCommand = !(opts & mongo::QueryOption_Exhaust); if (_isCommand && hasValidNToReturnForCommand && hasValidFlagsForCommand) { - toSend = - std::move(*assembleCommandRequest(_client, nsToDatabaseSubstring(ns), opts, query)); + toSend = assembleCommandRequest(_client, nsToDatabaseSubstring(ns), opts, query); return; } assembleQueryRequest(ns, query, nextBatchSize(), nToSkip, fieldsToReturn, opts, toSend); @@ -137,12 +136,12 @@ bool DBClientCursor::init() { Message toSend; _assembleInit(toSend); verify(_client); - if (!_client->call(toSend, *batch.m, false, &_originalHost)) { + if (!_client->call(toSend, batch.m, false, &_originalHost)) { // log msg temp? log() << "DBClientCursor::init call() failed" << endl; return false; } - if (batch.m->empty()) { + if (batch.m.empty()) { // log msg temp? log() << "DBClientCursor::init message from call() was empty" << endl; return false; @@ -161,13 +160,13 @@ void DBClientCursor::initLazy(bool isRetry) { } bool DBClientCursor::initLazyFinish(bool& retry) { - bool recvd = _client->recv(*batch.m); + bool recvd = _client->recv(batch.m); // If we get a bad response, return false - if (!recvd || batch.m->empty()) { + if (!recvd || batch.m.empty()) { if (!recvd) log() << "DBClientCursor::init lazy say() failed" << endl; - if (batch.m->empty()) + if (batch.m.empty()) log() << "DBClientCursor::init message from say() was empty" << endl; _client->checkResponse(NULL, -1, &retry, &_lazyHost); @@ -184,7 +183,7 @@ bool DBClientCursor::initCommand() { BSONObj res; bool ok = _client->runCommand(nsGetDB(ns), query, res, opts); - replyToQuery(0, *batch.m, res); + replyToQuery(0, batch.m, res); dataReceived(); return ok; @@ -205,16 +204,16 @@ void DBClientCursor::requestMore() { Message toSend; toSend.setData(dbGetMore, b.buf(), b.len()); - unique_ptr<Message> response(new Message()); + Message response; if (_client) { - _client->call(toSend, *response); + _client->call(toSend, response); this->batch.m = std::move(response); dataReceived(); } else { verify(_scopedHost.size()); ScopedDbConnection conn(_scopedHost); - conn->call(toSend, *response); + conn->call(toSend, response); _client = conn.get(); ON_BLOCK_EXIT([this] { _client = nullptr; }); this->batch.m = std::move(response); @@ -227,9 +226,9 @@ void DBClientCursor::requestMore() { void DBClientCursor::exhaustReceiveMore() { verify(cursorId && batch.pos == batch.nReturned); verify(!haveLimit); - unique_ptr<Message> response(new Message()); + Message response; verify(_client); - if (!_client->recv(*response)) { + if (!_client->recv(response)) { uasserted(16465, "recv failed while exhausting cursor"); } batch.m = std::move(response); @@ -237,13 +236,13 @@ void DBClientCursor::exhaustReceiveMore() { } void DBClientCursor::commandDataReceived() { - int op = batch.m->operation(); + int op = batch.m.operation(); invariant(op == opReply || op == dbCommandReply); batch.nReturned = 1; batch.pos = 0; - auto commandReply = rpc::makeReply(batch.m.get()); + auto commandReply = rpc::makeReply(&batch.m); auto commandStatus = getStatusFromCommandResult(commandReply->getCommandReply()); @@ -264,11 +263,11 @@ void DBClientCursor::commandDataReceived() { if (op == dbCommandReply) { // Need to take ownership here as we destroy the underlying message. BSONObj reply = commandReply->getCommandReply().getOwned(); - batch.m = stdx::make_unique<Message>(); - replyToQuery(0, *batch.m, reply); + batch.m.reset(); + replyToQuery(0, batch.m, reply); } - QueryResult::View qr = batch.m->singleData().view2ptr(); + QueryResult::View qr = batch.m.singleData().view2ptr(); batch.data = qr.data(); } @@ -279,7 +278,7 @@ void DBClientCursor::dataReceived(bool& retry, string& host) { return; } - QueryResult::View qr = batch.m->singleData().view2ptr(); + QueryResult::View qr = batch.m.singleData().view2ptr(); resultFlags = qr.getResultFlags(); if (qr.getResultFlags() & ResultFlag_ErrSet) { diff --git a/src/mongo/client/dbclientcursor.h b/src/mongo/client/dbclientcursor.h index 2dede943b4b..a085d90dbd9 100644 --- a/src/mongo/client/dbclientcursor.h +++ b/src/mongo/client/dbclientcursor.h @@ -191,7 +191,7 @@ public: } Message* getMessage() { - return batch.m.get(); + return &batch.m; } /** @@ -215,13 +215,13 @@ public: class Batch { MONGO_DISALLOW_COPYING(Batch); friend class DBClientCursor; - std::unique_ptr<Message> m; - int nReturned; - int pos; - const char* data; + Message m; + int nReturned{0}; + int pos{0}; + const char* data{nullptr}; public: - Batch() : m(new Message()), nReturned(), pos(), data() {} + Batch() = default; }; /** diff --git a/src/mongo/client/scoped_db_conn_test.cpp b/src/mongo/client/scoped_db_conn_test.cpp index b032ed2651c..bbc805dfbf2 100644 --- a/src/mongo/client/scoped_db_conn_test.cpp +++ b/src/mongo/client/scoped_db_conn_test.cpp @@ -119,10 +119,12 @@ public: commandResponse.append("minWireVersion", WireVersion::RELEASE_2_4_AND_BEFORE); } - port->reply(m, - *reply->setMetadata(rpc::makeEmptyMetadata()) - .setCommandReply(commandResponse.done()) - .done()); + + auto response = reply->setMetadata(rpc::makeEmptyMetadata()) + .setCommandReply(commandResponse.done()) + .done(); + + port->reply(m, response); } } dummyHandler; diff --git a/src/mongo/db/db.cpp b/src/mongo/db/db.cpp index 4f2d3676f19..ce1a9bfc4a3 100644 --- a/src/mongo/db/db.cpp +++ b/src/mongo/db/db.cpp @@ -173,10 +173,10 @@ public: // results after the response reaches the client } - if (dbresponse.response) { - port->reply(m, *dbresponse.response, dbresponse.responseTo); + if (!dbresponse.response.empty()) { + port->reply(m, dbresponse.response, dbresponse.responseTo); if (dbresponse.exhaustNS.size() > 0) { - MsgData::View header = dbresponse.response->header(); + MsgData::View header = dbresponse.response.header(); QueryResult::View qr = header.view2ptr(); long long cursorid = qr.getCursorId(); if (cursorid) { diff --git a/src/mongo/db/dbdirectclient.cpp b/src/mongo/db/dbdirectclient.cpp index 244669481c5..519cade63ad 100644 --- a/src/mongo/db/dbdirectclient.cpp +++ b/src/mongo/db/dbdirectclient.cpp @@ -121,11 +121,11 @@ bool DBDirectClient::call(Message& toSend, Message& response, bool assertOk, str DbResponse dbResponse; CurOp curOp(_txn); assembleResponse(_txn, toSend, dbResponse, dummyHost); - verify(dbResponse.response); + verify(!dbResponse.response.empty()); // can get rid of this if we make response handling smarter - dbResponse.response->concat(); - response = std::move(*dbResponse.response); + dbResponse.response.concat(); + response = std::move(dbResponse.response); return true; } diff --git a/src/mongo/db/dbmessage.cpp b/src/mongo/db/dbmessage.cpp index eb50ecb063a..5750542eb26 100644 --- a/src/mongo/db/dbmessage.cpp +++ b/src/mongo/db/dbmessage.cpp @@ -201,9 +201,9 @@ void replyToQuery(int queryResultFlags, } void replyToQuery(int queryResultFlags, Message& m, DbResponse& dbresponse, BSONObj obj) { - Message* resp = new Message(); - replyToQuery(queryResultFlags, *resp, obj); - dbresponse.response = resp; + Message resp; + replyToQuery(queryResultFlags, resp, obj); + dbresponse.response = std::move(resp); dbresponse.responseTo = m.header().getId(); } diff --git a/src/mongo/db/dbmessage.h b/src/mongo/db/dbmessage.h index aa74fd59ffb..2f0c6b6645b 100644 --- a/src/mongo/db/dbmessage.h +++ b/src/mongo/db/dbmessage.h @@ -312,16 +312,11 @@ public: * A response to a DbMessage. */ struct DbResponse { - Message* response; + Message response; MSGID responseTo; std::string exhaustNS; /* points to ns if exhaust mode. 0=normal mode*/ - DbResponse(Message* r, MSGID rt) : response(r), responseTo(rt) {} - DbResponse() { - response = 0; - } - ~DbResponse() { - delete response; - } + DbResponse(Message r, MSGID rt) : response(std::move(r)), responseTo(rt) {} + DbResponse() = default; }; void replyToQuery(int queryResultFlags, diff --git a/src/mongo/db/dbwebserver.cpp b/src/mongo/db/dbwebserver.cpp index 1ad6e54a236..b60e1dd77c3 100644 --- a/src/mongo/db/dbwebserver.cpp +++ b/src/mongo/db/dbwebserver.cpp @@ -280,13 +280,13 @@ public: .setCommandArgs(cmdObj); auto cmdRequestMsg = requestBuilder.done(); - rpc::CommandRequest cmdRequest{cmdRequestMsg.get()}; + rpc::CommandRequest cmdRequest{&cmdRequestMsg}; rpc::CommandReplyBuilder cmdReplyBuilder{}; Command::execCommand(txn, c, cmdRequest, &cmdReplyBuilder); auto cmdReplyMsg = cmdReplyBuilder.done(); - rpc::CommandReply cmdReply{cmdReplyMsg.get()}; + rpc::CommandReply cmdReply{&cmdReplyMsg}; responseCode = 200; diff --git a/src/mongo/db/ftdc/SConscript b/src/mongo/db/ftdc/SConscript index 1527dbdd5f9..b2ff0255bfb 100644 --- a/src/mongo/db/ftdc/SConscript +++ b/src/mongo/db/ftdc/SConscript @@ -63,4 +63,3 @@ env.CppUnitTest( 'ftdc', ], ) - diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp index 59e3bebb214..e752ae92fa6 100644 --- a/src/mongo/db/instance.cpp +++ b/src/mongo/db/instance.cpp @@ -260,9 +260,9 @@ static void receivedCommand(OperationContext* txn, auto response = builder.done(); - op->debug().responseLength = response->header().dataLen(); + op->debug().responseLength = response.header().dataLen(); - dbResponse.response = response.release(); + dbResponse.response = std::move(response); dbResponse.responseTo = responseTo; } @@ -301,9 +301,9 @@ void receivedRpc(OperationContext* txn, Client& client, DbResponse& dbResponse, auto response = replyBuilder.done(); - curOp->debug().responseLength = response->header().dataLen(); + curOp->debug().responseLength = response.header().dataLen(); - dbResponse.response = response.release(); + dbResponse.response = std::move(response); dbResponse.responseTo = responseTo; } @@ -378,7 +378,6 @@ static void receivedQuery(OperationContext* txn, DbMessage d(m); QueryMessage q(d); - unique_ptr<Message> resp(new Message()); CurOp& op = *CurOp::get(txn); @@ -388,16 +387,13 @@ static void receivedQuery(OperationContext* txn, audit::logQueryAuthzCheck(client, nss, q.query, status.code()); uassertStatusOK(status); - dbResponse.exhaustNS = runQuery(txn, q, nss, *resp); - verify(!resp->empty()); + dbResponse.exhaustNS = runQuery(txn, q, nss, dbResponse.response); } catch (const AssertionException& exception) { - resp.reset(new Message()); - generateLegacyQueryErrorResponse(&exception, q, &op, resp.get()); + dbResponse.response.reset(); + generateLegacyQueryErrorResponse(&exception, q, &op, &dbResponse.response); } - op.debug().responseLength = resp->header().dataLen(); - - dbResponse.response = resp.release(); + op.debug().responseLength = dbResponse.response.header().dataLen(); dbResponse.responseTo = responseTo; } @@ -536,13 +532,11 @@ void assembleResponse(OperationContext* txn, log(LogComponent::kQuery) << curTimeMillis64() % 10000 << " long msg received, len:" << len << endl; - Message* resp = new Message(); if (strcmp("end", p) == 0) - resp->setData(opReply, "dbMsg end no longer supported"); + dbresponse.response.setData(opReply, "dbMsg end no longer supported"); else - resp->setData(opReply, "i am fine - dbMsg deprecated"); + dbresponse.response.setData(opReply, "i am fine - dbMsg deprecated"); - dbresponse.response = resp; dbresponse.responseTo = m.header().getId(); } else { try { @@ -923,17 +917,15 @@ bool receivedGetMore(OperationContext* txn, DbResponse& dbresponse, Message& m, curop.debug().exceptionInfo = e.getInfo(); replyToQuery(ResultFlag_ErrSet, m, dbresponse, errObj); - curop.debug().responseLength = dbresponse.response->header().dataLen(); + curop.debug().responseLength = dbresponse.response.header().dataLen(); curop.debug().nreturned = 1; return false; } - Message* resp = new Message(); - resp->setData(msgdata.view2ptr(), true); - curop.debug().responseLength = resp->header().dataLen(); + dbresponse.response.setData(msgdata.view2ptr(), true); + curop.debug().responseLength = dbresponse.response.header().dataLen(); curop.debug().nreturned = msgdata.getNReturned(); - dbresponse.response = resp; dbresponse.responseTo = m.header().getId(); if (exhaust) { @@ -1106,11 +1098,11 @@ static void insertSystemIndexes(OperationContext* txn, DbMessage& d, CurOp& curO .setMetadata(rpc::makeEmptyMetadata()) .setCommandArgs(cmdObj) .done(); - rpc::LegacyRequest cmdRequest{cmdRequestMsg.get()}; + rpc::LegacyRequest cmdRequest{&cmdRequestMsg}; rpc::LegacyReplyBuilder cmdReplyBuilder{}; Command::execCommand(txn, createIndexesCmd, cmdRequest, &cmdReplyBuilder); auto cmdReplyMsg = cmdReplyBuilder.done(); - rpc::LegacyReply cmdReply{cmdReplyMsg.get()}; + rpc::LegacyReply cmdReply{&cmdReplyMsg}; uassertStatusOK(Command::getStatusFromCommandResult(cmdReply.getCommandReply())); } catch (const DBException& ex) { LastError::get(txn->getClient()).setLastError(ex.getCode(), ex.getInfo().msg); diff --git a/src/mongo/dbtests/mock/mock_remote_db_server.cpp b/src/mongo/dbtests/mock/mock_remote_db_server.cpp index 963677012f6..d4949e171d6 100644 --- a/src/mongo/dbtests/mock/mock_remote_db_server.cpp +++ b/src/mongo/dbtests/mock/mock_remote_db_server.cpp @@ -172,7 +172,7 @@ rpc::UniqueReply MockRemoteDBServer::runCommandWithMetadata(MockRemoteDBServer:: .setMetadata(rpc::makeEmptyMetadata()) .setCommandReply(reply) .done(); - auto replyView = stdx::make_unique<rpc::CommandReply>(message.get()); + auto replyView = stdx::make_unique<rpc::CommandReply>(&message); return rpc::UniqueReply(std::move(message), std::move(replyView)); } diff --git a/src/mongo/executor/async_mock_stream_factory.cpp b/src/mongo/executor/async_mock_stream_factory.cpp index 3132bab52b7..341615bdbce 100644 --- a/src/mongo/executor/async_mock_stream_factory.cpp +++ b/src/mongo/executor/async_mock_stream_factory.cpp @@ -308,22 +308,22 @@ void AsyncMockStreamFactory::MockStream::simulateServer( replyBuilder->setCommandReply(resp.data); auto replyMsg = replyBuilder->done(); - replyMsg->header().setResponseTo(messageId); + replyMsg.header().setResponseTo(messageId); { // The first read will be for the header. ReadEvent read{this}; - auto hdrBytes = reinterpret_cast<const uint8_t*>(replyMsg->header().view2ptr()); + auto hdrBytes = reinterpret_cast<const uint8_t*>(replyMsg.header().view2ptr()); pushRead({hdrBytes, hdrBytes + sizeof(MSGHEADER::Value)}); } { // The second read will be for the message data. ReadEvent read{this}; - auto dataBytes = reinterpret_cast<const uint8_t*>(replyMsg->buf()); + auto dataBytes = reinterpret_cast<const uint8_t*>(replyMsg.buf()); auto pastHeader = dataBytes; std::advance(pastHeader, sizeof(MSGHEADER::Value)); - pushRead({pastHeader, dataBytes + static_cast<std::size_t>(replyMsg->size())}); + pushRead({pastHeader, dataBytes + static_cast<std::size_t>(replyMsg.size())}); } if (ex) { diff --git a/src/mongo/executor/connection_pool_asio.cpp b/src/mongo/executor/connection_pool_asio.cpp index ea494194053..d63ef0488cc 100644 --- a/src/mongo/executor/connection_pool_asio.cpp +++ b/src/mongo/executor/connection_pool_asio.cpp @@ -146,7 +146,7 @@ Message ASIOConnection::makeIsMasterRequest(ASIOConnection* conn) { requestBuilder.setMetadata(rpc::makeEmptyMetadata()); requestBuilder.setCommandArgs(BSON("isMaster" << 1)); - return std::move(*(requestBuilder.done())); + return requestBuilder.done(); } void ASIOConnection::setTimeout(Milliseconds timeout, TimeoutCallback cb) { diff --git a/src/mongo/executor/network_interface_asio_auth.cpp b/src/mongo/executor/network_interface_asio_auth.cpp index 9202fbaf51e..a44f2cd2ca1 100644 --- a/src/mongo/executor/network_interface_asio_auth.cpp +++ b/src/mongo/executor/network_interface_asio_auth.cpp @@ -59,7 +59,7 @@ void NetworkInterfaceASIO::_runIsMaster(AsyncOp* op) { // Set current command to ismaster request and run auto beginStatus = op->beginCommand( - std::move(*(requestBuilder.done())), AsyncCommand::CommandType::kRPC, op->request().target); + requestBuilder.done(), AsyncCommand::CommandType::kRPC, op->request().target); if (!beginStatus.isOK()) { return _completeOperation(op, beginStatus); } diff --git a/src/mongo/executor/network_interface_asio_operation.cpp b/src/mongo/executor/network_interface_asio_operation.cpp index e890c7c4e5f..dbce0eb9c73 100644 --- a/src/mongo/executor/network_interface_asio_operation.cpp +++ b/src/mongo/executor/network_interface_asio_operation.cpp @@ -53,9 +53,9 @@ using asio::ip::tcp; namespace { // Metadata listener can be nullptr. -StatusWith<std::unique_ptr<Message>> messageFromRequest(const RemoteCommandRequest& request, - rpc::Protocol protocol, - rpc::EgressMetadataHook* metadataHook) { +StatusWith<Message> messageFromRequest(const RemoteCommandRequest& request, + rpc::Protocol protocol, + rpc::EgressMetadataHook* metadataHook) { BSONObj query = request.cmdObj; auto requestBuilder = rpc::makeRequestBuilder(protocol); @@ -179,7 +179,7 @@ Status NetworkInterfaceASIO::AsyncOp::beginCommand(const RemoteCommandRequest& r return newCommand.getStatus(); } return beginCommand( - std::move(*newCommand.getValue()), AsyncCommand::CommandType::kRPC, request.target); + std::move(newCommand.getValue()), AsyncCommand::CommandType::kRPC, request.target); } else if (isFindCmd) { auto downconvertedFind = downconvertFindCommandRequest(request); if (!downconvertedFind.isOK()) { diff --git a/src/mongo/executor/network_interface_asio_test.cpp b/src/mongo/executor/network_interface_asio_test.cpp index ceabe364287..baed65f122e 100644 --- a/src/mongo/executor/network_interface_asio_test.cpp +++ b/src/mongo/executor/network_interface_asio_test.cpp @@ -424,24 +424,24 @@ public: replyBuilder->setCommandReply(BSON("hello!" << 1)); auto message = replyBuilder->done(); - message->header().setResponseTo(messageId); + message.header().setResponseTo(messageId); - auto actualSize = message->header().getLen(); + auto actualSize = message.header().getLen(); // Allow caller to mess with the Message - hook(message->header()); + hook(message.header()); { // Load the header ReadEvent read{stream}; - auto headerBytes = reinterpret_cast<const uint8_t*>(message->header().view2ptr()); + auto headerBytes = reinterpret_cast<const uint8_t*>(message.header().view2ptr()); stream->pushRead({headerBytes, headerBytes + sizeof(MSGHEADER::Value)}); } if (loadBody) { // Load the body if we need to ReadEvent read{stream}; - auto dataBytes = reinterpret_cast<const uint8_t*>(message->buf()); + auto dataBytes = reinterpret_cast<const uint8_t*>(message.buf()); auto body = dataBytes; std::advance(body, sizeof(MSGHEADER::Value)); stream->pushRead({body, dataBytes + static_cast<std::size_t>(actualSize)}); diff --git a/src/mongo/rpc/command_reply_builder.cpp b/src/mongo/rpc/command_reply_builder.cpp index 8bc50590333..65744efa49f 100644 --- a/src/mongo/rpc/command_reply_builder.cpp +++ b/src/mongo/rpc/command_reply_builder.cpp @@ -40,10 +40,9 @@ namespace mongo { namespace rpc { -CommandReplyBuilder::CommandReplyBuilder() : CommandReplyBuilder(stdx::make_unique<Message>()) {} +CommandReplyBuilder::CommandReplyBuilder() : CommandReplyBuilder(Message{}) {} -CommandReplyBuilder::CommandReplyBuilder(std::unique_ptr<Message> message) - : _message{std::move(message)} { +CommandReplyBuilder::CommandReplyBuilder(Message&& message) : _message{std::move(message)} { _builder.skip(mongo::MsgData::MsgDataHeaderSize); } @@ -105,17 +104,17 @@ void CommandReplyBuilder::reset() { } _builder.reset(); _builder.skip(mongo::MsgData::MsgDataHeaderSize); - _message = stdx::make_unique<Message>(); + _message.reset(); _state = State::kMetadata; } -std::unique_ptr<Message> CommandReplyBuilder::done() { +Message CommandReplyBuilder::done() { invariant(_state == State::kOutputDocs); MsgData::View msg = _builder.buf(); msg.setLen(_builder.len()); msg.setOperation(dbCommandReply); - _builder.decouple(); // release ownership from BufBuilder - _message->setData(msg.view2ptr(), true); // transfer ownership to Message + _builder.decouple(); // release ownership from BufBuilder + _message.setData(msg.view2ptr(), true); // transfer ownership to Message _state = State::kDone; return std::move(_message); } diff --git a/src/mongo/rpc/command_reply_builder.h b/src/mongo/rpc/command_reply_builder.h index 92c567f64a6..b91b335fcdc 100644 --- a/src/mongo/rpc/command_reply_builder.h +++ b/src/mongo/rpc/command_reply_builder.h @@ -54,7 +54,7 @@ public: * Constructs an OP_COMMANDREPLY in an existing buffer. Ownership of the buffer * will be transfered to the CommandReplyBuilder. */ - CommandReplyBuilder(std::unique_ptr<Message> message); + CommandReplyBuilder(Message&& message); CommandReplyBuilder& setMetadata(const BSONObj& metadata) final; CommandReplyBuilder& setRawCommandReply(const BSONObj& commandReply) final; @@ -73,7 +73,7 @@ public: * The behavior of calling any methods on the object is subsequently * undefined. */ - std::unique_ptr<Message> done() final; + Message done() final; std::size_t availableBytes() const final; @@ -86,7 +86,7 @@ private: // Default values are all empty. BufBuilder _builder{}; - std::unique_ptr<Message> _message; + Message _message; State _state{State::kMetadata}; }; diff --git a/src/mongo/rpc/command_reply_test.cpp b/src/mongo/rpc/command_reply_test.cpp index 4ace31f46d5..b5d372b42be 100644 --- a/src/mongo/rpc/command_reply_test.cpp +++ b/src/mongo/rpc/command_reply_test.cpp @@ -51,12 +51,10 @@ using std::end; class ReplyTest : public mongo::unittest::Test { protected: std::vector<char> _cmdData{}; - // using unique ptr so we can destroy and replace easily - // since message does not have operator= or swap defined... - std::unique_ptr<Message> _message{}; + Message _message{}; virtual void setUp() override { - _message = stdx::make_unique<Message>(); + _message.reset(); } virtual void tearDown() override { @@ -73,8 +71,8 @@ protected: const Message* buildMessage() { _cmdData.shrink_to_fit(); - _message->setData(dbCommandReply, _cmdData.data(), _cmdData.size()); - return _message.get(); + _message.setData(dbCommandReply, _cmdData.data(), _cmdData.size()); + return &_message; } }; diff --git a/src/mongo/rpc/command_request_builder.cpp b/src/mongo/rpc/command_request_builder.cpp index 25f67c11e7f..60a013d218f 100644 --- a/src/mongo/rpc/command_request_builder.cpp +++ b/src/mongo/rpc/command_request_builder.cpp @@ -39,13 +39,11 @@ namespace mongo { namespace rpc { -CommandRequestBuilder::CommandRequestBuilder() - : CommandRequestBuilder(stdx::make_unique<Message>()) {} +CommandRequestBuilder::CommandRequestBuilder() : CommandRequestBuilder(Message()) {} CommandRequestBuilder::~CommandRequestBuilder() {} -CommandRequestBuilder::CommandRequestBuilder(std::unique_ptr<Message> message) - : _message{std::move(message)} { +CommandRequestBuilder::CommandRequestBuilder(Message&& message) : _message{std::move(message)} { _builder.skip(mongo::MsgData::MsgDataHeaderSize); // Leave room for message header. } @@ -98,13 +96,13 @@ Protocol CommandRequestBuilder::getProtocol() const { return rpc::Protocol::kOpCommandV1; } -std::unique_ptr<Message> CommandRequestBuilder::done() { +Message CommandRequestBuilder::done() { invariant(_state == State::kInputDocs); MsgData::View msg = _builder.buf(); msg.setLen(_builder.len()); msg.setOperation(dbCommand); - _builder.decouple(); // release ownership from BufBuilder. - _message->setData(msg.view2ptr(), true); // transfer ownership to Message. + _builder.decouple(); // release ownership from BufBuilder. + _message.setData(msg.view2ptr(), true); // transfer ownership to Message. _state = State::kDone; return std::move(_message); } diff --git a/src/mongo/rpc/command_request_builder.h b/src/mongo/rpc/command_request_builder.h index cf00e3f9652..50b3f69fab2 100644 --- a/src/mongo/rpc/command_request_builder.h +++ b/src/mongo/rpc/command_request_builder.h @@ -56,7 +56,7 @@ public: * Construct an OP_COMMAND in an existing buffer. Ownership of the buffer will be * transfered to the CommandRequestBuilder. */ - CommandRequestBuilder(std::unique_ptr<Message> message); + CommandRequestBuilder(Message&& message); CommandRequestBuilder& setDatabase(StringData database) final; @@ -79,11 +79,11 @@ public: * The behavior of calling any methods on the object is subsequently * undefined. */ - std::unique_ptr<Message> done() final; + Message done() final; private: BufBuilder _builder{}; - std::unique_ptr<Message> _message; + Message _message; State _state{State::kDatabase}; }; diff --git a/src/mongo/rpc/command_request_builder_test.cpp b/src/mongo/rpc/command_request_builder_test.cpp index cdfbc9d01a9..5972a41752f 100644 --- a/src/mongo/rpc/command_request_builder_test.cpp +++ b/src/mongo/rpc/command_request_builder_test.cpp @@ -78,7 +78,7 @@ TEST(RequestBuilder, RoundTrip) { .addInputDocs(inputDocRange) .done(); - rpc::CommandRequest parsed(msg.get()); + rpc::CommandRequest parsed(&msg); ASSERT_EQUALS(parsed.getDatabase(), databaseName); ASSERT_EQUALS(parsed.getCommandName(), commandName); diff --git a/src/mongo/rpc/command_request_test.cpp b/src/mongo/rpc/command_request_test.cpp index 8f681655b56..18dbc2edc66 100644 --- a/src/mongo/rpc/command_request_test.cpp +++ b/src/mongo/rpc/command_request_test.cpp @@ -115,7 +115,7 @@ TEST(CommandRequest, InvalidNSThrows) { crb.setMetadata(BSONObj()); crb.setCommandArgs(BSON("ping" << 1)); auto msg = crb.done(); - ASSERT_THROWS(rpc::CommandRequest{msg.get()}, AssertionException); + ASSERT_THROWS(rpc::CommandRequest{&msg}, AssertionException); } } // namespace diff --git a/src/mongo/rpc/legacy_reply_builder.cpp b/src/mongo/rpc/legacy_reply_builder.cpp index 592e3588b43..e4830c10f2a 100644 --- a/src/mongo/rpc/legacy_reply_builder.cpp +++ b/src/mongo/rpc/legacy_reply_builder.cpp @@ -65,9 +65,9 @@ bool isEmptyCommandReply(const BSONObj& bson) { const char LegacyReplyBuilder::kCursorTag[] = "cursor"; const char LegacyReplyBuilder::kFirstBatchTag[] = "firstBatch"; -LegacyReplyBuilder::LegacyReplyBuilder() : LegacyReplyBuilder(stdx::make_unique<Message>()) {} +LegacyReplyBuilder::LegacyReplyBuilder() : LegacyReplyBuilder(Message()) {} -LegacyReplyBuilder::LegacyReplyBuilder(std::unique_ptr<Message> message) +LegacyReplyBuilder::LegacyReplyBuilder(Message&& message) : _currentLength{kReservedSize}, _message{std::move(message)} {} LegacyReplyBuilder::~LegacyReplyBuilder() {} @@ -164,14 +164,14 @@ void LegacyReplyBuilder::reset() { if (_state == State::kMetadata) { return; } - _message = stdx::make_unique<Message>(); + _message.reset(); _currentLength = kReservedSize; _currentIndex = 0U; _state = State::kMetadata; } -std::unique_ptr<Message> LegacyReplyBuilder::done() { +Message LegacyReplyBuilder::done() { invariant(_state == State::kOutputDocs); BSONObj reply = uassertStatusOK(rpc::downconvertReplyMetadata(_commandReply, _metadata)); @@ -221,7 +221,7 @@ std::unique_ptr<Message> LegacyReplyBuilder::done() { qr.setStartingFrom(0); qr.setNReturned(1); - _message->setData(qr.view2ptr(), true); + _message.setData(qr.view2ptr(), true); bufBuilder.decouple(); _state = State::kDone; diff --git a/src/mongo/rpc/legacy_reply_builder.h b/src/mongo/rpc/legacy_reply_builder.h index 957265f0611..814fb9096e1 100644 --- a/src/mongo/rpc/legacy_reply_builder.h +++ b/src/mongo/rpc/legacy_reply_builder.h @@ -35,6 +35,7 @@ #include "mongo/rpc/document_range.h" #include "mongo/rpc/protocol.h" #include "mongo/rpc/reply_builder_interface.h" +#include "mongo/util/net/message.h" namespace mongo { namespace rpc { @@ -45,7 +46,7 @@ public: static const char kFirstBatchTag[]; LegacyReplyBuilder(); - LegacyReplyBuilder(std::unique_ptr<Message>); + LegacyReplyBuilder(Message&&); ~LegacyReplyBuilder() final; LegacyReplyBuilder& setMetadata(const BSONObj& metadata) final; @@ -58,7 +59,7 @@ public: void reset() final; - std::unique_ptr<Message> done() final; + Message done() final; Protocol getProtocol() const final; @@ -78,7 +79,7 @@ private: BSONObj _commandReply{}; std::size_t _currentLength; std::size_t _currentIndex = 0U; - std::unique_ptr<Message> _message; + Message _message; BSONObj _metadata{}; std::vector<BSONObj> _outputDocs{}; State _state{State::kMetadata}; diff --git a/src/mongo/rpc/legacy_request_builder.cpp b/src/mongo/rpc/legacy_request_builder.cpp index db333dc4c6a..33f66a4811f 100644 --- a/src/mongo/rpc/legacy_request_builder.cpp +++ b/src/mongo/rpc/legacy_request_builder.cpp @@ -42,12 +42,11 @@ namespace mongo { namespace rpc { -LegacyRequestBuilder::LegacyRequestBuilder() : LegacyRequestBuilder(stdx::make_unique<Message>()) {} +LegacyRequestBuilder::LegacyRequestBuilder() : LegacyRequestBuilder(Message()) {} LegacyRequestBuilder::~LegacyRequestBuilder() {} -LegacyRequestBuilder::LegacyRequestBuilder(std::unique_ptr<Message> message) - : _message{std::move(message)} { +LegacyRequestBuilder::LegacyRequestBuilder(Message&& message) : _message{std::move(message)} { _builder.skip(mongo::MsgData::MsgDataHeaderSize); } @@ -111,13 +110,13 @@ Protocol LegacyRequestBuilder::getProtocol() const { return rpc::Protocol::kOpQuery; } -std::unique_ptr<Message> LegacyRequestBuilder::done() { +Message LegacyRequestBuilder::done() { invariant(_state == State::kInputDocs); MsgData::View msg = _builder.buf(); msg.setLen(_builder.len()); msg.setOperation(dbQuery); - _builder.decouple(); // release ownership from BufBuilder - _message->setData(msg.view2ptr(), true); // transfer ownership to Message + _builder.decouple(); // release ownership from BufBuilder + _message.setData(msg.view2ptr(), true); // transfer ownership to Message _state = State::kDone; return std::move(_message); } diff --git a/src/mongo/rpc/legacy_request_builder.h b/src/mongo/rpc/legacy_request_builder.h index 0d44c47a513..d20011cc443 100644 --- a/src/mongo/rpc/legacy_request_builder.h +++ b/src/mongo/rpc/legacy_request_builder.h @@ -44,7 +44,7 @@ public: LegacyRequestBuilder(); ~LegacyRequestBuilder() final; - LegacyRequestBuilder(std::unique_ptr<Message>); + LegacyRequestBuilder(Message&&); LegacyRequestBuilder& setDatabase(StringData database) final; LegacyRequestBuilder& setCommandName(StringData commandName) final; @@ -58,10 +58,10 @@ public: Protocol getProtocol() const final; - std::unique_ptr<Message> done() final; + Message done() final; private: - std::unique_ptr<Message> _message; + Message _message; BufBuilder _builder{}; // we need to stash this as we need commandArgs to diff --git a/src/mongo/rpc/legacy_request_test.cpp b/src/mongo/rpc/legacy_request_test.cpp index caadc74e3e8..1413a139d71 100644 --- a/src/mongo/rpc/legacy_request_test.cpp +++ b/src/mongo/rpc/legacy_request_test.cpp @@ -46,7 +46,7 @@ TEST(LegacyRequest, InvalidNSThrows) { crb.setMetadata(BSONObj()); crb.setCommandArgs(BSON("ping" << 1)); auto msg = crb.done(); - ASSERT_THROWS(rpc::LegacyRequest{msg.get()}, AssertionException); + ASSERT_THROWS(rpc::LegacyRequest{&msg}, AssertionException); } } // namespace diff --git a/src/mongo/rpc/reply_builder_interface.h b/src/mongo/rpc/reply_builder_interface.h index c02511be567..5f3f2621746 100644 --- a/src/mongo/rpc/reply_builder_interface.h +++ b/src/mongo/rpc/reply_builder_interface.h @@ -131,7 +131,7 @@ public: * Writes data then transfers ownership of the message to the caller. The behavior of * calling any methods on the builder is subsequently undefined. */ - virtual std::unique_ptr<Message> done() = 0; + virtual Message done() = 0; protected: ReplyBuilderInterface() = default; diff --git a/src/mongo/rpc/reply_builder_test.cpp b/src/mongo/rpc/reply_builder_test.cpp index ad50b810989..9ae3544aba5 100644 --- a/src/mongo/rpc/reply_builder_test.cpp +++ b/src/mongo/rpc/reply_builder_test.cpp @@ -96,7 +96,7 @@ TEST(CommandReplyBuilder, MemAccess) { replyBuilder.setCommandReply(commandReply); auto msg = replyBuilder.done(); - rpc::CommandReply parsed(msg.get()); + rpc::CommandReply parsed(&msg); ASSERT_EQUALS(parsed.getMetadata(), metadata); ASSERT_EQUALS(parsed.getCommandReply(), commandReply); @@ -110,7 +110,7 @@ TEST(LegacyReplyBuilder, MemAccess) { replyBuilder.setCommandReply(commandReply); auto msg = replyBuilder.done(); - rpc::LegacyReply parsed(msg.get()); + rpc::LegacyReply parsed(&msg); ASSERT_EQUALS(parsed.getMetadata(), metadata); ASSERT_EQUALS(parsed.getCommandReply(), commandReply); @@ -164,7 +164,7 @@ void testRoundTrip(rpc::ReplyBuilderInterface& replyBuilder) { auto msg = replyBuilder.done(); - T parsed(msg.get()); + T parsed(&msg); ASSERT_EQUALS(parsed.getMetadata(), metadata); ASSERT_TRUE(parsed.getOutputDocs() == outputDocRange); @@ -207,10 +207,10 @@ TEST(LegacyReplyBuilderSpaceTest, DocSize) { replyBuilder0.setCommandReply(commandReply); auto msg0 = replyBuilder0.done(); - QueryResult::View qr0 = msg0->singleData().view2ptr(); + QueryResult::View qr0 = msg0.singleData().view2ptr(); auto dataLen0 = static_cast<std::size_t>(qr0.msgdata().dataLen()); - QueryResult::View qr = msg->singleData().view2ptr(); + QueryResult::View qr = msg.singleData().view2ptr(); auto dataLen = static_cast<std::size_t>(qr.msgdata().dataLen()); // below tests the array space estimates @@ -276,7 +276,7 @@ TEST_F(CommandReplyBuilderSpaceTest, MaxDocSize1) { availSpace = replyBuilder.availableBytes(); } auto msg = replyBuilder.done(); - auto sizeUInt = static_cast<std::size_t>(msg->size()); + auto sizeUInt = static_cast<std::size_t>(msg.size()); ASSERT_EQUALS(sizeUInt, mongo::MaxMessageSizeBytes); } @@ -304,7 +304,7 @@ TEST_F(CommandReplyBuilderSpaceTest, MaxDocSize2) { availSpace = replyBuilder.availableBytes(); } auto msg = replyBuilder.done(); - auto sizeUInt = static_cast<std::size_t>(msg->size()); + auto sizeUInt = static_cast<std::size_t>(msg.size()); ASSERT_EQUALS(sizeUInt, mongo::MaxMessageSizeBytes); } @@ -338,7 +338,7 @@ TEST_F(CommandReplyBuilderSpaceTest, MaxDocSize3) { auto msg = replyBuilder.done(); - auto sizeUInt = static_cast<std::size_t>(msg->size()); + auto sizeUInt = static_cast<std::size_t>(msg.size()); ASSERT_EQUALS(sizeUInt, mongo::MaxMessageSizeBytes); } diff --git a/src/mongo/rpc/request_builder_interface.h b/src/mongo/rpc/request_builder_interface.h index f27c7e3e10d..2325863dd0f 100644 --- a/src/mongo/rpc/request_builder_interface.h +++ b/src/mongo/rpc/request_builder_interface.h @@ -108,7 +108,7 @@ public: * The behavior of calling any methods on the object is subsequently * undefined. */ - virtual std::unique_ptr<Message> done() = 0; + virtual Message done() = 0; protected: RequestBuilderInterface() = default; diff --git a/src/mongo/rpc/unique_message.h b/src/mongo/rpc/unique_message.h index a763a82ec25..6b470b3f9dd 100644 --- a/src/mongo/rpc/unique_message.h +++ b/src/mongo/rpc/unique_message.h @@ -34,9 +34,9 @@ #include "mongo/base/disallow_copying.h" #include "mongo/rpc/reply_interface.h" #include "mongo/rpc/request_interface.h" +#include "mongo/util/net/message.h" namespace mongo { -class Message; namespace rpc { /** @@ -48,7 +48,7 @@ class UniqueMessage { MONGO_DISALLOW_COPYING(UniqueMessage); public: - UniqueMessage(std::unique_ptr<Message> message, std::unique_ptr<MessageViewType> view) + UniqueMessage(Message&& message, std::unique_ptr<MessageViewType> view) : _message{std::move(message)}, _view{std::move(view)} {} #if defined(_MSC_VER) && _MSC_VER < 1900 @@ -77,13 +77,13 @@ public: * Releases ownership of the underlying message. The result of calling any other methods * on the object afterward is undefined. */ - std::unique_ptr<Message> releaseMessage() { + Message releaseMessage() { _view.reset(); return std::move(_message); } private: - std::unique_ptr<Message> _message; + Message _message; std::unique_ptr<MessageViewType> _view; }; diff --git a/src/mongo/s/client/dbclient_multi_command.cpp b/src/mongo/s/client/dbclient_multi_command.cpp index b3801104f0d..41097339346 100644 --- a/src/mongo/s/client/dbclient_multi_command.cpp +++ b/src/mongo/s/client/dbclient_multi_command.cpp @@ -96,7 +96,8 @@ static void sayAsCmd(DBClientBase* conn, StringData dbName, const BSONObj& cmdOb requestBuilder->setMetadata(metadataBob.done()); requestBuilder->setCommandArgs(upconvertedCmd); // Send our command - conn->say(*requestBuilder->done()); + auto requestMsg = requestBuilder->done(); + conn->say(requestMsg); } // THROWS diff --git a/src/mongo/util/net/message.h b/src/mongo/util/net/message.h index 4518127f8ff..da5433a7f20 100644 --- a/src/mongo/util/net/message.h +++ b/src/mongo/util/net/message.h @@ -342,13 +342,15 @@ class Message { public: // we assume here that a vector with initial size 0 does no allocation (0 is the default, but // wanted to make it explicit). - Message() : _buf(0), _data(0), _freeIt(false) {} - Message(void* data, bool freeIt) : _buf(0), _data(0), _freeIt(false) { - _setData(reinterpret_cast<char*>(data), freeIt); - }; - Message(Message&& r) : _buf(0), _data(0), _freeIt(false) { - *this = std::move(r); + Message() = default; + + Message(void* data, bool freeIt) : _buf(reinterpret_cast<char*>(data)), _freeIt(freeIt) {} + + Message(Message&& r) : _buf(r._buf), _data(std::move(r._data)), _freeIt(r._freeIt) { + r._buf = nullptr; + r._freeIt = false; } + ~Message() { reset(); } @@ -420,30 +422,31 @@ public: if (&r == this) return *this; - verify(empty()); // This means that our _data must be empty() when we swap below. - verify(r._freeIt); - _buf = r._buf; - r._buf = 0; - if (r._data.size() > 0) { - _data.swap(r._data); + if (!empty()) { + reset(); } + + _buf = r._buf; + _data = std::move(r._data); + _freeIt = r._freeIt; + + r._buf = nullptr; r._freeIt = false; - _freeIt = true; return *this; } void reset() { if (_freeIt) { if (_buf) { - free(_buf); + std::free(_buf); } for (std::vector<std::pair<char*, int>>::const_iterator i = _data.begin(); i != _data.end(); ++i) { - free(i->first); + std::free(i->first); } } - _buf = 0; + _buf = nullptr; _data.clear(); _freeIt = false; } @@ -505,12 +508,12 @@ private: _buf = d; } // if just one buffer, keep it in _buf, otherwise keep a sequence of buffers in _data - char* _buf; + char* _buf{nullptr}; // byte buffer(s) - the first must contain at least a full MsgData unless using _buf for storage // instead typedef std::vector<std::pair<char*, int>> MsgVec; - MsgVec _data; - bool _freeIt; + MsgVec _data{}; + bool _freeIt{false}; }; |