utACK 2a4338c5054906b664e027b1b4bb9c6b4ed50f05

utACK 2a4338c

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.

@kazcw I prefer seeing correct synchronization, even for variables that are only accessed from one thread. There are probably several such violations already, but over time, I think we'll want multiple message handler threads.

Can one of the admins verify this patch?

@kazcw Can you add some form of synchronization back to CNode::minFeeFilter?

Sure, I popped off the minFeeFilter commit.

Is there any advantage to making this boolean thread-local instead of atomic?

@laanwj I assume a thread-local is slightly faster as it doesn't need memory synchronization (but that's a pure guess).

On the other hand, an atomic would be nice to guarantee monotonicity...

@laanwj I'd thought it was a little simpler to avoid the need for cross-thread reasoning, but I'm not so sure; an atomic seems to map more directly to the intent here. I'd also made a similar performance assumption to @sipa, 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.

utACK b07b3f9c1a4b1a0b5f7189f9a0e45cf7547582cf

utACK a91c897
Please squash and fix the typo.

Squashed & fixed comment

utACK b07b3f9

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.