evmap icon indicating copy to clipboard operation
evmap copied to clipboard

Adding serde support?

Open lexoliu opened this issue 5 years ago • 23 comments

The crate dependency hashbrown already provides serde support, but why doesn't evmp provide support for serde?

lexoliu avatar Aug 13 '20 13:08 lexoliu

It's just not entirely clear how it'd be provided, since you'd be blocking writes while serializing. There are also some questions around refreshing. I think now that we have MapReadRef, that's at least an obvious thing to implement Serialize for, and it should be decently easy using this guide. Deserialize is trickier. I guess we'd deserialize into a tuple of a read and write handle, though we'd probably need a newtype wrapping that tuple to be allowed to implement the trait.

Happy to take a look at a PR if you have some time!

jonhoo avatar Aug 13 '20 15:08 jonhoo

Thanks for the reply, I just want to save the Map to a file and recover it. I would like to provide an API to destroy Reader and Writer to get the hashbrown object for serialization

lexoliu avatar Aug 14 '20 05:08 lexoliu

I tried to implement this one structure.

{
    reader:evmap::ReadHandle<Vec<u8>,Key>,
    writer:evmap::WriteHandle<Vec<u8>,Key>,
    read_ref:evmap::MapReadRef<'a,Vec<u8>,Key>,
}

I think we should follow the implementation in std and return an object. Then get the Reader and Writer from the object.

lexoliu avatar Aug 14 '20 07:08 lexoliu

Giving a way to destroy the map to take ownership of the underlying map will probably be difficult in practice. Is there a reason why implementing Serialize for MapReadRef wouldn't be sufficient for you? Coupled with an implementation of Deserialize for (WriteHandle, ReadHandle)

jonhoo avatar Aug 14 '20 12:08 jonhoo

But that seems like a lot of work, so why not try building a map object and getting the Reader and Sender from it, so we only need to implement Deserialize once for map?

lexoliu avatar Aug 14 '20 12:08 lexoliu

It shouldn't be a lot of work — especially compared to finding a way to safely get an owned version of the underlying map. Implementing Serialize for MapReadRef should be fairly trivial. Deserialize should also be pretty easy, since you can just deserialize into a hashbrown::HashMap, and then use that to create the (WriteHandle, ReadHandle).

I'm not sure what you mean by a map object?

jonhoo avatar Aug 14 '20 12:08 jonhoo

In other words, we should allow the reader and sender to be retrieved from evmap::MapReadRef instead of evmap::new. This way we can serialize MapReadRef directly.

lexoliu avatar Aug 14 '20 12:08 lexoliu

I'm not sure what you mean by "reader" and "sender"?

You can implement the Serialize trait for MapReadRef pretty easily by just following this guide.

jonhoo avatar Aug 14 '20 12:08 jonhoo

Sorry, I may not have been clear enough. I use Reader for evmap::ReadHandle, Sender for evmap::WriteHandle, and object for evmap::MapReadRef. Sender, but delayed acquisition

lexoliu avatar Aug 14 '20 12:08 lexoliu

So, you cannot get a WriteHandle from a MapReadRef or from a ReadHandle — that just will never be safe. You can get a ReadHandle from a WriteHandle without too much trouble. You can also trivially get a MapReadRef from a ReadHandle.

I don't know what you mean by "delayed acquisition", nor why an "object" idea is helpful?

Implementing Serialize for MapReadRef means that you can serialize without having to hold the WriteHandle, which is a huge advantage. Deserialize could, I suppose, just generate a WriteHandle, though it seems more natural for it to produce a (ReadHandle, WriteHandle) the same way new does.

jonhoo avatar Aug 14 '20 13:08 jonhoo

Yes, I want to get readhandle and mapreadref from writehandle. And generate only writehandle when new! However, in order to prevent writehandle from being abused, it should be packaged as "MapObject", and then get writehandle, readhandle or mapreadref from "MapObject".

lexoliu avatar Aug 14 '20 13:08 lexoliu

Next, we implement serde only for MapObject and lock the HashMap during serialize

lexoliu avatar Aug 14 '20 13:08 lexoliu

WriteHandle already dereferences to ReadHandle, so getting a ReadHandle is trivial. You can then clone that to get a new one. And once you have a ReadHandle, you can get a MapReadRef by calling ReadHandle::read.

I don't understand what you mean by "prevent WriteHandle from being abused"? What kind of abuse are you thinking about? And why would wrapping it with a ReadHandle (and a MapReadRef?) make a difference?

I also still do not understand why implementing Serialize just for MapReadRef isn't the optimal choice? MapReadRef is already pinning the write handle. It is true that you're not guaranteed to see the latest writes, but that's an application decision that we shouldn't enforce. If the application wishes the serialization to have the latest state, it should simply call refresh before serializing.

For Deserialize, I'm fine with it just being implemented for WriteHandle too. I don't see the value in a MapObject type?

jonhoo avatar Aug 14 '20 13:08 jonhoo

I've read the documentation carefully and I seem to have found the method. I just need to implement serde to WriteHandle and I'm good to go. Thank you, I will submit this code to you when I finish testing ♥!

lexoliu avatar Aug 14 '20 13:08 lexoliu

Again, we should implement serde::Serialize for MapReadRef, not for WriteHandle.

Deserialize on the other hand should be implemented just for WriteHandle. We could also implement it for (ReadHandle, WriteHandle) for convenience.

Happy to look at what you come up with!

jonhoo avatar Aug 14 '20 13:08 jonhoo

How to convert evmap::WriteHandle to evmap:: readhandle?I'm implementing serde::Serialize for evmap:: writehandle

lexoliu avatar Aug 14 '20 13:08 lexoliu

image https://docs.rs/evmap/11.0.0-alpha.1/evmap/struct.WriteHandle.html#method.read I'm confused about this document

lexoliu avatar Aug 14 '20 13:08 lexoliu

You should not implement Serialize for WriteHandle, you should implement it for MapReadRef :)

jonhoo avatar Aug 14 '20 14:08 jonhoo

If you want to save HashMap to enum and implement serde for enum.How can this be handled?I don't know how to store HashMap right now. Should I save WriteHandle, MapReadRef. or build a tuple?

lexoliu avatar Aug 14 '20 14:08 lexoliu

Why is an enum needed? You should just follow this guide to implement serde::Serialize directly for MapReadRef.

jonhoo avatar Aug 14 '20 14:08 jonhoo

Hello,Jonhoo.I had some problems and I couldn't understand why evmap provided evmap::Values .Is it because evmap is ultimately consistent?That is, we may have to wait for them to execute WriteHandle:: refresh during serialization operations?I look forward to your help

lexoliu avatar Aug 15 '20 10:08 lexoliu

Does `multiset' mean multiple identical copies? If so.We just need one value!

lexoliu avatar Aug 15 '20 10:08 lexoliu

evmap::Values is there because there may be many (different) values for a given key in evmap, and we use different backing storage depending on how many there are. You'll probably want to implement Serialize and Deserialize for evmap::Values simply using a sequence (which I think is also explained in the same linked guide).

jonhoo avatar Aug 15 '20 16:08 jonhoo