AleoNet / snarkOS

A Decentralized Operating System for ZK Applications
http://snarkos.org
Apache License 2.0
4.24k stars 2.59k forks source link

Reduce the number of lookups in update_peer_locators #3236

Closed ljedrz closed 2 months ago

ljedrz commented 3 months ago

The update_peer_locators method is quite prominent in heap profiles, and tweaking it could yield considerable improvements.

This PR proposes a simple change: instead of starting from height 0 and "going up" the received list of locators (which would be most optimal in case of a deep fork) it starts with the last locator and, if it's not found in the local storage, goes over the remaining locators in reverse order, stopping once the most recent ancestor is found (which is much faster in the "happy path" scenario).

In a local --dev run with 4 validator nodes, this reduces the number of allocations in a non-tx-producing node by ~1/3 after 1h. Also, while this is presented as a memory-related improvement, this change should result in reduced (peer) block sync time as well.

As for whether this is as safe as the current setup; a malicious node that knows the most recent valid locator would also be able to provide valid past locators (and it would take longer to process those as well). As mentioned before, the one downside that I see is that this change would make this operation slower for very deep forks, where the number of diverging blocks is larger than the number of canonical ones.

howardwu commented 2 months ago

Closing this PR, and adding an important comment in commit https://github.com/AleoHQ/snarkOS/commit/913db14f08e34d7a27814194a22c9400fe599691 to the file.