Skip to content
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.

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

Make all of net_processing (and some of net) use std::chrono types #21015

Merged
merged 4 commits into from
Mar 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 17 additions & 17 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,8 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
stats.minFeeFilter = 0;
}

stats.m_ping_usec = m_last_ping_time;
stats.m_min_ping_usec = m_min_ping_time;
X(m_last_ping_time);
X(m_min_ping_time);

// Leave string empty if addrLocal invalid (not filled in yet)
CService addrLocalUnlocked = GetAddrLocal();
Expand Down Expand Up @@ -1761,12 +1761,11 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
}

// Initiate network connections
auto start = GetTime<std::chrono::seconds>();
auto start = GetTime<std::chrono::microseconds>();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think GetTime should actually return a std::chrono::time_point instead of a duration. But that's probably out of scope for this PR. Then we'd have a few more compile time checks concerning the allowed operations (e.g. you can't add two timepoints, you can't accidentally compare a timepoints with a duration and you can't accidentally mix timepoints from different clocks)

Copy link
Contributor

Choose a reason for hiding this comment

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

I've had a look at this a while ago: https://github.com/ajtowns/bitcoin/commits/201908-systime It's particularly nice that your types can then encode what sort of time you've got -- if it's a high precision clock, if it's a stable clock, or, for us, if it's a mockable clock. One drawback is that time_point's aren't compatible with atomic, so you have to hack around that. I think we'd want to think some more about exactly what sorts of times we care about before doing that conversion seriously.


// Minimum time before next feeler connection (in microseconds).

int64_t nNextFeeler = PoissonNextSend(count_microseconds(start), FEELER_INTERVAL);
int64_t nNextExtraBlockRelay = PoissonNextSend(count_microseconds(start), EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL);
auto next_feeler = PoissonNextSend(start, FEELER_INTERVAL);
auto next_extra_block_relay = PoissonNextSend(start, EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL);
const bool dnsseed = gArgs.GetBoolArg("-dnsseed", DEFAULT_DNSSEED);
bool add_fixed_seeds = gArgs.GetBoolArg("-fixedseeds", DEFAULT_FIXEDSEEDS);

Expand Down Expand Up @@ -1849,7 +1848,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
}

ConnectionType conn_type = ConnectionType::OUTBOUND_FULL_RELAY;
int64_t nTime = GetTimeMicros();
auto now = GetTime<std::chrono::microseconds>();
bool anchor = false;
bool fFeeler = false;

Expand All @@ -1861,7 +1860,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
// GetTryNewOutboundPeer() gets set when a stale tip is detected, so we
// try opening an additional OUTBOUND_FULL_RELAY connection. If none of
// these conditions are met, check to see if it's time to try an extra
// block-relay-only peer (to confirm our tip is current, see below) or the nNextFeeler
// block-relay-only peer (to confirm our tip is current, see below) or the next_feeler
// timer to decide if we should open a FEELER.

if (!m_anchors.empty() && (nOutboundBlockRelay < m_max_outbound_block_relay)) {
Expand All @@ -1873,7 +1872,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
conn_type = ConnectionType::BLOCK_RELAY;
} else if (GetTryNewOutboundPeer()) {
// OUTBOUND_FULL_RELAY
} else if (nTime > nNextExtraBlockRelay && m_start_extra_block_relay_peers) {
} else if (now > next_extra_block_relay && m_start_extra_block_relay_peers) {
// Periodically connect to a peer (using regular outbound selection
// methodology from addrman) and stay connected long enough to sync
// headers, but not much else.
Expand All @@ -1895,10 +1894,10 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
// Because we can promote these connections to block-relay-only
// connections, they do not get their own ConnectionType enum
// (similar to how we deal with extra outbound peers).
nNextExtraBlockRelay = PoissonNextSend(nTime, EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL);
next_extra_block_relay = PoissonNextSend(now, EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL);
conn_type = ConnectionType::BLOCK_RELAY;
} else if (nTime > nNextFeeler) {
nNextFeeler = PoissonNextSend(nTime, FEELER_INTERVAL);
} else if (now > next_feeler) {
next_feeler = PoissonNextSend(now, FEELER_INTERVAL);
conn_type = ConnectionType::FEELER;
fFeeler = true;
} else {
Expand Down Expand Up @@ -2983,20 +2982,21 @@ bool CConnman::ForNode(NodeId id, std::function<bool(CNode* pnode)> func)
return found != nullptr && NodeFullyConnected(found) && func(found);
}

int64_t CConnman::PoissonNextSendInbound(int64_t now, int average_interval_seconds)
std::chrono::microseconds CConnman::PoissonNextSendInbound(std::chrono::microseconds now, std::chrono::seconds average_interval)
{
if (m_next_send_inv_to_incoming < now) {
if (m_next_send_inv_to_incoming.load() < now) {
// If this function were called from multiple threads simultaneously
// it would possible that both update the next send variable, and return a different result to their caller.
// This is not possible in practice as only the net processing thread invokes this function.
m_next_send_inv_to_incoming = PoissonNextSend(now, average_interval_seconds);
m_next_send_inv_to_incoming = PoissonNextSend(now, average_interval);
}
return m_next_send_inv_to_incoming;
}

int64_t PoissonNextSend(int64_t now, int average_interval_seconds)
std::chrono::microseconds PoissonNextSend(std::chrono::microseconds now, std::chrono::seconds average_interval)
{
return now + (int64_t)(log1p(GetRand(1ULL << 48) * -0.0000000000000035527136788 /* -1/2^48 */) * average_interval_seconds * -1000000.0 + 0.5);
double unscaled = -log1p(GetRand(1ULL << 48) * -0.0000000000000035527136788 /* -1/2^48 */);
dhruv marked this conversation as resolved.
Show resolved Hide resolved
return now + std::chrono::duration_cast<std::chrono::microseconds>(unscaled * average_interval + 0.5us);
}

CSipHasher CConnman::GetDeterministicRandomizer(uint64_t id) const
Expand Down
34 changes: 14 additions & 20 deletions src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ static const bool DEFAULT_WHITELISTFORCERELAY = false;

/** Time after which to disconnect, after waiting for a ping response (or inactivity). */
static const int TIMEOUT_INTERVAL = 20 * 60;
/** Run the feeler connection loop once every 2 minutes or 120 seconds. **/
static const int FEELER_INTERVAL = 120;
/** Run the feeler connection loop once every 2 minutes. **/
static constexpr auto FEELER_INTERVAL = 2min;
/** Run the extra block-relay-only connection loop once every 5 minutes. **/
static const int EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL = 300;
static constexpr auto EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL = 5min;
/** The maximum number of addresses from our addrman to return in response to a getaddr message. */
static constexpr size_t MAX_ADDR_TO_SEND = 1000;
/** Maximum length of incoming protocol messages (no message over 4 MB is currently acceptable). */
Expand Down Expand Up @@ -261,8 +261,8 @@ class CNodeStats
uint64_t nRecvBytes;
mapMsgCmdSize mapRecvBytesPerMsgCmd;
NetPermissionFlags m_permissionFlags;
int64_t m_ping_usec;
int64_t m_min_ping_usec;
std::chrono::microseconds m_last_ping_time;
std::chrono::microseconds m_min_ping_time;
CAmount minFeeFilter;
// Our address, as reported by the peer
std::string addrLocal;
Expand Down Expand Up @@ -573,7 +573,7 @@ class CNode
/** Minimum fee rate with which to filter inv's to this node */
std::atomic<CAmount> minFeeFilter{0};
CAmount lastSentFeeFilter{0};
int64_t nextSendTimeFeeFilter{0};
std::chrono::microseconds m_next_send_feefilter{0};
};

// m_tx_relay == nullptr if we're not relaying transactions with this peer
Expand All @@ -593,11 +593,11 @@ class CNode
std::atomic<int64_t> nLastTXTime{0};

/** Last measured round-trip time. Used only for RPC/GUI stats/debugging.*/
std::atomic<int64_t> m_last_ping_time{0};
std::atomic<std::chrono::microseconds> m_last_ping_time{0us};

/** Lowest measured round-trip time. Used as an inbound peer eviction
* criterium in CConnman::AttemptToEvictConnection. */
std::atomic<int64_t> m_min_ping_time{std::numeric_limits<int64_t>::max()};
std::atomic<std::chrono::microseconds> m_min_ping_time{std::chrono::microseconds::max()};

CNode(NodeId id, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion);
~CNode();
Expand Down Expand Up @@ -719,8 +719,8 @@ class CNode

/** A ping-pong round trip has completed successfully. Update latest and minimum ping times. */
void PongReceived(std::chrono::microseconds ping_time) {
m_last_ping_time = count_microseconds(ping_time);
m_min_ping_time = std::min(m_min_ping_time.load(), count_microseconds(ping_time));
m_last_ping_time = ping_time;
m_min_ping_time = std::min(m_min_ping_time.load(), ping_time);
}

private:
Expand Down Expand Up @@ -1021,7 +1021,7 @@ class CConnman
Works assuming that a single interval is used.
Variable intervals will result in privacy decrease.
*/
int64_t PoissonNextSendInbound(int64_t now, int average_interval_seconds);
std::chrono::microseconds PoissonNextSendInbound(std::chrono::microseconds now, std::chrono::seconds average_interval);

void SetAsmap(std::vector<bool> asmap) { addrman.m_asmap = std::move(asmap); }

Expand Down Expand Up @@ -1256,7 +1256,7 @@ class CConnman
*/
std::atomic_bool m_start_extra_block_relay_peers{false};

std::atomic<int64_t> m_next_send_inv_to_incoming{0};
std::atomic<std::chrono::microseconds> m_next_send_inv_to_incoming{0us};

/**
* A vector of -bind=<address>:<port>=onion arguments each of which is
Expand All @@ -1269,13 +1269,7 @@ class CConnman
};

/** Return a timestamp in the future (in microseconds) for exponentially distributed events. */
int64_t PoissonNextSend(int64_t now, int average_interval_seconds);

/** Wrapper to return mockable type */
inline std::chrono::microseconds PoissonNextSend(std::chrono::microseconds now, std::chrono::seconds average_interval)
{
return std::chrono::microseconds{PoissonNextSend(now.count(), average_interval.count())};
}
std::chrono::microseconds PoissonNextSend(std::chrono::microseconds now, std::chrono::seconds average_interval);

/** Dump binary message to file, with timestamp */
void CaptureMessage(const CAddress& addr, const std::string& msg_type, const Span<const unsigned char>& data, bool is_incoming);
Expand All @@ -1284,7 +1278,7 @@ struct NodeEvictionCandidate
{
NodeId id;
int64_t nTimeConnected;
int64_t m_min_ping_time;
std::chrono::microseconds m_min_ping_time;
int64_t nLastBlockTime;
int64_t nLastTXTime;
bool fRelevantServices;
Expand Down
Loading