Do not use signals for communication between net and net_processing. #9442

pull gmaxwell wants to merge 1 commits into bitcoin:master from gmaxwell:no_node_signals changing 7 files +57 −94

gmaxwell commented at 5:31 PM on December 29, 2016: contributor

The code was previously using boost::signals2 for communication between net and net_processing. Because there are no other participants and the relationship was completely static, this was providing no value.

Boost signals are rather complex and expensive compared to a direct function call: They involve locking and roughly a screen full of backtrace, an they make the control flow less clear. In places where there are multiple participants or wiring that changes, they can be worth their cost-- but none of that is being used in this case and I can't foresee it being used here.

I also found that removing this indirection resulted in a ~4% IBD speedup on top of #9415.

Perhaps someone has a reason we should be using this? I asked sipa before opening this PR but didn't seem to know.

Do not use signals for communication between net and net_processing.

The code was previously using boost::signals2 for communication between
 net and net_processing.  Because there are no other participants and
 the relationship was completely static, this was providing no value.

Boost signals are rather complex and expensive compared to a direct
 function call:  They involve locking and roughly a screen full of
 backtrace, an they make the control flow less clear. In places
 where there are multiple participants or wiring that changes,
 they can be worth their cost-- but none of that is being used in this
 case and I can't foresee it being used here.

I also found that removing this indirection resulted in a ~4% IBD
 speedup on top of #9415.

dd718c8f54

gmaxwell commented at 5:35 PM on December 29, 2016: contributor

I was surprised it made that much of a performance difference, and I wouldn't completely discount measurement error, but I did test it four times each way and got a pretty consistent speedup.
sipa commented at 5:35 PM on December 29, 2016: member

I'm fine with avoiding signals when unneeded, but is it possible to avoid a circular dependency between net and net_processing?
gmaxwell closed this on Dec 29, 2016
theuni commented at 5:54 PM on December 29, 2016: member

Yes, with an abstract class. Another part of https://github.com/theuni/bitcoin/commit/1a6b10aea129ac363727c2d68fae809c2861e4da which I'm integrating one chunk at a time.
DrahtBot locked this on Sep 8, 2021

Contributors

gmaxwell

sipa

theuni

Linked (view graph)

#9415 Reduce latency of ThreadMessageHandler. (3.2x speedup for IBD to 200k)