Skip to content

Conversation

@orizi
Copy link
Collaborator

@orizi orizi commented Jan 23, 2026

Summary

Added serde derive attributes to ordered collection types and improved their serialization/deserialization implementation. This PR:

  1. Added transparent serde derive attributes to OrderedHashMap, OrderedHashSet, and SmallOrderedMap with appropriate bounds
  2. Reimplemented serialize_ordered_hashmap_vec and deserialize_ordered_hashmap_vec with more efficient code
  3. Removed redundant serde implementation for OrderedHashSet in favor of derive-based approach
  4. Made imports more explicit by using fully qualified paths instead of wildcard imports
  5. Made the salsa::Update implementation for OrderedHashMap more generic by allowing any BuildHasher implementation

Type of change

Please check one:

  • Performance improvement
  • Bug fix (fixes incorrect behavior)
  • New feature
  • Documentation change with concrete technical impact
  • Style, wording, formatting, or typo-only change

Why is this change needed?

The serialization/deserialization implementation for ordered collections was inefficient and required redundant code. By using serde derive attributes with transparent serialization, we can simplify the code while maintaining the same behavior. Additionally, making the salsa::Update implementation 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 OrderedHashMap implementation for salsa::Update was 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_vec and deserialize_ordered_hashmap_vec functions have been reimplemented more efficiently, and the salsa::Update implementation for OrderedHashMap now works with any BuildHasher implementation.

Additional context

This change also reorganizes imports to be more explicit and removes redundant code, improving overall code quality and maintainability.

@reviewable-StarkWare
Copy link

This change is Reviewable

@orizi orizi force-pushed the orizi/01-23-refactor_utils_simplified_serde_implementation_of_map_and_set_wrappers branch from ed9d971 to 19723f9 Compare January 25, 2026 11:51
Copy link
Collaborator

@TomerStarkware TomerStarkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@TomerStarkware reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @eytan-starkware).

Copy link
Contributor

@eytan-starkware eytan-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@eytan-starkware reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @orizi).

@orizi orizi added this pull request to the merge queue Jan 25, 2026
Merged via the queue into main with commit 58308ae Jan 25, 2026
55 checks passed
@orizi orizi deleted the orizi/01-23-refactor_utils_simplified_serde_implementation_of_map_and_set_wrappers branch January 26, 2026 21:15
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.

4 participants