Allow SendMessages to run partially without cs_main #2840

pull sipa wants to merge 3 commits into bitcoin:master from sipa:nosendlock changing 2 files +38 −22

sipa commented at 12:36 PM on July 20, 2013: member

SendMessages() tries to acquire a cs_main lock now, but this isn't nessecary for much of its functionality. Move those parts out of the if condition.
sipa commented at 12:40 AM on July 25, 2013: member

Added a few more lock improvements.
sipa commented at 10:24 PM on August 15, 2013: member

Rebased.
Diapolo commented at 12:03 PM on August 17, 2013: none

Does this make the code more stable or is it a performance-improving patch? That locking stuff isn't my main expertise ^^.
sipa commented at 12:22 AM on August 18, 2013: member

@Diapolo It improves performance/throughput, by allowing more code to run without needing the cs_main lock, and conversely, holding that lock for a shorter time, so more other code can run in parallel.

At this point the improvements may not be noticable, but if we ever want a decently parallellizable core, we can't avoid doing this everywhere it's possible.
sipa commented at 11:19 PM on October 14, 2013: member

Rebased.

PS: the first commit may look large, but it really just changes some indentation. If needed, I can make a version that keeps the indentation identical for now.

in src/main.cpp:None in 3d3de38dac outdated

4465 | +    {
4466 | +        const CInv& inv = (*pto->mapAskFor.begin()).second;
4467 | +        if (!AlreadyHave(inv))
4468 | +        {
4469 | +            if (fDebugNet)
4470 | +                printf("sending getdata: %s\n", inv.ToString().c_str());

gavinandresen commented at 3:09 AM on October 15, 2013:

Should be LogPrint("net", ...)

gavinandresen commented at 3:10 AM on October 15, 2013: contributor

Looks good to me except for an errant printf (reviewed using a graphical diff tool-- opendiff on the mac-- using its "compress whitespace" preference).
sipa commented at 8:54 AM on October 15, 2013: member

Modified to not do indentation changes for now.

Allow SendMessages to run partially without cs_main

SendMessages() tries to acquire a cs_main lock now, but this isn't nessecary
for much of its functionality. Move those parts out of the locked section,
so they can always be performed, and we hold cs_main for a shorter time.

6055b9101b

Run node deletions outside of cs_vNodes 49d754d915
Push down cs_main locking in ProcessMessage 7d38af3c49
BitcoinPullTester commented at 9:17 PM on October 15, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/7d38af3c493f9ea24c722ec2e6d3c51f4e851364 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
gavinandresen referenced this in commit 749230d05c on Oct 21, 2013
gavinandresen merged this on Oct 21, 2013
gavinandresen closed this on Oct 21, 2013
IntegralTeam referenced this in commit 6982d98548 on Jun 4, 2019
DrahtBot locked this on Sep 8, 2021

Contributors

Linked (view graph)

#3114 Give peer time-adjustment data an own lock