sharded-slab

sharded-slab copied to clipboard

Currently Slab::get returns the Guard type while Pool::get returns a PoolGuard. But looking at the contents of these two types, we can see that they carry the same information. It would be a great win for users if we are able to unify these types.

The reason for the difference for the two types comes to their Drop implementations:

• Here is the Drop impl for Guard

https://github.com/hawkw/sharded-slab/blob/ffa5c7a0eeb576c159b86a700c00bb1adf2a812c/src/lib.rs#L499-L511

• And for PoolGuard

https://github.com/hawkw/sharded-slab/blob/ffa5c7a0eeb576c159b86a700c00bb1adf2a812c/src/pool/mod.rs#L232-L249

Both these impls, while calling different methods, are calling the "mark for release" chain. The final guard being dropped is responsible for actually clearing the storage. The calls eventually lead to Slot::try_remove_value for Guards drop impl and Slot::try_clear_storage for PoolGuards drop impl. The sole difference comes down to the mutator parameter being passed to the release_with method for the Slab and Pool impls.

The mutator closure in Pools case calls the Clear::clear on T, therefore we need the T: Clear bound to be true. In the Slabs case we need the type to be an Option<T> in order to call Option::take.

If we can find a way to scoping those two pieces of functionality to impl block bounded by the specific generic types we will be able to expose a unified guard type. Not only that but we will be able to share a lot more code throughout the crate as a lot of the functions in this particular code path are repetitive.

In the Slabs case we need the type to be an Option<T> in order to call Option::take.

Since we're not actually returning the taken item in this code path, can't we just use the Clear impl for Option? https://github.com/hawkw/sharded-slab/blob/ffa5c7a0eeb576c159b86a700c00bb1adf2a812c/src/clear.rs#L15-L19 This should have equivalent behavior to the current implementation. I think it should be fine to have one code path that's used by both guards. It's only Slab::take that requires special-casing.

@bIgBV Are you interested in working on this? I'd like this to be addressed before we publish a release with the object pool API. If you're busy, I'm happy to take care of it so we can move closer to a release.

@hawkw yeah I'd like to work on this. And the Clear impl for Option looks like a great way to move forward with this.

Okay, great. I think this is the last release blocker for the pool.

Looks like this probably isn't going to be possible with the current state of things (see https://github.com/hawkw/sharded-slab/pull/33#issuecomment-607039927 and https://github.com/hawkw/sharded-slab/pull/33#issuecomment-607352267) so I'm not going to block the release on it.