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.

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

test(fil-proofs-tooling): warm up cache for window-post verify #1508

Merged
merged 2 commits into from
Sep 29, 2021
Merged

test(fil-proofs-tooling): warm up cache for window-post verify #1508

merged 2 commits into from
Sep 29, 2021

Conversation

dignifiedquire
Copy link
Contributor

No description provided.

vmx
vmx previously approved these changes Sep 29, 2021
View reviewed changes
Copy link
Contributor

@vmx vmx left a comment

Choose a reason for hiding this comment

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

LGTM, once it's rust fmt'ed correctly.

cryptonemo
cryptonemo approved these changes Sep 29, 2021
View reviewed changes
Copy link
Collaborator

@cryptonemo cryptonemo 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, although if we're doing this I'd measure and output the cold verify as well as the warm to get a better picture.

@dignifiedquire
Copy link
Contributor Author

Looks good, although if we're doing this I'd measure and output the cold verify as well as the warm to get a better picture.

the difference is 1.9s vs 6ms on my machine :D

@dignifiedquire dignifiedquire merged commit dad4e62 into master Sep 29, 2021
@dignifiedquire dignifiedquire deleted the test-warm-cache-verify branch September 29, 2021 11:44
@cryptonemo
Copy link
Collaborator

Looks good, although if we're doing this I'd measure and output the cold verify as well as the warm to get a better picture.

the difference is 1.9s vs 6ms on my machine :D

Yes. This is information that should be reported in the bench.

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

@cryptonemo cryptonemo cryptonemo approved these changes

@vmx vmx vmx 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.
3 participants
@dignifiedquire @cryptonemo @vmx