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

Change get_or_try_insert_with to return a concrete error type rather than a trait object #23

Merged
merged 4 commits into from
Aug 9, 2021

Conversation

tatsuya6502
Copy link
Member

@tatsuya6502 tatsuya6502 commented Aug 3, 2021

Change get_or_try_insert_with of sync::{Cache, SegmentedCache} and future::Cache to return a concrete error type rather than a trait object.

Before (sync::Cache)

pub fn get_or_try_insert_with<F>(&self, key: K, init: F) ->
    Result<V, Arc<Box<dyn Error + Send + Sync + 'static>>>
where
    F: FnOnce() -> Result<V, Box<dyn Error + Send + Sync + 'static>>

After

pub fn get_or_try_insert_with<F, E>(&self, key: K, init: F) -> Result<V, Arc<E>>
where
    F: FnOnce() -> Result<V, E>,
    E: Error + Send + Sync + 'static,

Behavior Change

There is a subtle change in the behavior. get_or_try_insert_with used to guarantee that the closure F to evaluate once even if multiple threads call it at the same time on the same cache key.

Now this is not always true:

  • Only one of the closures must be evaluated if the closures in all concurrent calls on the same cache key return the same concrete error type.
  • One closure of each error type should be evaluated if the closures in concurrent calls on the same cache key does not return the same concrete error type.

…ocation

Replace `Arc<Box<dyn ..>>` in the return type of `get_or_try_insert_with`
with `Arc<dyn ..>`.
@tatsuya6502 tatsuya6502 changed the title Change the signature of get_or_try_insert_with to avoid redundant allocation Change the return type of get_or_try_insert_with to avoid redundant heap allocation Aug 4, 2021
…r than

a trait object

Now the return type is `Result<V, Arc<E>>` where `E: Error + Send + Sync + 'static`.
@tatsuya6502 tatsuya6502 changed the title Change the return type of get_or_try_insert_with to avoid redundant heap allocation Change get_or_try_insert_with to return a concrete error type rather than a trait object Aug 8, 2021
@tatsuya6502 tatsuya6502 self-assigned this Aug 9, 2021
@tatsuya6502 tatsuya6502 marked this pull request as ready for review August 9, 2021 10:05
Copy link
Member Author

@tatsuya6502 tatsuya6502 left a comment

Choose a reason for hiding this comment

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

Merging.

@tatsuya6502 tatsuya6502 merged commit 76b06af into master Aug 9, 2021
@tatsuya6502 tatsuya6502 deleted the get-or-try-insert-return-type branch August 9, 2021 10:06
@tatsuya6502 tatsuya6502 added this to the v0.6.0 milestone Aug 28, 2021
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.

1 participant