Adding serde support?

[lexoliu]

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

[jonhoo]

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!

[lexoliu]

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]

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.

[jonhoo]

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)

[lexoliu]

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?

[jonhoo]

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?

[lexoliu]

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.

[jonhoo]

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.

[lexoliu]

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

[jonhoo]

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.

[lexoliu]

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]

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

[jonhoo]

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?

[lexoliu]

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 ♥!

[jonhoo]

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!