Skip to content

Fix Several Sources of Frequent Lock Contention #1688

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cassandras-lies wants to merge 2 commits into
base: master
Choose a base branch
from
+30 −16
Open

Fix Several Sources of Frequent Lock Contention #1688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
158d43a
Stop copying vNodes dozens of times per second in CConnman::ThreadSoc…
cassandras-lies Oct 23, 2025
fbcf197
Make CSigSharesManager::WorkThreadMain run only every 1s rather than …
cassandras-lies Oct 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/llmq/quorums_signing_shares.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1338,6 +1338,7 @@ void CSigSharesManager::BanNode(NodeId nodeId)
void CSigSharesManager::WorkThreadMain()
{
int64_t lastSendTime = 0;
int64_t lastRemoveBannedNodeStatesTime = 0;

while (!workInterrupt) {
if (!quorumSigningManager || !g_connman) {
Expand All @@ -1349,7 +1350,13 @@ void CSigSharesManager::WorkThreadMain()

bool didWork = false;

RemoveBannedNodeStates();
// RemoveBannedNodeStates holds cs_main, and its functionality is only required for local memory management. It
// is much more performant to call this rarely.
if (GetTimeMillis() - lastRemoveBannedNodeStatesTime > 30000 /* 30s */) {
RemoveBannedNodeStates();
lastRemoveBannedNodeStatesTime = GetTimeMillis();
}

didWork |= quorumSigningManager->ProcessPendingRecoveredSigs(*g_connman);
didWork |= ProcessPendingSigShares(*g_connman);
didWork |= SignPendingSigShares();
Expand All @@ -1364,7 +1371,7 @@ void CSigSharesManager::WorkThreadMain()

// TODO Wakeup when pending signing is needed?
if (!didWork) {
if (!workInterrupt.sleep_for(std::chrono::milliseconds(100))) {
if (!workInterrupt.sleep_for(std::chrono::milliseconds(1000))) {
Copy link
Contributor

@aleflm aleflm Oct 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, PR looks correct to me. But I suspect that 1000ms is too high for this sleep_for. Maybe something like 250ms is more reasonable. Just a suggestion. Since this is not going to make a huge difference.

Overall, I don't see any specific issues with the PR.

return;
}
}
Expand Down
35 changes: 21 additions & 14 deletions src/net.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1358,27 +1358,34 @@ void CConnman::ThreadSocketHandler()
// Disconnect nodes
//
{
LOCK(cs_vNodes);
// Disconnect unused nodes
std::vector<CNode*> vNodesCopy = vNodes;
BOOST_FOREACH(CNode* pnode, vNodesCopy)
LOCK(cs_vNodes);

// Only copy nodes which are actually disconnected so as to minimise lock time.
std::vector<CNode*> vDisconnectedNodes;
BOOST_FOREACH(CNode* pnode, vNodes)
{
if (pnode->fDisconnect)
{
// remove from vNodes
vNodes.erase(remove(vNodes.begin(), vNodes.end(), pnode), vNodes.end());
vDisconnectedNodes.push_back(pnode);
}
}

// release outbound grant (if any)
pnode->grantOutbound.Release();
pnode->grantMasternodeOutbound.Release();
BOOST_FOREACH(CNode* pnode, vDisconnectedNodes)
{
// remove from vNodes
vNodes.erase(remove(vNodes.begin(), vNodes.end(), pnode), vNodes.end());

// close socket and cleanup
pnode->CloseSocketDisconnect();
// release outbound grant (if any)
pnode->grantOutbound.Release();
pnode->grantMasternodeOutbound.Release();

// hold in disconnected pool until all refs are released
pnode->Release();
vNodesDisconnected.push_back(pnode);
}
// close socket and cleanup
pnode->CloseSocketDisconnect();

// hold in disconnected pool until all refs are released
pnode->Release();
vNodesDisconnected.push_back(pnode);
}
}
{
Expand Down