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

Serde support #79

Open
aminya opened this issue Jun 17, 2023 · 6 comments
Open

Serde support #79

aminya opened this issue Jun 17, 2023 · 6 comments

Comments

@aminya
Copy link

aminya commented Jun 17, 2023

It would be great if we can add Serde support.

My personal use case is to use sharded slab in place of a vector which I can access without using locks, however, I will lose the ability to serialize/deserialize the data.

@hawkw
Copy link
Owner

hawkw commented Jun 17, 2023

In theory, I would be happy to accept a pull request adding the capability to serialize a Slab as though it were a vector.

However, this is somewhat complex: serializing the slab will require (immutably) iterating over the data stored in all shards of the slab. Because items may be concurrently inserted and removed by multiple shards, iterating over a slab currently requires exclusive (&mut) access to the slab (see: https://docs.rs/sharded-slab/0.1.4/sharded_slab/struct.Slab.html#method.unique_iter). This ensures that the thread iterating over the elements in the slab sees a consistent "point in time" snapshot of the slab's elements while it's iterating.

Therefore, this means that in order to serialize everything in the slab, including elements inserted by other threads, you would need to wrap the slab in a read-write lock (or something). You could still insert and remove elements using a read lock, so most slab operations would be uncontended. When it becomes necessary to serialize the slab's entire contents, a write lock is temporarily acquired so that the slab is not mutated during serialization. Then, the serializing thread releases the write lock, allowing normal concurrent access to the slab.

This is probably better off implemented in user code using a custom wrapper type, rather than in sharded_slab itself. Does that make sense?

@aminya
Copy link
Author

aminya commented Jun 19, 2023

That makes sense!

In my case, serialization and deserialization happen only on one thread when other threads are not doing any insertion. So, I can happily use a mutable reference without a lock.

@hawkw hawkw closed this as completed Sep 27, 2023
@aminya
Copy link
Author

aminya commented Sep 30, 2023

I checked the commit history, but I could not find serde support. Where was this added?

@hawkw
Copy link
Owner

hawkw commented Oct 1, 2023

I checked the commit history, but I could not find serde support. Where was this added?

I closed this issue because, as I stated in my previous comment, I think it makes more sense to serialize a slab's contents in user code. I took your reply to mean that you agreed and that my explanation of how you might serialize a slab's contents in your own code was a sufficient resolution to this issue.

If that's not the case, I'm happy to reopen this!

@aminya
Copy link
Author

aminya commented Oct 1, 2023

Sorry, I missed the last part of your comment.

Wrapping the type just for the sake of Serde doesn't help. All the usage of this type needs to be replaced just to get Serde support. This means patching any dependency that uses this type.

Almost all data structures provide Serde through a feature. I do not see why this package wants to be different.

@hawkw
Copy link
Owner

hawkw commented Oct 2, 2023

Again, because it is not possible to iterate over a Slab through an &-reference, the serde::Serialize implementation would have to be for a &mut Slab<T>, rather than for Slab<T>. This might be difficult to use with deriving Serialize on a struct that owns a Slab.

Is that acceptable to you? Because I'm open to adding a Serialize implementation for &mut Slab<T> where T: Serialize, but I'm not sure if it would be as useful as implementing Serialize for Slab<T>>. This is why I suggested implementing Serialize for a newtype wrapping a RwLock<Slab<T>>, instead. Unfortunately, we can't provide a Serialize implementation for RwLock<Slab<T>>, since it's a foreign type to this crate.

Another question about a potential Serialize implementation: would you want the slab to serialize itself as a map of slab keys to values, or as a list of values? We could do either, but because this type is not really an ordered collection like a Vec, serializing it as a map might make more sense, IMO.

@hawkw hawkw reopened this Oct 2, 2023
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

No branches or pull requests

2 participants