snarkOS icon indicating copy to clipboard operation
snarkOS copied to clipboard

Cache transmissions for faster processing

Open kpandl opened this issue 1 year ago • 4 comments

Motivation

The prepare_advance_to_next_quorum_block function incurs significant processing time, which contributes to increased block times. Based on a flamegraph analysis, time-consuming operations are the deserialization of objects, or more general, retrieving transmissions from storage.

Therefore, this PR introduces an LRU cache for transmissions inside the BFTPersistentStorage. The transmissions are inserted into the cache in the insert_transmissions function, alongside with the insertion into the persistent storage. The get_transmission function will first try to retrieve the transmissions from the LRU cache, and if it fails, try to retrieve it from the persistent storage.

Test Plan

Ran it locally, and also ran a load test (high tps of transfer_public executions) on aws. Thereby, we observed a ~50% reduction in block times.

Please note: The cache aims to cover two rounds of transmissions. As such, it can store MAX_COMMITTEE_SIZE * MAX_TRANSMISSIONS_PER_BATCH * 2 = 1500 transmissions. At the current MAX_TRANSACTION_SIZE of 128 kB, this results in approximately 188 MB additional memory use.

kpandl avatar Sep 05 '24 13:09 kpandl

CI is failing. I suggest running node and node-bft locally to find out if its actually failing. For node-router, consider using a more lenient deadline instead of the sleep (see other tests using deadline! for examples)

vicsn avatar Sep 10 '24 12:09 vicsn

Thanks for the feedback! I've updated the code. The node test ran successfully locally. The node-bft test did not, but that was likely due to constraint local resources, not an issue with the code itself.

kpandl avatar Sep 12 '24 13:09 kpandl

@kpandl this branch has conflicts. Could you please resolve them before we attempt to merge?

zosorock avatar Oct 18 '24 22:10 zosorock

@zosorock thanks, I've merged the latest staging into this branch, there should be no conflicts now

kpandl avatar Oct 21 '24 11:10 kpandl