Skip to content

Proof generation memory optimisation #1637

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.

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

Merged
merged 8 commits into from
Apr 15, 2024

Conversation

Alenar
Copy link
Collaborator

@Alenar Alenar commented Apr 15, 2024

Content

This PR do some memory usage enhancement when computing a proof of Cardano transactions. Nothing ground breaking but this should reduce somehow the maximum memory needed for this operation.

  • It mostly reduce the number of copy of the full transactions set retrieved from database needed to compute the merkle tree used for proof generation.
  • It also separate the retrieval of the transactions to prove from the retrieval of the full transactions set (before we walked through the later to find the former).

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Issue(s)

Relates to #1629

Copy link

github-actions bot commented Apr 15, 2024

Test Results

    3 files  ±0     42 suites  ±0   9m 39s ⏱️ + 1m 12s
  915 tests +1    915 ✅ +1  0 💤 ±0  0 ❌ ±0 
1 009 runs  +1  1 009 ✅ +1  0 💤 ±0  0 ❌ ±0 

Results for commit 030936f. ± Comparison against base commit 5e22115.

♻️ This comment has been updated with latest results.

@Alenar Alenar temporarily deployed to testing-preview April 15, 2024 09:35 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet April 15, 2024 09:35 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-preview April 15, 2024 10:13 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet April 15, 2024 10:13 — with GitHub Actions Inactive
Copy link
Collaborator

@sfauvel sfauvel left a comment

Choose a reason for hiding this comment

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

Great job

Alenar added 6 commits April 15, 2024 14:26
The goal is to use it to retrieves the transactions to prove instead of
walking throught the entire transactions list.
By isolating the prover configuration using a closure.
Instead of finding them by walking the entire transactions list used to
create the merkle tree for the proof.
@Alenar Alenar force-pushed the djo/1629/proof_gen_memory_optimisation branch from 0fa371f to 65cb219 Compare April 15, 2024 12:26
It check that it won't retrieve data that were not asked and won't
retrieve data that doesn't exist.
+ Simplify test & add not exist case to `repository_create_and_get_transaction`
as well.
@Alenar Alenar force-pushed the djo/1629/proof_gen_memory_optimisation branch from 65cb219 to 7b252a7 Compare April 15, 2024 12:29
@Alenar Alenar temporarily deployed to testing-preview April 15, 2024 12:36 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet April 15, 2024 12:36 — with GitHub Actions Inactive
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM 👍

* Mithril-aggregator from `0.4.58` to `0.4.59`
* Mithril-common from `0.3.31` `0.3.32`
@Alenar Alenar temporarily deployed to testing-preview April 15, 2024 13:08 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet April 15, 2024 13:08 — with GitHub Actions Inactive
@Alenar Alenar merged commit 0e40969 into main Apr 15, 2024
41 checks passed
@Alenar Alenar deleted the djo/1629/proof_gen_memory_optimisation branch April 15, 2024 13:10
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.

3 participants