A tweak to IsInitialBlockDownload prevents taking cs_main most of the time: it already remembers a negative result to save some work; doing so per-thread allows the check to be pulled out of the lock.
cs_feeFilter is used only to guard minFeeFilter, which is an integer. This is the simplest use case of an atomic variable: the only guarantee needed is atomicity.
Force-pushed a new head commit: I just noticed that minFeeFilter is actually only accessed in the message handling thread and doesn’t need any kind of synchronization.
The IsInitialBlockChanges commit is unmodified.
minFeeFilter commit.
1574@@ -1574,18 +1575,24 @@ CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams)
1575 bool IsInitialBlockDownload()
1576 {
1577 const CChainParams& chainParams = Params();
1578+
1579+ // Once IIBD is first found to be false, it must remain false.
Optimistically test the latch bool before taking the lock.
For all IsInitialBlockDownload calls after the first to return false,
this avoids the need to lock cs_main.
utACK https://github.com/bitcoin/bitcoin/commit/b07b3f9c1a4b1a0b5f7189f9a0e45cf7547582cf
but looking in to it now there are apparently some thorny thread-local storage implementations in the wild. So, I’ve reimplemented it with an atomic for clarity and consistent performance.
Right. On hardware that natively supports atomic operations (which are most modern CPUs), I’d expect an atomic to be slightly more performant. Thread-local variables, classically at least, come with serious overhead, like having to set up a special segment. That’s worth it for larger per-thread data structures, but not one boolean.
Labels
Refactoring