Skip to content
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

Update documentation about read_vec function in SP1 gitbook #1840

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

sashafrolov
Copy link

Motivation

I am working on an SP1 application that reads in ~200kB of input data from input. I was able to improve prover performance by a factor of 3 by replacing read with read_vec. I had to figure this out by reading code, so I want to document this big performance difference in the relevant documentation.

Solution

I am just documenting this for now, but when Rust adds specialization for functions, read<Vec> should just default to a non-serializing version by default. (specialization is a nightly feature right now)

PR Checklist

  • [ N/A ] Added Tests
  • [ x ] Added Documentation
  • [ N/A ] Breaking changes

@nhtyy
Copy link
Collaborator

nhtyy commented Dec 6, 2024

Thanks this is a good callout. We have a new documentation framework coming soon, so Im not gonna merge this PR yet but will make sure its included next release.

@nhtyy nhtyy added the documentation Improvements or additions to documentation label Dec 6, 2024
@sashafrolov
Copy link
Author

Dumb question: I have almost entirely worked with Phabricator as a code review system before. Is there something I'm supposed to do to get this PR merged? Or is the ball in somebody else's court now? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants