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

Improve FieldElement struct construction #550

Closed
wants to merge 3 commits into from

Conversation

tcoratger
Copy link
Collaborator

Description

This pull request refactors the FieldElement structure to remove the unnecessary inner field, enhancing efficiency and minimizing struct size. Additionally, it implements the std::ops::Deref and std::ops::DerefMut traits to facilitate standard struct manipulations.

Changes

  • Removed the inner field from the FieldElement structure.
  • Implemented std::ops::Deref and std::ops::DerefMut traits for improved struct operations.

Impact

  • Improved efficiency by eliminating redundant struct fields.
  • Reduced struct size for better memory optimization.
  • Simplified struct manipulation with the addition of trait implementations.

@xJonathanLEI
Copy link
Owner

TBH I don't think the inner part makes any difference. I could be wrong but I think a single field struct should be exactly equivalent to a newtype struct in terms of performance. So changing that for better performance doesn't feel justified.

@tcoratger
Copy link
Collaborator Author

TBH I don't think the inner part makes any difference. I could be wrong but I think a single field struct should be exactly equivalent to a newtype struct in terms of performance. So changing that for better performance doesn't feel justified.

This one doesn't seem really relevant since we have this one in the pipeline: #562 which will delete all of this.

@xJonathanLEI
Copy link
Owner

Yeah that's true.

@xJonathanLEI
Copy link
Owner

starknet-ff was removed as part of #562. Closing this now as it's no longer relevant.

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.

2 participants