serde-rs / serde

Serialization framework for Rust
https://serde.rs/
Apache License 2.0
8.81k stars 747 forks source link

AsyncDeserializer #2739

Open coder0xff opened 1 month ago

coder0xff commented 1 month ago

I'm considering extending serde to deserialize (and serialize) from a futures::io::AsyncRead (and AsyncWrite). Would it be enough to make an AsyncDeserializer, or is an AsyncVisitor also needed? I don't yet fully understand what the Visitor does, but since the method signatures take a value as input (at least for primitive types) it seems that reading from IO is already complete by the time Visitors are in play.

oli-obk commented 1 month ago

At that point you're probably better off just writing your own async_serde crate, as it will not really share anything with serde. Afaict you'll need your own Serialize and Deserialize traits, too, as everything will need to have async functions if it ever ends up invoking a Deserializer method that could perform an async operation (like reading from AsyncRead).

coder0xff commented 1 month ago

Afaict you'll need your own Serialize and Deserialize traits, too

You've pinpointed why I want to do this. I recently did exactly what you're suggesting. I made a bespoke async serialization library, derive macros and all, when I learned that serde couldn't accommodate async. When it works it works great, but it will never work for any library I don't control, because Rust doesn't permit implementing foreign traits on foreign types. I learned that, yes I can add hand-written serializers to the library where I define my trait, or fall back to blocking serde, but I shouldn't have to, and it flies in the face of separation of concerns.

I'll extend serde maintaining backward compatibility so any type already using #[derive(Serialize, Deserialize)] will generate AsyncSerialize, AsyncDeserialize, and AsyncVisitor, implementations. Then AsyncSerializer and AsyncDeserializer can be implemented by libraries like serde-rs/json so that we can all use async, just like we all enjoy today with blocking IO, thanks to serde being the one true serialization library.

P.S. I figured out that it would require an AsyncVisitor in fact, since Visitors uses Deserialize implementations in turn for composites.

oli-obk commented 1 month ago

Ah yes, the derive macros could be used to generate async impls together with the sync ones.

While it's all possible, I don't think we'll be taking on the extra maintenance burden of having two of everything (even if the async parts can be behind a cargo feature, so only ppl using it will pay the cost).

coder0xff commented 1 month ago

I'll choose to take the qualification in your second paragraph as a non-zero possibility. I wouldn't make two copies of everything of course, since there should be ample opportunity for code reuse. Primitive values are reusable as is. The composite types just need a few new trait methods in another impl. Based on my still limited perspective, the most challenging and most useful aspect of the crate is producing the data model that drives macro expansion, and that would be untouched. Generation is comparatively trivial.

I have no doubts that my pull request will be considered on its merits without preconceptions when it arrives.

oli-obk commented 1 month ago

have no doubts that my pull request will be considered on its merits without preconceptions when it arrives.

We don't have the review capacity for more than trivial features. At the current rate you're looking at a multi year wait time before you get the first maintainer review

coder0xff commented 1 month ago

I understand. Until then, [patch.crates-io] in cargo.toml will work.