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

Merged
merged 3 commits into from
Dec 19, 2024

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 2 times, most recently 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!

@Al-Kindi-0
Copy link
Collaborator Author

This is ready to be merged

@Al-Kindi-0 Al-Kindi-0 force-pushed the al-recursive-verifier-optimizations branch from 411b92c to b3204d6 Compare December 19, 2024 05:18
chore: update changelog

chore: address feedback
Signed-off-by: Al-Kindi-0 <[email protected]>
chore: changelog update

chore: add back file and update it

Revert "refactor: remove file"

This reverts commit 384a5af.
@Al-Kindi-0 Al-Kindi-0 force-pushed the al-recursive-verifier-optimizations branch from 8d1547c to d586616 Compare December 19, 2024 05:41
@Al-Kindi-0
Copy link
Collaborator Author

I have also included the changes from #1596 as it was the easiest way to get commits in that one signed

@plafer plafer merged commit 5944710 into next Dec 19, 2024
9 checks passed
@plafer plafer deleted the al-recursive-verifier-optimizations branch December 19, 2024 11:27
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