search

tendermint / tm-db

Common database interface for various database backends for Tendermint Core and Cosmos SDK
Apache License 2.0
89 stars 136 forks source link

memdb: Implement btree.Item on `item` directly, in addition to `*item` #186

Closed ValarDragon closed 2 years ago

ValarDragon commented 3 years ago

Currently we force every btree item to be *item, not just item. This in turn causes every set to require new heap allocations, which turns out to be a significant overhead. These allocations cause lots of memory to need to be garbage collected, and this ends up often being the bottleneck, not the actual btree searching operations.

In Osmosis, we get that we have ~no item spent in the btree operations, its all spent in allocating the newPair onto Heap. Heres a screenshot of a relevant flamegraph output here:

Screenshot 2021-09-03 at 12 28 35 PM
tac0turtle commented 3 years ago

Hey, I can help in getting a release out to fix this, if you are willing to make a PR. I can push a branch for you to target as master has diverged too much to be used for this

ValarDragon commented 3 years ago

smh stale bot

ValarDragon commented 3 years ago

Can we please remove stale bot issue closing =/ Its not even that long of a wait atm

tac0turtle commented 3 years ago

yea ill fix

faddat commented 2 years ago

What's the status of this one?

I guess I want to include it in the notinal tracking issue

@ValarDragon do you think this is needed still?

ValarDragon commented 2 years ago

Yeah this is still useful

faddat commented 2 years ago

great!

faddat commented 2 years ago

So, fair to say this one is more important than #188 ?

that’s good because ah, wow

Pain is a teacher I guess?

faddat commented 2 years ago

Seems set :)