From a61ffab3f9010d8accfaa98aa3cacc7d38e74121 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Wed, 18 Mar 2015 12:29:40 -0400 Subject: [PATCH] Remove unnecessary allocation/deallocation from masterLock * Add make_lock. * Rename Application::LockType to Application::MutexType: * Rename getMasterLock to getMasterMutex. * Use getMasterMutex and make_lock. * Remove unused code. --- src/beast/beast/utility/make_lock.h | 38 ++++++++++++++++++++ src/ripple/app/consensus/LedgerConsensus.cpp | 13 ++++--- src/ripple/app/main/Application.cpp | 4 +-- src/ripple/app/main/Application.h | 14 ++------ src/ripple/app/misc/NetworkOPs.cpp | 7 ++-- src/ripple/app/tx/TransactionAcquire.cpp | 5 +-- src/ripple/overlay/impl/PeerImp.cpp | 6 ++-- src/ripple/rpc/handlers/Connect.cpp | 3 +- src/ripple/rpc/handlers/ConsensusInfo.cpp | 3 +- src/ripple/rpc/handlers/LedgerAccept.cpp | 3 +- src/ripple/rpc/handlers/Peers.cpp | 3 +- src/ripple/rpc/handlers/Stop.cpp | 3 +- src/ripple/rpc/handlers/UnlAdd.cpp | 3 +- src/ripple/rpc/handlers/UnlDelete.cpp | 3 +- src/ripple/rpc/handlers/UnlList.cpp | 3 +- src/ripple/rpc/handlers/UnlLoad.cpp | 3 +- src/ripple/rpc/handlers/UnlNetwork.cpp | 3 +- src/ripple/rpc/handlers/UnlReset.cpp | 3 +- src/ripple/rpc/handlers/UnlScore.cpp | 3 +- src/ripple/rpc/handlers/ValidationSeed.cpp | 3 +- 20 files changed, 85 insertions(+), 41 deletions(-) create mode 100644 src/beast/beast/utility/make_lock.h diff --git a/src/beast/beast/utility/make_lock.h b/src/beast/beast/utility/make_lock.h new file mode 100644 index 0000000000..76f9d7ff1d --- /dev/null +++ b/src/beast/beast/utility/make_lock.h @@ -0,0 +1,38 @@ +//------------------------------------------------------------------------------ +/* + This file is part of Beast: https://github.com/vinniefalco/Beast + Copyright 2015, Howard Hinnant + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef BEAST_UTILITY_MAKE_LOCK_H_INCLUDED +#define BEAST_UTILITY_MAKE_LOCK_H_INCLUDED + +#include +#include + +namespace beast { + +template +inline +std::unique_lock +make_lock(Mutex& mutex, Args&&... args) +{ + return std::unique_lock(mutex, std::forward(args)...); +} + +} // beast + +#endif diff --git a/src/ripple/app/consensus/LedgerConsensus.cpp b/src/ripple/app/consensus/LedgerConsensus.cpp index df0cba9daa..c0c07b5bf2 100644 --- a/src/ripple/app/consensus/LedgerConsensus.cpp +++ b/src/ripple/app/consensus/LedgerConsensus.cpp @@ -18,7 +18,6 @@ //============================================================================== #include -#include #include #include #include @@ -42,6 +41,8 @@ #include #include #include +#include +#include namespace ripple { @@ -976,8 +977,7 @@ private: { { - Application::ScopedLockType lock - (getApp ().getMasterLock ()); + std::lock_guard lock(getApp().getMasterMutex()); // put our set where others can get it later if (set->getHash ().isNonZero ()) @@ -1137,11 +1137,10 @@ private: } { - Application::ScopedLockType lock - (getApp ().getMasterLock ()); - + auto lock = beast::make_lock(getApp().getMasterMutex(), std::defer_lock); LedgerMaster::ScopedLockType sl - (getApp().getLedgerMaster ().peekMutex ()); + (getApp().getLedgerMaster ().peekMutex (), std::defer_lock); + std::lock(lock, sl); // Apply transactions from the old open ledger Ledger::pointer oldOL = getApp().getLedgerMaster().getCurrentLedger(); diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 77d363c1d6..617322d726 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -248,7 +248,7 @@ private: public: Logs& m_logs; beast::Journal m_journal; - Application::LockType m_masterMutex; + Application::MutexType m_masterMutex; // Required by the SHAMapStore TransactionMaster m_txMaster; @@ -524,7 +524,7 @@ public: return *m_nodeStore; } - Application::LockType& getMasterLock () + Application::MutexType& getMasterMutex () { return m_masterMutex; } diff --git a/src/ripple/app/main/Application.h b/src/ripple/app/main/Application.h index c395f742fc..0e868d7892 100644 --- a/src/ripple/app/main/Application.h +++ b/src/ripple/app/main/Application.h @@ -70,7 +70,7 @@ class Application : public beast::PropertyStream::Source public: /* VFALCO NOTE - The master lock protects: + The master mutex protects: - The open ledger - Server global state @@ -79,16 +79,8 @@ public: other things */ - typedef std::recursive_mutex LockType; - typedef std::unique_lock ScopedLockType; - typedef std::unique_ptr ScopedLock; - - virtual LockType& getMasterLock () = 0; - - ScopedLock masterLock() - { - return std::make_unique (getMasterLock()); - } + using MutexType = std::recursive_mutex; + virtual MutexType& getMasterMutex () = 0; public: Application (); diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index db73460d77..636d5cc693 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -60,6 +60,7 @@ #include #include #include // +#include #include #include @@ -652,7 +653,7 @@ void NetworkOPsImp::onDeadlineTimer (beast::DeadlineTimer& timer) void NetworkOPsImp::processHeartbeatTimer () { { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); // VFALCO NOTE This is for diagnosing a crash on exit Application& app (getApp ()); @@ -1004,7 +1005,7 @@ Transaction::pointer NetworkOPsImp::processTransactionCb ( } { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); bool didApply; TER r = m_ledgerMaster.doTransaction ( @@ -1600,7 +1601,7 @@ void NetworkOPsImp::processTrustedProposal ( uint256 checkLedger, bool sigGood) { { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); bool relay = true; diff --git a/src/ripple/app/tx/TransactionAcquire.cpp b/src/ripple/app/tx/TransactionAcquire.cpp index 1866a4019d..d97f1ddaad 100644 --- a/src/ripple/app/tx/TransactionAcquire.cpp +++ b/src/ripple/app/tx/TransactionAcquire.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include namespace ripple { @@ -52,7 +53,7 @@ TransactionAcquire::~TransactionAcquire () static void TACompletionHandler (uint256 hash, std::shared_ptr map) { { - Application::ScopedLockType lock (getApp ().getMasterLock ()); + std::lock_guard lock(getApp().getMasterMutex()); getApp().getOPs ().mapComplete (hash, map); @@ -88,7 +89,7 @@ void TransactionAcquire::onTimer (bool progress, ScopedLockType& psl) WriteLog (lsWARNING, TransactionAcquire) << "Ten timeouts on TX set " << getHash (); psl.unlock(); { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); if (getApp().getOPs ().stillNeedTXSet (mHash)) { diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 6fa5c0ba4a..ff63da023a 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -1478,7 +1478,7 @@ PeerImp::checkPropose (Job& job, uint256 consensusLCL; if (! set.has_previousledger() || ! isTrusted) { - Application::ScopedLockType lock (getApp ().getMasterLock ()); + std::lock_guard lock(getApp().getMasterMutex()); consensusLCL = getApp().getOPs ().getConsensusLCL (); } @@ -1614,7 +1614,7 @@ PeerImp::getLedger (std::shared_ptr const& m) memcpy (txHash.begin (), packet.ledgerhash ().data (), 32); { - Application::ScopedLockType lock (getApp ().getMasterLock ()); + std::lock_guard lock(getApp().getMasterMutex()); map = getApp().getOPs ().getTXMap (txHash); } @@ -1976,7 +1976,7 @@ PeerImp::peerTXData (Job&, uint256 const& hash, SHAMapAddNode san; { - Application::ScopedLockType lock (getApp ().getMasterLock ()); + std::lock_guard lock(getApp().getMasterMutex()); san = getApp().getOPs().gotTXData (shared_from_this(), hash, nodeIDs, nodeData); diff --git a/src/ripple/rpc/handlers/Connect.cpp b/src/ripple/rpc/handlers/Connect.cpp index eab545f7d6..2009b1a869 100644 --- a/src/ripple/rpc/handlers/Connect.cpp +++ b/src/ripple/rpc/handlers/Connect.cpp @@ -19,6 +19,7 @@ #include #include +#include namespace ripple { @@ -29,7 +30,7 @@ namespace ripple { // XXX Might allow domain for manual connections. Json::Value doConnect (RPC::Context& context) { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); if (getConfig ().RUN_STANDALONE) return "cannot connect in standalone mode"; diff --git a/src/ripple/rpc/handlers/ConsensusInfo.cpp b/src/ripple/rpc/handlers/ConsensusInfo.cpp index 643aeea6b7..d5902ccea8 100644 --- a/src/ripple/rpc/handlers/ConsensusInfo.cpp +++ b/src/ripple/rpc/handlers/ConsensusInfo.cpp @@ -18,6 +18,7 @@ //============================================================================== #include +#include namespace ripple { @@ -26,7 +27,7 @@ Json::Value doConsensusInfo (RPC::Context& context) Json::Value ret (Json::objectValue); { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); ret[jss::info] = context.netOps.getConsensusInfo (); } diff --git a/src/ripple/rpc/handlers/LedgerAccept.cpp b/src/ripple/rpc/handlers/LedgerAccept.cpp index 6928d96434..a9a185b781 100644 --- a/src/ripple/rpc/handlers/LedgerAccept.cpp +++ b/src/ripple/rpc/handlers/LedgerAccept.cpp @@ -18,12 +18,13 @@ //============================================================================== #include +#include namespace ripple { Json::Value doLedgerAccept (RPC::Context& context) { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); Json::Value jvResult; if (!getConfig ().RUN_STANDALONE) diff --git a/src/ripple/rpc/handlers/Peers.cpp b/src/ripple/rpc/handlers/Peers.cpp index 7865d469a9..1c14d17c2b 100644 --- a/src/ripple/rpc/handlers/Peers.cpp +++ b/src/ripple/rpc/handlers/Peers.cpp @@ -20,6 +20,7 @@ #include #include #include +#include namespace ripple { @@ -29,7 +30,7 @@ Json::Value doPeers (RPC::Context& context) Json::Value jvResult (Json::objectValue); { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); jvResult[jss::peers] = getApp().overlay ().json (); getApp().getUNL().addClusterStatus(jvResult); diff --git a/src/ripple/rpc/handlers/Stop.cpp b/src/ripple/rpc/handlers/Stop.cpp index fa69974625..303250f04b 100644 --- a/src/ripple/rpc/handlers/Stop.cpp +++ b/src/ripple/rpc/handlers/Stop.cpp @@ -18,12 +18,13 @@ //============================================================================== #include +#include namespace ripple { Json::Value doStop (RPC::Context& context) { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); getApp().signalStop (); return RPC::makeObjectValue (systemName () + " server stopping"); diff --git a/src/ripple/rpc/handlers/UnlAdd.cpp b/src/ripple/rpc/handlers/UnlAdd.cpp index 4dedec68b6..9a882eb8f5 100644 --- a/src/ripple/rpc/handlers/UnlAdd.cpp +++ b/src/ripple/rpc/handlers/UnlAdd.cpp @@ -19,6 +19,7 @@ #include #include +#include namespace ripple { @@ -28,7 +29,7 @@ namespace ripple { // } Json::Value doUnlAdd (RPC::Context& context) { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); std::string strNode = context.params.isMember (jss::node) ? context.params[jss::node].asString () : ""; diff --git a/src/ripple/rpc/handlers/UnlDelete.cpp b/src/ripple/rpc/handlers/UnlDelete.cpp index 3e34253221..871b36f9c2 100644 --- a/src/ripple/rpc/handlers/UnlDelete.cpp +++ b/src/ripple/rpc/handlers/UnlDelete.cpp @@ -18,6 +18,7 @@ //============================================================================== #include +#include namespace ripple { @@ -26,7 +27,7 @@ namespace ripple { // } Json::Value doUnlDelete (RPC::Context& context) { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); if (!context.params.isMember (jss::node)) return rpcError (rpcINVALID_PARAMS); diff --git a/src/ripple/rpc/handlers/UnlList.cpp b/src/ripple/rpc/handlers/UnlList.cpp index cda890419b..3a28077106 100644 --- a/src/ripple/rpc/handlers/UnlList.cpp +++ b/src/ripple/rpc/handlers/UnlList.cpp @@ -18,12 +18,13 @@ //============================================================================== #include +#include namespace ripple { Json::Value doUnlList (RPC::Context& context) { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); Json::Value obj (Json::objectValue); obj[jss::unl] = getApp().getUNL ().getUnlJson (); diff --git a/src/ripple/rpc/handlers/UnlLoad.cpp b/src/ripple/rpc/handlers/UnlLoad.cpp index d230ccf5b9..592b1ec481 100644 --- a/src/ripple/rpc/handlers/UnlLoad.cpp +++ b/src/ripple/rpc/handlers/UnlLoad.cpp @@ -18,13 +18,14 @@ //============================================================================== #include +#include namespace ripple { // Populate the UNL from a local validators.txt file. Json::Value doUnlLoad (RPC::Context& context) { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); if (getConfig ().VALIDATORS_FILE.empty () || !getApp().getUNL ().nodeLoad (getConfig ().VALIDATORS_FILE)) diff --git a/src/ripple/rpc/handlers/UnlNetwork.cpp b/src/ripple/rpc/handlers/UnlNetwork.cpp index 511ac0e679..4272d9b2b6 100644 --- a/src/ripple/rpc/handlers/UnlNetwork.cpp +++ b/src/ripple/rpc/handlers/UnlNetwork.cpp @@ -18,13 +18,14 @@ //============================================================================== #include +#include namespace ripple { // Populate the UNL from ripple.com's validators.txt file. Json::Value doUnlNetwork (RPC::Context& context) { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); getApp().getUNL ().nodeNetwork (); return RPC::makeObjectValue ("fetching"); diff --git a/src/ripple/rpc/handlers/UnlReset.cpp b/src/ripple/rpc/handlers/UnlReset.cpp index 890e7784d5..49e445a566 100644 --- a/src/ripple/rpc/handlers/UnlReset.cpp +++ b/src/ripple/rpc/handlers/UnlReset.cpp @@ -18,12 +18,13 @@ //============================================================================== #include +#include namespace ripple { Json::Value doUnlReset (RPC::Context& context) { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); getApp().getUNL ().nodeReset (); return RPC::makeObjectValue ("removing nodes"); diff --git a/src/ripple/rpc/handlers/UnlScore.cpp b/src/ripple/rpc/handlers/UnlScore.cpp index 2da4a7e6b7..12584a217f 100644 --- a/src/ripple/rpc/handlers/UnlScore.cpp +++ b/src/ripple/rpc/handlers/UnlScore.cpp @@ -18,13 +18,14 @@ //============================================================================== #include +#include namespace ripple { // unl_score Json::Value doUnlScore (RPC::Context& context) { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); getApp().getUNL ().nodeScore (); return RPC::makeObjectValue ("scoring requested"); diff --git a/src/ripple/rpc/handlers/ValidationSeed.cpp b/src/ripple/rpc/handlers/ValidationSeed.cpp index 6b0d8f045a..947ae47549 100644 --- a/src/ripple/rpc/handlers/ValidationSeed.cpp +++ b/src/ripple/rpc/handlers/ValidationSeed.cpp @@ -18,6 +18,7 @@ //============================================================================== #include +#include namespace ripple { @@ -26,7 +27,7 @@ namespace ripple { // } Json::Value doValidationSeed (RPC::Context& context) { - auto lock = getApp().masterLock(); + auto lock = beast::make_lock(getApp().getMasterMutex()); Json::Value obj (Json::objectValue); if (!context.params.isMember (jss::secret))