Buffer block downloading and commit/check blocks in parallel #1233
Conversation
|
For the record, cblockstore's download times from local nodes comes in reliably under master, but only by a very tiny margin. This, on the other hand, comes in around 20% lower on tmpfs chain sync. |
| printf("mapBlockIndex.size() = %d\n", mapBlockIndex.size()); | ||
| printf("nBestHeight = %d\n", nBestHeight); | ||
| //printf("mapBlockIndex.size() = %d\n", mapBlockIndex.size()); | ||
| printf("BestBlockHeight = %d\n", pblockstore->GetBestBlockIndex()->nHeight); |
There was a problem hiding this comment.
Could comment out this line too, since the height is mentioned in debug.log just few lines before this line.
|
can this pull request be done in such a way that there isn't so much of a diff output? Did so much code need to move from main.cpp to protocol.cpp, for example? |
|
Note that this pull is based on #771, which has radically different design goals. This pull is fairly small on its own. |
|
I think those are very sensible design goals. That said, in that case we should definitely pull #771 first, to prevent unrelated changes being merged into one pull request. Github seems to almost lock up when I try to view the diff. |
|
Ah, ok. Well, I'd certainly find the diffs easier to view if they were kept small. Probably would be better therefore to base this commit from a post-#771 commit then, in order not to effectively include that commit in its entirety within this commit, then it can be reviewed without having to manually save the files and manually diff them. |
|
@TheBlueMatt, do you mean within github? I'm not sure how to do that. Could you provide a URL, perhaps? |
|
You have to manually compare the list of commits, and then you can just open each commit from the commits list in the pull...if you feel like doing some URL hacking, you will notice git style ...s in diff URLs which you can replace manually using any branch like: |
|
Rebased onto #1429 |
The goal is for p2p code/wallet to only get information/communicate information about the blockchain through CHub, giving Bitcoin a much more clearly-defined structure and allowing for the removal of a ton of the current global mess.
Replacing ProcessBlock with EmitBlock, and creating callbacks for CommitBlock.
Also rename NotifyBlocksChanged to NotifyNewBlock and remove from the uiInterface signals list. This removes some functionality, but NofiyBlocksChanged was not used anyway, so it shouldn't matter. That said, if it is ever needed, it would be fairly trivial to add a new callback for it in CHub.
Replace ProcessAlert with calls to EmitAlert and create callbacks for CommitAlert.
There was no reason to use CDataStream as the transaction was already being serialized/deserialized several times, with this change, transactions coming in over network are deserialized once when received, and then only reserialized in the call to RelayMessage, which will be called in a callback thread, not blocking cs_main.
* This removes not only CTransaction::AcceptToMemoryPool, but
also CMerkleTx::AcceptToMemoryPool. It also moves
CWalletTx::AcceptWalletTransaction to wallet.cpp
* This adds a fCheckInputs flag to EmitTransaction, which is
similar to the fCheckInputs flag to AcceptToMemoryPool,
however, it has stricter guidlines that it should only be set
"when transaction is a supporting tx for one of our own."
Additionally, "fCheckInputs is ignored (and set to true)
if !IsInitialBlockDownload() && !fClient"
As a part of these guidelines,
CWalletTx::AcceptWalletTransaction calls EmitTransaction with
fCheckInputs set to true (the default) on the final
transaction, whereas it used to call with fCheckInputs set to
false. This has the important side-effect of allowing wallet-
generated transactions to end up getting AddOrphanTx'd.
However, if a supporting transaction to one of our own had
previously been AddOrphanTx'd, it would immediately be added
to memory pool as it is "a supporting tx for one of our own"
and thus is re-added with fCheckInputs=false.
Note that the possibility of a wallet transaction getting
AddOrphanTx'd is very low, and should only happen if
a) a transaction's input is a generate and we are missing that
block (note that no transactions should be generated with a
generation input if we don't have that block anyway).
b) We match the !IsInitialBlockDownload() && !fClient check,
are not caught up to the latest block, and an input is in a
block we do not yet have (possible after the last
checkpoint). This situation is temporary and should resolve
itself once we catch up (though AddOrphanTx'd transactions
may be permanently orphaned).
Largely, these guidelines are there because there is no reason
to add a transaction without checking its inputs, as we have
those inputs available, and checking them as any other
transaction would provides additional sanity-checks.
* A second EmitTransaction was added with tx of type CMerkleTx.
This keeps behavior of CMerkleTx::AcceptToMemoryPool the same
in fClient mode. Note that new behavior was invented for
CHub::EmitTransaction(CTransaction&...) in fClient mode,
namely that ClientConnectInputs is only checked if
fCheckInputs is true. This was chosen to make emitting a
transaction possible in fClient mode even if its inputs are
not available, but could be changed if support for that is not
needed when fClient mode is actually implemented.
Note that this changes the way the GUI shows coinbases of users: it now shows them only after the first non-orphan block based on the generating block, instead of after the first block based on the generating one period.
| CBlock* pblock2 = new CBlock(*pblock); | ||
| mapOrphanBlocks.insert(make_pair(hash, pblock2)); | ||
| mapOrphanBlocksByPrev.insert(make_pair(pblock2->hashPrevBlock, pblock2)); | ||
| printf("CBlockStore::FinishEmitBlock: ORPHAN BLOCK, prev=%s\n", block.hashPrevBlock.ToString().substr(0,20).c_str()); |
There was a problem hiding this comment.
Why make this line longer than it already was?
There was a problem hiding this comment.
Because the name of the function it is in, is being renamed.
This removes the locking of cs_main before calling Emit* and should allow multiple ProcessMessage calls at once. It no doubt locks cs_main in places where it isnt neccessary, but to avoid complicated and unforseen interactions, cs_main is locked for nearly every message in ProcessMessage.
It has a fBlocking flag to wait for the block being requested to be committed after having been emitted (for use in the "force request" block request in the "inv" message handler). This fixes a potential segfault if -blockbuffersize is overly big.
This resolves a bug where the block buffer is allowed to deplete because the getdata that is required to continue is not sent.
This keeps the block buffer from running out until the final block's FinishEmitBlock is run.
|
This needs rebasing, and Im not going to keep rebasing this stuff without any interest in eventually merging. If it ever gets interest, I may reopen. |
tps added for transactions accepted into the mempool. Tidy up formatting for getmempoolinfo help
f9857f7 [Trivial] Remove spammy log in in StakeV1 (random-zebra) Pull request description: This is not an error, the function should just return false. No need to spam the debug.log. This will become a non-issue after time v2 enforcement, but better to fix it anyway. ACKs for top commit: furszy: ACK f9857f7 Fuzzbawls: ACK f9857f7 Tree-SHA512: dc624fd7d81994f7aaac2b1a421000d05ca402d2864985cadbfd44f668971f6f2ed40138aa31615054cef02a71ca9f245eb0b7b19c72aea5ef4a52fcbcaadb36
Pull request description: This pull request updates the release notes for Omni Core 0.11.0.
This is based on cblockstore and commits blocks in a separate thread from downloading/initial checking (in CheckBlock). It offers a nice performance gain over cblockstore alone.