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

Optimize computation of DEEP queries in recursive verifier #1594

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

Conversation

Al-Kindi-0
Copy link
Collaborator

Using better stack management, optimizes the computation of the DEEP queries by about 6%. This is less than the 10% we got in the signature verifier as the VM trace is wider and hence the impact of the optimizations is smaller.

@Al-Kindi-0 Al-Kindi-0 changed the title Optimize computation of DEEP queries Optimize computation of DEEP queries in recursive verifier Dec 6, 2024
Copy link
Contributor

@plafer plafer left a comment

Choose a reason for hiding this comment

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

LGTM! (based on the fact that tests pass - didn't thoroughly check the logic)

@Al-Kindi-0 Al-Kindi-0 force-pushed the al-recursive-verifier-optimizations branch from d6cde73 to 7ae500f Compare December 7, 2024 07:23
@Al-Kindi-0 Al-Kindi-0 force-pushed the al-recursive-verifier-optimizations branch from 7ae500f to bec35e8 Compare December 7, 2024 07:27
@Al-Kindi-0
Copy link
Collaborator Author

Thanks for the review!

Copy link
Contributor

@bobbinth bobbinth 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! Thank you! Not a super deep review, but I did leave some comments inline (most are very minor).

stdlib/asm/crypto/stark/deep_queries.masm Outdated Show resolved Hide resolved
stdlib/asm/crypto/stark/deep_queries.masm Outdated Show resolved Hide resolved
stdlib/asm/crypto/stark/deep_queries.masm Outdated Show resolved Hide resolved
stdlib/asm/crypto/stark/deep_queries.masm Show resolved Hide resolved
stdlib/asm/crypto/stark/deep_queries.masm Show resolved Hide resolved
stdlib/asm/crypto/stark/deep_queries.masm Show resolved Hide resolved
stdlib/asm/crypto/stark/deep_queries.masm Outdated Show resolved Hide resolved
Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

All looks good! Thank you!

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