RPC: Get rid of the internal miner’s hashmeter #5599

1. 
   jtimon commented at 8:12 pm on January 4, 2015: contributor

   Removes “hashespersec” in “getmininginfo” and also removes “gethashespersec”. That also allows the removal of some code in miner.cpp, simplifying ScanHash() and BitcoinMiner().
2. Get rid of the internal miner's hashmeter 0cc0d8d60b
3. jtimon renamed this:
   Get rid of the internal miner's hashmeter
   RPC: Get rid of the internal miner's hashmeter
   on Jan 4, 2015
4. 
   gavinandresen commented at 5:03 pm on January 5, 2015: contributor

   Code review ACK, but there is a functional change: with this code the internal miner will never search the nonce space above 0xfff.

   Also: how did you test this?

5. 
   jtimon commented at 7:30 pm on January 5, 2015: contributor

   We discussed this in #4793 . @sipa said in some outdated commit that removing if ((nNonce & 0xffff) == 0) was safe but I can’t remember the reason why it was that if instead of the later if ((nNonce & 0xfff) == 0) what can be removed. Now that you’re saying it I think it is a mistake and what can be removed is just:

   0if ((nNonce & 0xfff) == 0)
   1    boost::this_thread::interruption_point();
   

6. 
   sipa commented at 11:05 pm on January 5, 2015: member

   @gavinandresen I haven’t tested, but it’s just ScanHash that returns after 0x1000 iterations - the mining loop continues with the same nonce until 0xffff0000 is reached (which could actually become 0xfffff000 then, but who cares).
7. 
   sipa commented at 3:37 pm on January 6, 2015: member

   Tested ACK: adding a LogPrintf("Nonce: %x\n", nNonce); in the loop that calls ScanHash, and running setgenerate true in -testnet mode confirms higher nonces are being tried.
8. 
   jtimon commented at 11:36 pm on January 7, 2015: contributor

   Oh, here it was the missing piece of earlier conversation: https://github.com/jtimon/bitcoin/commit/9d12ff72a6f97ed1e53e0d106abe1a3a7e1f56f8#diff-4a59b408ad3778278c3aeffa7da33c3cR384
9. laanwj added the label Mining on Jan 8, 2015
10. 
    laanwj commented at 12:02 pm on January 8, 2015: member

    @luke-jr this is mining related, can you ACK?
11. in src/miner.cpp: in 0cc0d8d60b

    392@@ -395,10 +393,8 @@ bool static ScanHash(const CBlockHeader *pblock, uint32_t& nNonce, uint256 *phas
    393             return true;
    394 
    395         // If nothing found after trying for a while, return -1
    396-        if ((nNonce & 0xffff) == 0)
    397-            return false;
    398         if ((nNonce & 0xfff) == 0)
    399-            boost::this_thread::interruption_point();
    400+            return false;
    

    ---

    ---

    

    luke-jr commented at 12:32 pm on January 8, 2015:

    I think this is okay, but seems unrelated from the PR description?

    ---

    

    jtimon commented at 6:46 pm on January 8, 2015:

    @maaku suggested on #4793 when I changed this boost::this_thread::interruption_point() to return false that the change belonged to the removal of the hashmeter as it was the reason for the interruption point to be there in the first place.
12. 
    luke-jr commented at 1:16 pm on January 8, 2015: member

    ACK, only tested on regtest since testnet-in-a-box style apparently refuses to mine in master… :/
13. 
    luke-jr commented at 1:20 pm on January 8, 2015: member

    ACK on testnet via gdb testing
14. 
    gavinandresen commented at 8:52 pm on January 8, 2015: contributor

    Better is better… so ACK.

    But after removing the hashmeter, and assuming we no longer care about performance of the CPU miner (we don’t) it looks to me like the code could be simplified a lot by getting rid of ScanHash and just having the mining loop be:

     0   while true:
     1       ... create block...
     2       ... serialize header without nonce..
     3       set nonce to zero
     4       do:
     5           compute hash with nonce
     6           ++nonce
     7           if hash <= target: BLOCK FOUND
     8           if nonce%0xfff == 0 : interruption point
     9        while nonce < 0xffffffff
    10        Increment extraNonce
    11  end while true
    

15. 
    jtimon commented at 7:19 pm on January 9, 2015: contributor

    Yes, more simplifications can be done later. This is what I had in mind for next steps: https://github.com/jtimon/bitcoin/commit/a029a67fd29bcc04b463bd4dcf7507b5b7e0ed7f https://github.com/jtimon/bitcoin/commit/da36fb5ebb8655392ec278dd632bdf7a52531e0f Different things can be done (my preference is that we share as much code as possible with regtest, optimizing only for regtest), but most of the options I have considered imply the removal of the hashmeter as a first step, so here it is isolated from other decisions that people seem to care less about.
16. 
    jaromil commented at 1:06 pm on January 24, 2015: contributor

    ut ACK.

    lovely to have this part of the code simplified.

17. laanwj merged this on Jan 24, 2015
18. laanwj closed this on Jan 24, 2015

    ---

19. laanwj referenced this in commit 40e96a3016 on Jan 24, 2015
20. MarcoFalke locked this on Sep 8, 2021

Contributors
jtimon gavinandresen sipa laanwj luke-jr jaromil

Labels
Mining