hawkw / sharded-slab

a lock-free concurrent slab (experimental)
MIT License
269 stars 17 forks source link

feat(Pool): unify guard types #33

Closed bIgBV closed 4 years ago

bIgBV commented 4 years ago

Begin unification of guard types by using common code paths for clearing an entry.

Closes #30

bIgBV commented 4 years ago

Hitting a problem with how the the Guard type is defined. Currently Guard is generic over T, but the compiler isn't happy about constructing a guard where the shard type is Option<T>.

https://github.com/hawkw/sharded-slab/blob/c66204d0d66fd22f743282b7bd31df34db2abd98/src/lib.rs#L423-L430

It errors out with the following error message:

error[E0308]: mismatched types
   --> src/lib.rs:430:29
    |
240 | impl<T, C: cfg::Config> Slab<T, C> {
    |      - this type parameter
...
430 |         Some(Guard { inner, shard, key })
    |                             ^^^^^ expected type parameter `T`, found enum `std::option::Option`
    |
    = note: expected reference `&shard::Shard<T, C>`
               found reference `&shard::Shard<std::option::Option<T>, C>`
    = help: type parameters must be constrained to match other types
    = note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters

Plus there are errors with the Drop impl due to the trait bounds not being met.

I might have an idea of how to fix it though. It should be possible to generic from Option to in shard.

hawkw commented 4 years ago

Hitting a problem with how the the Guard type is defined. Currently Guard is generic over T, but the compiler isn't happy about constructing a guard where the shard type is Option<T>.

Hmm. Ah, yeah, that's a roadblock I didn't think of. My guess is this may not really work the way we want it to. Maybe we should just continue having a separate pool guard type.

But, we can probably still have both guards use mark_clear rather than remove, which reduces some duplication. As that's an internal change, it wouldn't be a release blocker.

It would be nice to figure out whether or not we'll have to have two guard types or not, to stop holding up the release. But, I guess, this is 0.0.x, so we can ship now and break it if we figure out a better solution later.

bIgBV commented 4 years ago

Closing this PR for now. I'll create a new one which includes the changes in this PR to unify the clear code path.