lighthouse/beacon_node
Paul Hauner 4c7bb4984c Use the forwards iterator more often (#2376)
## Issue Addressed

NA

## Primary Change

When investigating memory usage, I noticed that retrieving a block from an early slot (e.g., slot 900) would cause a sharp increase in the memory footprint (from 400mb to 800mb+) which seemed to be ever-lasting.

After some investigation, I found that the reverse iteration from the head back to that slot was the likely culprit. To counter this, I've switched the `BeaconChain::block_root_at_slot` to use the forwards iterator, instead of the reverse one.

I also noticed that the networking stack is using `BeaconChain::root_at_slot` to check if a peer is relevant (`check_peer_relevance`). Perhaps the steep, seemingly-random-but-consistent increases in memory usage are caused by the use of this function.

Using the forwards iterator with the HTTP API alleviated the sharp increases in memory usage. It also made the response much faster (before it felt like to took 1-2s, now it feels instant).

## Additional Changes

In the process I also noticed that we have two functions for getting block roots:

- `BeaconChain::block_root_at_slot`: returns `None` for a skip slot.
- `BeaconChain::root_at_slot`: returns the previous root for a skip slot.

I unified these two functions into `block_root_at_slot` and added the `WhenSlotSkipped` enum. Now, the caller must be explicit about the skip-slot behaviour when requesting a root. 

Additionally, I replaced `vec![]` with `Vec::with_capacity` in `store::chunked_vector::range_query`. I stumbled across this whilst debugging and made this modification to see what effect it would have (not much). It seems like a decent change to keep around, but I'm not concerned either way.

Also, `BeaconChain::get_ancestor_block_root` is unused, so I got rid of it 🗑️.

## Additional Info

I haven't also done the same for state roots here. Whilst it's possible and a good idea, it's more work since the fwds iterators are presently block-roots-specific.

Whilst there's a few places a reverse iteration of state roots could be triggered (e.g., attestation production, HTTP API), they're no where near as common as the `check_peer_relevance` call. As such, I think we should get this PR merged first, then come back for the state root iters. I made an issue here https://github.com/sigp/lighthouse/issues/2377.
2021-05-31 04:18:20 +00:00
..
beacon_chain Use the forwards iterator more often (#2376) 2021-05-31 04:18:20 +00:00
client Monitoring service api (#2251) 2021-05-26 05:58:41 +00:00
eth1 Reduce outbound requests to eth1 endpoints (#2340) 2021-05-31 04:18:18 +00:00
eth2_libp2p Minimum Outbound-Only Peers Requirement (#2356) 2021-05-31 04:18:19 +00:00
genesis Add SensitiveUrl to redact user secrets from endpoints (#2326) 2021-05-04 01:59:51 +00:00
http_api Use the forwards iterator more often (#2376) 2021-05-31 04:18:20 +00:00
http_metrics Tune GNU malloc (#2299) 2021-05-28 05:59:45 +00:00
network Use the forwards iterator more often (#2376) 2021-05-31 04:18:20 +00:00
operation_pool Pack attestations into blocks in parallel (#2307) 2021-04-13 05:27:42 +00:00
src Tune GNU malloc (#2299) 2021-05-28 05:59:45 +00:00
store Use the forwards iterator more often (#2376) 2021-05-31 04:18:20 +00:00
tests Update to tokio 1.1 (#2172) 2021-02-10 23:29:49 +00:00
timer Update to tokio 1.1 (#2172) 2021-02-10 23:29:49 +00:00
websocket_server Server sent events (#1920) 2020-12-04 00:18:58 +00:00
Cargo.toml Monitoring service api (#2251) 2021-05-26 05:58:41 +00:00