Skip to content

Commit

Permalink
refactor(spork): use Mutex not CCriticalSection, use distinct mutexes…
Browse files Browse the repository at this point in the history 
… for caches (#4737)
  • Loading branch information
@PastaPastaPasta
PastaPastaPasta authored Apr 11, 2022
1 parent 2d66616 commit eec6c13
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 28 deletions.
28 changes: 17 additions & 11 deletions src/spork.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@ bool CSporkManager::SporkValueIsActive(SporkId nSporkID, int64_t& nActiveValueRe

if (!mapSporksActive.count(nSporkID)) return false;

if (auto it = mapSporksCachedValues.find(nSporkID); it != mapSporksCachedValues.end()) {
nActiveValueRet = it->second;
return true;
{
LOCK(cs_mapSporksCachedValues);
if (auto it = mapSporksCachedValues.find(nSporkID); it != mapSporksCachedValues.end()) {
nActiveValueRet = it->second;
return true;
}
}

// calc how many values we have and how many signers vote for every value
Expand All @@ -42,7 +45,7 @@ bool CSporkManager::SporkValueIsActive(SporkId nSporkID, int64_t& nActiveValueRe
// nMinSporkKeys is always more than the half of the max spork keys number,
// so there is only one such value and we can stop here
nActiveValueRet = spork.nValue;
mapSporksCachedValues[nSporkID] = nActiveValueRet;
WITH_LOCK(cs_mapSporksCachedValues, mapSporksCachedValues[nSporkID] = nActiveValueRet);
return true;
}
}
Expand Down Expand Up @@ -162,8 +165,8 @@ void CSporkManager::ProcessSpork(const CNode* pfrom, std::string_view strCommand
mapSporksByHash[hash] = spork;
mapSporksActive[spork.nSporkID][keyIDSigner] = spork;
// Clear cached values on new spork being processed
mapSporksCachedActive.erase(spork.nSporkID);
mapSporksCachedValues.erase(spork.nSporkID);
WITH_LOCK(cs_mapSporksCachedActive, mapSporksCachedActive.erase(spork.nSporkID));
WITH_LOCK(cs_mapSporksCachedValues, mapSporksCachedValues.erase(spork.nSporkID));
}
spork.Relay(connman);
}
Expand Down Expand Up @@ -204,8 +207,8 @@ bool CSporkManager::UpdateSpork(SporkId nSporkID, int64_t nValue, CConnman& conn
mapSporksByHash[spork.GetHash()] = spork;
mapSporksActive[nSporkID][keyIDSigner] = spork;
// Clear cached values on new spork being processed
mapSporksCachedActive.erase(spork.nSporkID);
mapSporksCachedValues.erase(spork.nSporkID);
WITH_LOCK(cs_mapSporksCachedActive, mapSporksCachedActive.erase(spork.nSporkID));
WITH_LOCK(cs_mapSporksCachedValues, mapSporksCachedValues.erase(spork.nSporkID));
}

spork.Relay(connman);
Expand All @@ -214,17 +217,20 @@ bool CSporkManager::UpdateSpork(SporkId nSporkID, int64_t nValue, CConnman& conn

bool CSporkManager::IsSporkActive(SporkId nSporkID) const
{
LOCK(cs);
// If nSporkID is cached, and the cached value is true, then return early true
if (auto it = mapSporksCachedActive.find(nSporkID); it != mapSporksCachedActive.end() && it->second) {
return true;
{
LOCK(cs_mapSporksCachedActive);
if (auto it = mapSporksCachedActive.find(nSporkID); it != mapSporksCachedActive.end() && it->second) {
return true;
}
}

int64_t nSporkValue = GetSporkValue(nSporkID);
// Get time is somewhat costly it looks like
bool ret = nSporkValue < GetAdjustedTime();
// Only cache true values
if (ret) {
LOCK(cs_mapSporksCachedActive);
mapSporksCachedActive[nSporkID] = ret;
}
return ret;
Expand Down
36 changes: 19 additions & 17 deletions src/spork.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,14 @@ class CSporkManager
private:
static constexpr std::string_view SERIALIZATION_VERSION_STRING = "CSporkManager-Version-2";

mutable CCriticalSection cs;
mutable Mutex cs_mapSporksCachedActive;
mutable std::unordered_map<const SporkId, bool> mapSporksCachedActive GUARDED_BY(cs_mapSporksCachedActive);

mutable std::unordered_map<const SporkId, bool> mapSporksCachedActive GUARDED_BY(cs);
mutable Mutex cs_mapSporksCachedValues;
mutable std::unordered_map<SporkId, int64_t> mapSporksCachedValues GUARDED_BY(cs_mapSporksCachedValues);

mutable Mutex cs;

mutable std::unordered_map<SporkId, int64_t> mapSporksCachedValues GUARDED_BY(cs);
std::unordered_map<uint256, CSporkMessage, StaticSaltedHasher> mapSporksByHash GUARDED_BY(cs);
std::unordered_map<SporkId, std::map<CKeyID, CSporkMessage> > mapSporksActive GUARDED_BY(cs);

Expand All @@ -185,7 +188,7 @@ class CSporkManager
CSporkManager() = default;

template<typename Stream>
void Serialize(Stream &s) const
void Serialize(Stream &s) const LOCKS_EXCLUDED(cs)
{
// We don't serialize pubkey ids because pubkeys should be
// hardcoded or be set with cmdline or options, should
Expand All @@ -196,7 +199,7 @@ class CSporkManager
}

template<typename Stream>
void Unserialize(Stream &s)
void Unserialize(Stream &s) LOCKS_EXCLUDED(cs)
{
LOCK(cs);
std::string strVersion;
Expand All @@ -213,7 +216,7 @@ class CSporkManager
*
* This method was introduced along with the spork cache.
*/
void Clear();
void Clear() LOCKS_EXCLUDED(cs);

/**
* CheckAndRemove is defined to fulfill an interface as part of the on-disk
Expand All @@ -223,7 +226,7 @@ class CSporkManager
*
* This method was introduced along with the spork cache.
*/
void CheckAndRemove();
void CheckAndRemove() LOCKS_EXCLUDED(cs);

/**
* ProcessSporkMessages is used to call ProcessSpork and ProcessGetSporks. See below
Expand All @@ -236,26 +239,25 @@ class CSporkManager
* For 'spork', it validates the spork and adds it to the internal spork storage and
* performs any necessary processing.
*/
void ProcessSpork(const CNode* pfrom, std::string_view strCommand, CDataStream& vRecv, CConnman& connman);
void ProcessSpork(const CNode* pfrom, std::string_view strCommand, CDataStream& vRecv, CConnman& connman) LOCKS_EXCLUDED(cs);


/**
* ProcessGetSporks is used to handle the 'getsporks' p2p message.
*
* For 'getsporks', it sends active sporks to the requesting peer.
*/
void ProcessGetSporks(CNode* pfrom, std::string_view strCommand, CConnman& connman);
void ProcessGetSporks(CNode* pfrom, std::string_view strCommand, CConnman& connman) LOCKS_EXCLUDED(cs);

/**
* UpdateSpork is used by the spork RPC command to set a new spork value, sign
* and broadcast the spork message.
*/
bool UpdateSpork(SporkId nSporkID, int64_t nValue, CConnman& connman);
bool UpdateSpork(SporkId nSporkID, int64_t nValue, CConnman& connman) LOCKS_EXCLUDED(cs);

/**
* IsSporkActive returns a bool for time-based sporks, and should be used
* to determine whether the spork can be considered active or not.
*
* For value-based sporks such as SPORK_5_INSTANTSEND_MAX_VALUE, the spork
* value should not be considered a timestamp, but an integer value
* instead, and therefore this method doesn't make sense and should not be
Expand All @@ -267,7 +269,7 @@ class CSporkManager
* GetSporkValue returns the spork value given a Spork ID. If no active spork
* message has yet been received by the node, it returns the default value.
*/
int64_t GetSporkValue(SporkId nSporkID) const;
int64_t GetSporkValue(SporkId nSporkID) const LOCKS_EXCLUDED(cs);

/**
* GetSporkIDByName returns the internal Spork ID given the spork name.
Expand All @@ -282,7 +284,7 @@ class CSporkManager
* hash-based index of sporks for this reason, and this function is the access
* point into that index.
*/
bool GetSporkByHash(const uint256& hash, CSporkMessage &sporkRet) const;
bool GetSporkByHash(const uint256& hash, CSporkMessage &sporkRet) const LOCKS_EXCLUDED(cs);

/**
* SetSporkAddress is used to set a public key ID which will be used to
Expand All @@ -291,7 +293,7 @@ class CSporkManager
* This can be called multiple times to add multiple keys to the set of
* valid spork signers.
*/
bool SetSporkAddress(const std::string& strAddress);
bool SetSporkAddress(const std::string& strAddress) LOCKS_EXCLUDED(cs);

/**
* SetMinSporkKeys is used to set the required spork signer threshold, for
Expand All @@ -300,7 +302,7 @@ class CSporkManager
* This value must be at least a majority of the total number of spork
* keys, and for obvious reasons cannot be larger than that number.
*/
bool SetMinSporkKeys(int minSporkKeys);
bool SetMinSporkKeys(int minSporkKeys) LOCKS_EXCLUDED(cs);

/**
* SetPrivKey is used to set a spork key to enable setting / signing of
Expand All @@ -309,12 +311,12 @@ class CSporkManager
* This will return false if the private key does not match any spork
* address in the set of valid spork signers (see SetSporkAddress).
*/
bool SetPrivKey(const std::string& strPrivKey);
bool SetPrivKey(const std::string& strPrivKey) LOCKS_EXCLUDED(cs);

/**
* ToString returns the string representation of the SporkManager.
*/
std::string ToString() const;
std::string ToString() const LOCKS_EXCLUDED(cs);
};

#endif // BITCOIN_SPORK_H

0 comments on commit eec6c13

Please sign in to comment.