I was noticing a >10s delay between finding a block and broadcasting it:
2015-12-28 03:04:09 starting stake
2015-12-28 03:04:14 CWallet::CreateCoinStake() iteration 0
2015-12-28 03:04:20 CWallet::CreateCoinStake() iteration 15000
2015-12-28 03:04:25 CWallet::CreateCoinStake() iteration 30000
2015-12-28 03:04:25 selecting random speech with nRandom fbf8f24ad783f703 % 115084440191574 = 1425297350953
2015-12-28 03:04:25 selected random speech: '46.07% of the Just-Dice.com bankroll abstains from supporting any petition'
<- 11 second gap
2015-12-28 03:04:36 CWallet::CreateCoinStake() true
2015-12-28 03:04:36 successful stake took 27s
2015-12-28 03:04:36 CheckStake() : new proof-of-stake block found
I added extra logging to determine where the time was being spent:
2015-12-28 03:40:59 selecting random speech with nRandom b4c5bfeaa91cb6c4 % 115097570229426 = 91196244682794
2015-12-28 03:40:59 selected random speech: 'clamour 5afa074c 7a69a853'
2015-12-28 03:40:59 done select weighted speech
2015-12-28 03:40:59 checking if we need a new output
2015-12-28 03:40:59 check splitting
2015-12-28 03:40:59 check merging
<- 13 second gap
2015-12-28 03:41:12 extra output?
2015-12-28 03:41:12 signing
2015-12-28 03:41:12 successfully generated coinstake
2015-12-28 03:41:12 CWallet::CreateCoinStake() true
It turns out it was in the loop which checks to see whether any existing outputs can be merged with the staking output. The code was calling GetHash() on every output in the wallet to calculate its transaction ID, but the hash never changes so this can be cached.
As a further optimisation, it seems that the hash is only used to check that each coin being considered for merging isn't the coin being staked. If we removed the coin being staked from the set we're iterating through then this check would become completely unnecessary and could be removed. I'll make a separate pull request for that.
I was noticing a >10s delay between finding a block and broadcasting it:
I added extra logging to determine where the time was being spent:
It turns out it was in the loop which checks to see whether any existing outputs can be merged with the staking output. The code was calling GetHash() on every output in the wallet to calculate its transaction ID, but the hash never changes so this can be cached.
As a further optimisation, it seems that the hash is only used to check that each coin being considered for merging isn't the coin being staked. If we removed the coin being staked from the set we're iterating through then this check would become completely unnecessary and could be removed. I'll make a separate pull request for that.
Edit: the separate pull request is #275.