Skip to content

Conversation

bhartnett
Copy link
Contributor

@bhartnett bhartnett commented Oct 2, 2025

This PR removes the snapshots cache from the forked chain. My reasoning for removing it is mainly due to the fact that when we create a snapshot on a txFrame if there is an existing snapshot on any parent frame then it will be moved to the new frame. This means that parent snapshots will be removed once processing the next block and so I believe the logic in the updateSnapshot function was flawed because it cleans up old snapshots which usually would have already been moved.

When processing multiple branches/heads at once it is more likely that the clearSnapshot function will actually remove a snapshot but in that case we might be clearing a needed snapshot. When processing blocks on a longer chain the snapshots from the shorter chains are more likely to get cleaned up.

If we just snapshot each block without using the snapshot cache then each of the branch heads will hold one snapshot and so if we are processing 3 branches in parallel then we will only have 3 snapshots in memory. This is simpler and doesn't risk clearing snapshots that are actually needed.

The downside of doing this change is the potential for increased memory usage if and only if we somehow need to process a large amount of branches in parallel (more than 10 for example)

@arnetheduck
Copy link
Member

The aim of this cache as I remember it was to ensure that it would be fast to switch branches for shallow forks (ie a few blocks deep) and not so much to deal with long-running forks - the latter are rare while the former happen on a regular basis.

@bhartnett
Copy link
Contributor Author

The aim of this cache as I remember it was to ensure that it would be fast to switch branches for shallow forks (ie a few blocks deep) and not so much to deal with long-running forks - the latter are rare while the former happen on a regular basis.

Switching branches should still work fine without this cache. The snapshot is stored in the head txFrame of each branch.

@arnetheduck
Copy link
Member

Switching branches

It's not so much about switching branches, but rather creating a new branch from a recent block, most often head - 1 - ie the vast majority of branches happen because the "current head" gets orphaned or that the proposer is a block late and tries to build on the previous head instead.

@bhartnett
Copy link
Contributor Author

It's not so much about switching branches, but rather creating a new branch from a recent block, most often head - 1 - ie the vast majority of branches happen because the "current head" gets orphaned or that the proposer is a block late and tries to build on the previous head instead.

Regardless of the scenario this 'cache' does not actually hold 10 snapshots for the last 10 blocks, but rather it holds the txFrames of the last 10 blocks where the updateSnapshot function was called. Every time we create a new snapshot, the last snapshot in the chain is moved to the current txFrame so my main point is that this data structure may not be functioning as originally intended or desired.

In the example you gave here the snapshot would have been moved to the current (orphaned) head and so building the snapshot off the parent txFrame would be slow because it would have to collect the changes from all ancestor frames.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants