Hash P2P messages as they are received instead of at process-time #9045

1. 
   TheBlueMatt commented at 10:09 pm on October 30, 2016: member

   Better to hash as you receive bytes from the wire so that you reduce latency when you have the full message, arguably. If anything else probably best to not do the hashing in the message processing thread.
2. Hash P2P messages as they are received instead of at process-time fe1dc62cef
3. 
   sipa commented at 10:28 pm on October 30, 2016: member

   utACK. Somehow I thought we were already doing this.
4. fanquake added the label P2P on Oct 31, 2016
5. 
   gmaxwell commented at 6:30 am on October 31, 2016: contributor

   ACK
6. 
   rebroad commented at 8:50 am on October 31, 2016: contributor

   @TheBlueMatt Did you time how long the hashing was taking on a typical full block?
7. 
   paveljanik commented at 12:16 pm on October 31, 2016: contributor

   @rebroad This is about P2P messages and their checksums…
8. 
   paveljanik commented at 12:16 pm on October 31, 2016: contributor

   ACK https://github.com/bitcoin/bitcoin/pull/9045/commits/fe1dc62cef88280d2490a619beded052f313c6fc
9. 
   TheBlueMatt commented at 12:33 pm on October 31, 2016: member

   I did not Rome the hashing, though I expect it to be between 3 and 10 ms (we use a particularly slow SHA256 implementation, as we don’t incorporate any handcoded asm), but the lower bound is about 3, assuming it’s cache-hot.

   On October 31, 2016 4:50:20 AM EDT, R E Broadley notifications@github.com wrote:

   @TheBlueMatt Did you time how long the hashing was taking on a typical full block?

   You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #9045 (comment)

10. 
    theuni commented at 8:48 pm on October 31, 2016: member

    mm, NACK unless there are numbers that show that this is preferable.

    1. This removes our ability to selectively hash. If we’re flooded with thousands of messages that we would otherwise drop after the first, we still end up hashing them.
    2. Removes the ability to skip hashing for known values. For example, hashing for “sendheaders” could be skipped and we could just check against the known-correct value.
    3. The socket polling loop needs to be as tight as possible.
    4. Message processing could definitely be done multi-threaded, but multi-threaded polling is unlikely.
11. 
    TheBlueMatt commented at 10:46 pm on October 31, 2016: member

    @theuni While hashing is incredibly fast, it can be cause a big hit to latency for time-from-block-receive-to-relay (lower bound on hashing 1MB is around 3ms, whereas total runtime for deserialize and AcceptBlock, not including storage, is something like 15ms), especially if you received the full “block” message. Doing it as packets come in off the wire should never be a bottleneck - hashing is, more often than not, going to be able to easily keep up with your wire speed. This means that you can hash the message as it comes off the wire and when you receive the last packet (ie are ready to start processing) you dont have to wait for hashing to complete.

    Indeed, it complicates CConnman somewhat but CNetMessage will likely have to change for protocol encryption anyway, so its not like the logic in the same place isnt gonna have to learn protocol encryption.

12. 
    theuni commented at 7:07 pm on November 7, 2016: member

    @TheBlueMatt ok, I’ve come around on this. Thanks to @gmaxwell and @sipa for the discussion over the weekend. utACK https://github.com/bitcoin/bitcoin/pull/9045/commits/fe1dc62cef88280d2490a619beded052f313c6fc.
13. sipa merged this on Nov 7, 2016
14. sipa closed this on Nov 7, 2016

    ---

15. sipa referenced this in commit 9f554e03eb on Nov 7, 2016
16. codablock referenced this in commit 9b8cc0b1ce on Jan 15, 2018
17. andvgal referenced this in commit 13874fc795 on Jan 6, 2019
18. CryptoCentric referenced this in commit 5b6a3dac57 on Feb 24, 2019
19. barrystyle referenced this in commit ae602cc60f on Jul 6, 2019
20. random-zebra referenced this in commit bffe509aed on Jun 28, 2021
21. DrahtBot locked this on Sep 8, 2021

Contributors
TheBlueMatt sipa gmaxwell rebroad paveljanik theuni

Labels
P2P