Skip to content

Improve snapshot creation performance #5516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
lehins merged 6 commits into from
Jan 24, 2026
Merged

Improve snapshot creation performance #5516

lehins merged 6 commits into from
Jan 24, 2026
+1,076 −582

Conversation

@lehins
Copy link
Collaborator

@lehins lehins commented Jan 9, 2026 •
edited
Loading

Description

There was some duplicate computation as well as redundant retention of unnecessary data in the SnapShots. This commit introdcues a better approach that eliminates the need for the above overhead, while temporarely keeping the old appraoch with some assertions that will be further used to confirm accuracy on mainnet data. Keeping old approach should not incur any overhead, since it was made lazy and is only forced during testing with assertions. The plan is to remove this old approach before the next release, however there is no danger to go to production with these follow up changes, due to lazy evaluation of an old approach.

See my comments for more details on individual optimizations

Checklist

  • Commits in meaningful sequence and with useful messages.
  • Tests added or updated when needed.
  • CHANGELOG.md files updated for packages with externally visible changes.
    NOTE: New section is never added with the code changes. (See RELEASING.md).
  • Versions updated in .cabal and CHANGELOG.md files when necessary, according to the
    versioning process.
  • Version bounds in .cabal files updated when necessary.
    NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
  • Code formatted (use scripts/fourmolize.sh).
  • Cabal files formatted (use scripts/cabal-format.sh).
  • CDDL files are up to date (use scripts/gen-cddl.sh)
  • hie.yaml updated (use scripts/gen-hie.sh).
  • Self-reviewed the diff.

@lehins lehins changed the title Lehins/improve snapshot performance Improve snapshot creation performance Jan 9, 2026
@lehins lehins force-pushed the lehins/improve-snapshot-performance branch 2 times, most recently from 8698376 to 7fb6177 Compare January 9, 2026 22:55
lehins
lehins commented Jan 12, 2026
View reviewed changes
@lehins lehins force-pushed the lehins/improve-snapshot-performance branch from 7fb6177 to a4fd4ae Compare January 14, 2026 03:11
lehins
lehins commented Jan 14, 2026
View reviewed changes
@lehins lehins marked this pull request as ready for review January 14, 2026 16:16
@lehins lehins requested a review from a team as a code owner January 14, 2026 16:16
lehins
lehins commented Jan 15, 2026
View reviewed changes
lehins added a commit that referenced this pull request Jan 15, 2026
@lehins
Improve perfomance and reduce overhead of SnapShot creation:
2f2aace 
There was some duplicate computation as well as redundant retention of
unnecessary data in the `SnapShots`. This commit introdcues a better
approach that eliminates the need for the above overhead, while
temporarely keeping the old appraoch with some assertions that will be
further used to confirm accuracy on mainnet data. Keeping old approach
should not incur any overhead, since it was made lazy and is only forced
during testing with assertions. The plan is to remove this old approach
before the next release, however there is no danger to go to production
with these follow up changes, due to lazy evaluation of an old approach.

See my comments on #5516 for more details on individual optimizations.
@lehins lehins force-pushed the lehins/improve-snapshot-performance branch from a4fd4ae to 2f2aace Compare January 15, 2026 05:03
lehins added a commit that referenced this pull request Jan 15, 2026
@lehins
Improve perfomance and reduce overhead of SnapShot creation:
cf6e7f1 
There was some duplicate computation as well as redundant retention of
unnecessary data in the `SnapShots`. This commit introdcues a better
approach that eliminates the need for the above overhead, while
temporarely keeping the old appraoch with some assertions that will be
further used to confirm accuracy on mainnet data. Keeping old approach
should not incur any overhead, since it was made lazy and is only forced
during testing with assertions. The plan is to remove this old approach
before the next release, however there is no danger to go to production
with these follow up changes, due to lazy evaluation of an old approach.

See my comments on #5516 for more details on individual optimizations.
@lehins lehins force-pushed the lehins/improve-snapshot-performance branch from 2f2aace to cf6e7f1 Compare January 15, 2026 05:03
lehins
lehins commented Jan 16, 2026
View reviewed changes
teodanciu
teodanciu reviewed Jan 16, 2026
View reviewed changes
Copy link
Contributor

@teodanciu teodanciu left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed mostly the new computations from core, would like to look closer at the rules changes, but I need a break, will do it tomorrow!

lehins added a commit that referenced this pull request Jan 17, 2026
@lehins
Improve perfomance and reduce overhead of SnapShot creation:
d41fa15 
There was some duplicate computation as well as redundant retention of
unnecessary data in the `SnapShots`. This commit introdcues a better
approach that eliminates the need for the above overhead, while
temporarely keeping the old appraoch with some assertions that will be
further used to confirm accuracy on mainnet data. Keeping old approach
should not incur any overhead, since it was made lazy and is only forced
during testing with assertions. The plan is to remove this old approach
before the next release, however there is no danger to go to production
with these follow up changes, due to lazy evaluation of an old approach.

See my comments on #5516 for more details on individual optimizations.
@lehins lehins force-pushed the lehins/improve-snapshot-performance branch from cf6e7f1 to d41fa15 Compare January 17, 2026 20:03
lehins added a commit that referenced this pull request Jan 17, 2026
@lehins
Improve perfomance and reduce overhead of SnapShot creation:
a206f95 
There was some duplicate computation as well as redundant retention of
unnecessary data in the `SnapShots`. This commit introdcues a better
approach that eliminates the need for the above overhead, while
temporarely keeping the old appraoch with some assertions that will be
further used to confirm accuracy on mainnet data. Keeping old approach
should not incur any overhead, since it was made lazy and is only forced
during testing with assertions. The plan is to remove this old approach
before the next release, however there is no danger to go to production
with these follow up changes, due to lazy evaluation of an old approach.

See my comments on #5516 for more details on individual optimizations.
@lehins lehins force-pushed the lehins/improve-snapshot-performance branch from d41fa15 to a206f95 Compare January 17, 2026 20:04
teodanciu
teodanciu approved these changes Jan 20, 2026
View reviewed changes
Copy link
Contributor

@teodanciu teodanciu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Rewards and snapshots are such unknown territories, but thanks to this PR I started to understand they work.
The calculations using the new structure seem equivalent to the old ones to me, and how awesome that we're now avoiding redundant computations!

Soupstraw
Soupstraw reviewed Jan 21, 2026
View reviewed changes
lehins added a commit that referenced this pull request Jan 23, 2026
@lehins
Improve perfomance and reduce overhead of SnapShot creation:
6c73fcf 
There was some duplicate computation as well as redundant retention of
unnecessary data in the `SnapShots`. This commit introdcues a better
approach that eliminates the need for the above overhead, while
temporarely keeping the old appraoch with some assertions that will be
further used to confirm accuracy on mainnet data. Keeping old approach
should not incur any overhead, since it was made lazy and is only forced
during testing with assertions. The plan is to remove this old approach
before the next release, however there is no danger to go to production
with these follow up changes, due to lazy evaluation of an old approach.

See my comments on #5516 for more details on individual optimizations.
@lehins lehins force-pushed the lehins/improve-snapshot-performance branch from a206f95 to dad6b50 Compare January 23, 2026 06:31
@lehins lehins enabled auto-merge January 23, 2026 06:32
@lehins lehins force-pushed the lehins/improve-snapshot-performance branch from dad6b50 to 513ef70 Compare January 23, 2026 06:33
lehins added a commit that referenced this pull request Jan 23, 2026
@lehins
Improve perfomance and reduce overhead of SnapShot creation:
853df55 
There was some duplicate computation as well as redundant retention of
unnecessary data in the `SnapShots`. This commit introdcues a better
approach that eliminates the need for the above overhead, while
temporarely keeping the old appraoch with some assertions that will be
further used to confirm accuracy on mainnet data. Keeping old approach
should not incur any overhead, since it was made lazy and is only forced
during testing with assertions. The plan is to remove this old approach
before the next release, however there is no danger to go to production
with these follow up changes, due to lazy evaluation of an old approach.

See my comments on #5516 for more details on individual optimizations.
@lehins lehins force-pushed the lehins/improve-snapshot-performance branch from 513ef70 to e8212f4 Compare January 23, 2026 20:33
lehins and others added 4 commits January 23, 2026 14:18
@lehins
Add elemAt to vector-map
67600f1
@lehins
Stop re-exporting QuickCheck's NonZero
4ff395e 
Because it conflcicts with our internal type.
@lehins
Improve type clarity
6eeca1b
@lehins
Improve perfomance and reduce overhead of SnapShot creation:
fb75106 
There was some duplicate computation as well as redundant retention of
unnecessary data in the `SnapShots`. This commit introdcues a better
approach that eliminates the need for the above overhead, while
temporarely keeping the old appraoch with some assertions that will be
further used to confirm accuracy on mainnet data. Keeping old approach
should not incur any overhead, since it was made lazy and is only forced
during testing with assertions. The plan is to remove this old approach
before the next release, however there is no danger to go to production
with these follow up changes, due to lazy evaluation of an old approach.

See my comments on #5516 for more details on individual optimizations.
@lehins lehins force-pushed the lehins/improve-snapshot-performance branch from e8212f4 to e612ea2 Compare January 23, 2026 21:18
@lehins lehins force-pushed the lehins/improve-snapshot-performance branch from e612ea2 to 22b039e Compare January 23, 2026 23:26
@lehins lehins merged commit e723971 into master Jan 24, 2026
118 checks passed
@lehins lehins deleted the lehins/improve-snapshot-performance branch January 24, 2026 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teodanciu teodanciu teodanciu approved these changes

@Soupstraw Soupstraw Soupstraw left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lehins @teodanciu @Soupstraw