-
Notifications
You must be signed in to change notification settings - Fork 738
refactor(utils): Simplified serde implementation of map and set wrappers. #9519
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
refactor(utils): Simplified serde implementation of map and set wrappers. #9519
Conversation
d3485d7 to
ed9d971
Compare
ed9d971 to
19723f9
Compare
TomerStarkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TomerStarkware reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @eytan-starkware).
eytan-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eytan-starkware reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @orizi).

Summary
Added
serdederive attributes to ordered collection types and improved their serialization/deserialization implementation. This PR:serdederive attributes toOrderedHashMap,OrderedHashSet, andSmallOrderedMapwith appropriate boundsserialize_ordered_hashmap_vecanddeserialize_ordered_hashmap_vecwith more efficient codeOrderedHashSetin favor of derive-based approachsalsa::Updateimplementation forOrderedHashMapmore generic by allowing anyBuildHasherimplementationType of change
Please check one:
Why is this change needed?
The serialization/deserialization implementation for ordered collections was inefficient and required redundant code. By using
serdederive attributes with transparent serialization, we can simplify the code while maintaining the same behavior. Additionally, making thesalsa::Updateimplementation more generic allows for more flexibility when using custom hash builders.What was the behavior or documentation before?
Previously, ordered collections had custom serialization/deserialization implementations that were less efficient and required more code. The
OrderedHashMapimplementation forsalsa::Updatewas restricted to the default hash builder, limiting flexibility.What is the behavior or documentation after?
Now, ordered collections use transparent serialization with appropriate bounds, making the code more maintainable while preserving the same behavior. The
serialize_ordered_hashmap_vecanddeserialize_ordered_hashmap_vecfunctions have been reimplemented more efficiently, and thesalsa::Updateimplementation forOrderedHashMapnow works with anyBuildHasherimplementation.Additional context
This change also reorganizes imports to be more explicit and removes redundant code, improving overall code quality and maintainability.