Skip to content

Conversation

@kazcw
Copy link
Contributor

@kazcw kazcw commented May 2, 2016

I profiled bitcoind's memory usage and noticed that the tx hashes as keys in mapNextTx are taking a large amount of space, but they're redundant since the corresponding value has a pointer to the CTransaction that has the hash in it. On a system with 64-bit pointers, replacing these hashes with pointers or references would, once the mempool warms up, save about 7% of total memory usage.

I explored different ways of doing this.

I ruled out std::reference_wrapper (possibly the most straightforward way of using references as keys in a map), because it's a big footgun and also the syntax for using it would be weird; you'd have to static_cast<COutpoint&>(it->first) to get the "value" of a key from an iterator.

The simplest approach I found was to use a map<const COutPoint*, CInpoint, DereferencingComparator<const COutPoint*>> -- but this also required strange syntax: map.find(&outpoint) is misleading, since the outpoints will be compared as values.

The implementation here is a refinement of the DereferencingComparator approach: I created refmap as a simple proxy to a map that has pointers as keys and uses the DereferencingComparator; refmap passes through all methods except those that normally take a comparison value, which it takes by const reference and gives the address to the underlying map. This allows a map to use pointers as keys, and:

  • compare keys by pointed-to value
  • insert keys or return them from the map as pointers (which makes clear to the map user the constraint on the lifetime of the pointed-to key object)
  • perform value lookups by reference as a normal map would (making it clear that keys are compared by value)

kazcw added 3 commits May 2, 2016 12:55
Saves about 7% of total memory usage (halves the size of each entry in
mapNextTx, which comprised about 15% of memory usage in overnight test run).
Since the mempool is DynamicUsage-regulated, this will translate to a larger
mempool in the same amount of space.
@dcousens
Copy link
Contributor

dcousens commented May 3, 2016

Any reason for close?

@kazcw
Copy link
Contributor Author

kazcw commented May 3, 2016

I'm exploring an alternative solution; I'll PR one or the other soon.

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants