Speed up generating blocks in regression test mode, by moving block-creating and nonce-finding directly into the setgenerate RPC call (instead of starting up a mining thread and waiting for it to find a block).
This makes the forknotify RPC test three times quicker, for example (10 seconds runtime instead of 30 seconds, assuming the initial blockchain cache is already built).
164@@ -161,19 +165,30 @@ Value setgenerate(const Array& params, bool fHelp)
165 nHeightEnd = nHeightStart+nGenerate;
166 }
167 int nHeightLast = -1;
Unused:
0rpcmining.cpp: In function ‘json_spirit::Value setgenerate(const Array&, bool)’:
1rpcmining.cpp:167:13: warning: unused variable ‘nHeightLast’ [-Wunused-variable]
2 int nHeightLast = -1;
3 ^
Speed up generating blocks in regression test mode, by moving
block-creating and nonce-finding directly into the setgenerate
RPC call (instead of starting up a mining thread and waiting for
it to find a block).
This makes the forknotify RPC test three times quicker, for
example (10 seconds runtime instead of 30 seconds, assuming
the initial blockchain cache is already built).
I am not opposed to optimizing regtest block generation. Heavy users of regtest, apparently Gavin included, might find a use case or two. After all, testing has been declared the bottleneck for development in general, so we need more of it, and I view this commit as a step in support of it.
What seems out of place in the context of regtest here are error messages coming out of CheckProofOfWork, while the right nonce is searched for, misleadingly clogging the logs.
Heavy users of regtest, apparently Gavin included, might find a use case or two
We’re all heavy users of regtest when waiting for Travis to finish. Optimizing here is indeed an overall win.
The logical conclusion is that the miner optimization doesn’t make sense. Let’s remove it to simplify that part of the code. Less code to maintain and no drawbacks.
That has been discussed before. Another feature that complicates the internal miner code, and doesn’t have much of a point for testing, is support for multiple threads. I’d not be opposed to removing those things. But anything related to the (non-regtest) internal miner has extremely low priority in my eyes. It works …
if (Params().MineBlocksOnDemand()) part smaller and the whole mining code simpler if we’re willing to sacrifice non-regtest performance.
There’s also another way to make regtest mining more similar to the others: by removing mining code that regtest doesn’t use like in #5599 (or @laanwj ’s proposal to remove multiple thread support for non-regtest mining).
To reiterate and summarize: non-regtest mining performance is meaningless, and non-regtest mining code should be more like regtest mining (instead of the other way around).