search

cometbft / cometbft-db

Database wrapper for CometBFT
Apache License 2.0
29 stars 51 forks source link

Make a second DB "IterKey" API that returns a key to the caller that can potentially mutate #156

Closed ValarDragon closed 3 months ago

ValarDragon commented 6 months ago

Currently the Iterator.Key() API makes a copy of the key it gets from the database. This is because the database's iterator returns something it will mutate on the subsequent .Next() call for heap efficiency. This extra copy causes very large heap allocation (and time overheads) to query serving nodes, and a 1% time overhead to the entire state machine time for Osmosis.

On a heap allocation profile of a query serving Osmosis RPC node over an hour, it has 450 gigabytes allocated from this API. On spot-check, none of the big ones need this copying behavior. (160GB removed from a tendermint update, but the remaining 290GB are still from this API)

image

In the state machine, we see 1% of state machine execution time is blocked on copying this key, again in situations where I don't think we need any of this either. image

Proposal: Add a new method KeyMut() to the interface for Iterator. The caller should not mutate this key, and the expectation is that the key may get mutated on the next .Next() call.

I'm not stoked about the naming of this method, so happy for better ideas

melekes commented 4 months ago

Do we even modify the key in CometBFT? Because the contract says CONTRACT: readonly []byte