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

Add support for constant keys in our Mapping type #9

Merged
merged 1 commit into from
Sep 4, 2023

Conversation

simu
Copy link
Member

@simu simu commented Sep 1, 2023

We implement handling of constant keys (keys prefixed with KeyPrefix::Constant) directly in the Mapping implementation. Keys with other prefixes are inserted into the map unchanged.

To make the implementation more streamlined (and potentially faster), we switch the type of const_keys to be a HashSet instead of a Vec, which gives us O(1) removal of elements (for Mapping::remove()).

To insure integrity of the constant key tracking, We remove the map_as_mut() method on the Mapping implementation which would allow callers to break the constant key invariants that the other functions provide.

Some functions (insert, get_mut, and entry) will return an Error value when called for a key that's marked constant.

Follow-up to #8

Checklist

  • The PR has a meaningful title. The title will be used to auto generate the changelog
  • PR contains a single logical change (to build a better changelog).
  • Update the documentation.
  • Update tests.
  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency, internal
    as they show up in the changelog
  • Link this PR to related PRs or issues.

@simu simu added the enhancement New feature or request label Sep 1, 2023
@simu simu mentioned this pull request Sep 1, 2023
5 tasks
@simu simu force-pushed the feat/mapping-const-keys branch 2 times, most recently from e4c3432 to c8615c8 Compare September 1, 2023 11:20
Base automatically changed from feat/custom-parameters-types to main September 1, 2023 14:17
We implement handling of constant keys (keys prefixed with
`KeyPrefix::Constant`) directly in the Mapping implementation. Keys with
other prefixes are inserted into the map unchanged.

To make the implementation more streamlined (and potentially faster), we
switch the type of `const_keys` to be a `HashSet` instead of a `Vec`,
which gives us `O(1)` removal of elements (for `Mapping::remove()`).

To insure integrity of the constant key tracking, We remove the
`map_as_mut()` method on the Mapping implementation which would allow
callers to break the constant key invariants that the other functions
provide.

Some functions (`insert`, `get_mut`, and `entry`) will return an Error
value when called for a key that's marked constant.
@simu simu force-pushed the feat/mapping-const-keys branch from c8615c8 to ac8b8ac Compare September 1, 2023 14:17
@simu simu marked this pull request as ready for review September 1, 2023 14:17
@simu simu requested a review from a team September 1, 2023 14:17
@simu simu changed the title Implement support for constant keys in our Mapping type Add support for constant keys in our Mapping type Sep 1, 2023
@simu simu merged commit ad822c6 into main Sep 4, 2023
@simu simu deleted the feat/mapping-const-keys branch September 4, 2023 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants