Avoid recalculating vchKeyedNetGroup in eviction logic. #8088

pull pstratem wants to merge 1 commits into bitcoin:master from pstratem:2016-05-22-efficient-keyed-eviction changing 2 files +29 −34

pstratem commented at 7:23 AM on May 23, 2016: contributor

Lazy calculate vchKeyedNetGroup in CNode::GetKeyedNetGroup.

This is a (very small) performance improvement.

Fix mentioned by gmaxwell in #8086
pstratem renamed this:
~~Avoid recalculating vchKeyedNetGroup in eviction logic.~~
[WIP]Avoid recalculating vchKeyedNetGroup in eviction logic.
on May 23, 2016
gmaxwell commented at 7:37 AM on May 23, 2016: contributor

bleh. There really is no reason to do this lazily. It should eagerly generate it at connection time, saving a hashing operation that it's going to perform anyways is not a good trade-off for having an extra heap allocation. Doubly so when the hash is changed to siphash.
Avoid recalculating vchKeyedNetGroup in eviction logic.
Lazy calculate vchKeyedNetGroup in CNode::GetKeyedNetGroup.
ee57c208da
pstratem force-pushed on May 23, 2016
pstratem renamed this:
~~[WIP]Avoid recalculating vchKeyedNetGroup in eviction logic.~~
Avoid recalculating vchKeyedNetGroup in eviction logic.
on May 23, 2016
pstratem commented at 12:39 PM on May 23, 2016: contributor

@gmaxwell i mean ok..
jonasschnelli added the label P2P on May 23, 2016
in src/net.h:None in ee57c208da
```
 364 | @@ -363,6 +365,8 @@ class CNode
 365 |      CBloomFilter* pfilter;
 366 |      int nRefCount;
 367 |      NodeId id;
 368 | +
 369 | +    std::vector<unsigned char> vchKeyedNetGroup;
```
sipa commented at 3:40 PM on May 24, 2016:

Use a uint256, or even a uint64_t.

gmaxwell commented at 6:08 PM on May 24, 2016:

uint64_t please. The probability that two distinct net-groups share a 64-bit hash is negligible, and if it ever happens its harmless (and would have no effect at all unless it's a collision with one of the four lowest ones that are in use).
sipa commented at 3:51 PM on May 24, 2016: member

Concept ACK. I think SHA256 is still overkill, but this is a clear improvement.
sipa commented at 1:04 PM on May 26, 2016: member

Included into #8086.
gmaxwell commented at 7:03 PM on June 10, 2016: contributor

This should be closed, was merged via 8086->8173.
pstratem closed this on Jun 11, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#8086 Use SipHash for node eviction