Fix Parallel verification issues (v2)#1423
Fix Parallel verification issues (v2)#1423shargon wants to merge 9 commits intoneo-project:masterfrom
Conversation
cloud8little
left a comment
There was a problem hiding this comment.
Test Passed. Wallet with 10000 neo, 500 gas, send rpc (sendtoaddress)requests for 2 times, one with 400 parrallel threads. finally, 493 tx are dealt with, and the balance is consistent with the result without this PR.
Kindly let me know if you want other user scenarios involved.
Are you be able to reproduce the issue? |
| reason = RelayResultReason.OutOfMemory; | ||
| else if (parallelVerified.ShouldRelay) | ||
| system.LocalNode.Tell(new LocalNode.RelayDirectly { Inventory = parallelVerified.Transaction }); | ||
| reason = parallelVerified.Transaction.VerifyForEachBlock(currentSnapshot, MemPool.SendersFeeMonitor.GetSenderFee(parallelVerified.Transaction.Sender)); |
There was a problem hiding this comment.
if (hashes.Length != Witnesses.Length) return RelayResultReason.Invalid;
for (int i = 0; i < hashes.Length; i++)
{
if (Witnesses[i].VerificationScript.Length > 0) continue;
if (snapshot.Contracts.TryGet(hashes[i]) is null) return RelayResultReason.Invalid;
}Maybe some of these could be avoided, at least the first line.
But it is a just a minor thing.
I am not sure if the snapshot could contain a previous contract that is now deleted.
There was a problem hiding this comment.
If we remove the first line, we can have an exception
|
@shargon @eryeer Here is the TPS result for comparing #1423 (parallel thread verification) and #1429 (single thread verification), it shows tps with parallel thread verification is nearly double of single thread verification: Test Env: Test Condition: Total Tx: 1000,000txs Test Result: Test Env: Test Condition: Total Tx: 500,000txs Test Result: |
|
Good to know! @cloud8little thanks! |
|
@erikzhang Could you approve it? Test is OK. |
| reason = RelayResultReason.OutOfMemory; | ||
| else if (parallelVerified.ShouldRelay) | ||
| system.LocalNode.Tell(new LocalNode.RelayDirectly { Inventory = parallelVerified.Transaction }); | ||
| reason = parallelVerified.Transaction.VerifyForEachBlock(currentSnapshot, MemPool.SendersFeeMonitor.GetSenderFee(parallelVerified.Transaction.Sender)); |
There was a problem hiding this comment.
I think no. We need to check the L410 line in advance to avoid wasting resources on parallel VerifyWitness for unqualified transactions.
There was a problem hiding this comment.
I am not sure, we save resource in certain cases and waste in other, we should check what it will be more usual.
There was a problem hiding this comment.
Perhaps reverting to single-threaded mode is currently the best option.
There was a problem hiding this comment.
This was this alternative #1429 but the speed it's lower
Close #1420
Alternative of #1417