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
zkVM: change segment storage behavior #1483
Open
SchmErik
wants to merge
10
commits into
main
Choose a base branch
from
erik/seg-storage
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TODO: add support for client/server
From segments less than the limit, they will be stored in memory. For segments beyond the limit, we will store on disk...
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
converting to draft - need to write tests |
SchmErik
force-pushed
the
erik/seg-storage
branch
from
April 15, 2024 17:22
f4ce96c
to
1333190
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes the segment storage behavior. Before this change, the default behavior is to store segments on disk. Change the behavior so that the first 64 segments are stored in memory. Beyond this limit, we will store segments on disk. This limit is configurable through the
ExecutorEnv
.Here's my train of thought as I implemented this:
There’s two paths to using the zkVM: using
client/server
and using-F prove
. For both client/server mode and using the -F prove flags, I wanted the default behavior to be “save x amount of segments to memory and save any segment beyond this limit to files”. This means that we need to initialize the segment path in either case so that in case we go over the default limit, we can save them to thetempdir
.-F prove
mode:gives the user the ability to use their own callbacks. So my thought was that calling
Executor.run()
should trigger the aforementioned default behavior. If they want to customize the behavior and provide their own call backs, they should callExecutor.run_with_callback()
.client/server mode:
With this mode, we don't support adding custom segment callbacks. So the only behavior that’s supported is the default that I’ve described above. I’ve allowed a way for the developer to configure
ExecutorEnv
to state how many segments they want stored in RAM. If they want to store only to file, they can set this value toSome(0)
.Fixes: #1479