Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize mempool maps #873

Closed

Conversation

yyforyongyu
Copy link
Collaborator

Depends on #872, this PR removes a redundant Add(tx) for bitcoind ZMQ events, and optimized the memory usage of mempool based on the fact that, multiple inputs may share the same tx hash that leaves room for optimization.

Previously when adding a replacement tx to the local mempool, when the
txid's nested map becomes empty, we forgot to delete the key from the
root map. And when adding the same tx to the `txids` map(which happens
in ZMQ), we may treat the tx as a replacement tx. These are now fixed
and the related unit tests are added/updated to reflect the fix.
This commit moves the coinbase from updating inputs to adding txes so it
never enters the mempool's internal state. A unit test is patched to
guard the updates.
This commit creates a new interface for `rpcClient.Client` so it can be
easily mocked in mempool's unit tests.
This commit removes adding tx to mempool when received from `rawtx` zmq
to prevent the same tx gets to be added multiple times since it will be
notified multiple times. This also means we need to send the new tx in
`mempoolPoller` to make sure the subscriber won't miss the event.

The two good cases,
1. subscribe the input, `LookupInputSpend` return false
2. tx seen by `rawtx` or mempool poller
3. notified in `filterTx`

1. tx seen by `rawtx` or mempool poller
2. subscribe the input
3. will be notified via `LookupInputSpend`

The bad case is,
1. sent by `rawtx`, which marks the tx as seen in `filterTx`
2. subscribe the input, `LoopupInputSpend` return false
3. sent by the mempool poller, `filterTx` will ignore this tx, thus the
   subscriber won't be notified
This commit changes the data structure of the cachedInputs to more
efficiently store the data.
@yyforyongyu yyforyongyu marked this pull request as draft June 20, 2023 18:16
@yyforyongyu
Copy link
Collaborator Author

Unfortunately the optimization doesn't work based pprof, will investigate more.

@yyforyongyu
Copy link
Collaborator Author

Replaced by #877 which focuses on removing the redundant Add. I don't think the maps in mempool can be optimized more, given most of the inputs don't share the same tx hash, and most spending txes are consuming inputs from different parent txes. Plus the space overhead of using nested maps, my pprof says it actually takes more space.

@yyforyongyu yyforyongyu deleted the optimize-mempool-maps branch June 21, 2023 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant