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

feat: configurable nightly optimizations #1655

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: configurable nightly optimizations #1655

wants to merge 1 commit into from
+400 −296

Conversation

NikitaMasych
Copy link
Contributor

What?

Resolves #1638

Details

Nightly is required due to reliance on specialization feature, which allows keeping default implementation but leverage more optimized algorithms for specific fields. However, considering this is more desirable than critical, this PR elevates the requirement for nightly toolchain, allowing usage of the library in stable mode with, likely, worsened performance.

@NikitaMasych NikitaMasych force-pushed the feat/stable-via-cfg branch 2 times, most recently from d6f34e1 to 7e779ed Compare January 29, 2025 13:43
@NikitaMasych
Copy link
Contributor Author

@Nashtare @LindaGuiga would appreciate your thoughts on this and #1654, as soon as you have time, guys!

@NikitaMasych
Copy link
Contributor Author

Hi guys!

Allow me to additionally clarify the ideas behind this PR 🙂

Impact

If we merge it, plonky2 will be possible to compile using stable rust toolchain. For certain projects it will unlock new opportunities of integration (for example, at a time, I work on a project, where we have to maintain custom fork for this).

How

It requires quite a complex work to remove specialization feature, while preserving that code; we can however gate nightly-requiring specialization optimizations behind detecting toolchain version at compile-time.

Considerations

  • This approach is non-breaking.

  • It is possible to use another solution by adding nightly feature for highlighting impact.

  • It is fairly simple to revert this, if specialization feature will be removed.

P.S

Feel free to ask any questions!

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

@Nashtare Nashtare Awaiting requested review from Nashtare Nashtare is a code owner

@LindaGuiga LindaGuiga Awaiting requested review from LindaGuiga LindaGuiga is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
Zero EVM
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Nightly to Stable clarification
1 participant
@NikitaMasych