Adding Async support

[hamidrezakp]

Hi, I was wondering that if there is any plan for adding Async support to (de)serializer?

[frol]

@hamidrezakp There is nothing to make async, the whole process is CPU-bound, and fast. What is your use case?

[hamidrezakp]

I meant we support ‍reading from polllable sources like TCP/IP or sockets.
Right now we have support for Read types which can not be used with Tokio TCP Streams for example.

[frol]

I see, no there are no plans and no capacity to implement the support, but I am ready to review a PR if anyone will contribute the support with tests and documentation

[hamidrezakp]

Great, Probably i can work on it in the next week.
Thank you!

[iho]

@hamidrezakp hi, are you going to finish it?

[hamidrezakp]

Hey,
Sorry for not being active here.
I have tested the idea and it seems we need to change the BorshSerialize and BorshDeserialize trait and add some new bounds like Send + Sync to the reader and writer the same.

This change is breaking and it makes a bad user experience, because with this approach we would have two set of traits, blocking and non-blocking, which is quite bad and no one is gonna use it.

Unless you or somebody else have a better workaround or idea to add async support, i think i may create a crate like tokio-serde for borsh.

[iho]

Why do we need a new trait bound? Can you share your code, please? I am not very familiar with async Rust.

I guess we could just add an async crate feature and expose a new set of async functions something like from_reader_async, and to_writer_async.

@frol what do you think about this?

[hamidrezakp]

Here is the code i came up for serialization:

#[async_trait::async_trait]
pub trait BorshSerialize {
    async fn serialize<W: AsyncWrite + Send + Unpin>(&self, writer: &mut W) -> Result<()>;

    /// Serialize this instance into a vector of bytes.
    async fn try_to_vec(&self) -> Result<Vec<u8>> {
        let mut result = Vec::with_capacity(DEFAULT_SERIALIZER_CAPACITY);
        self.serialize(&mut result).await?;
        Ok(result)
    }

    #[inline]
    #[doc(hidden)]
    fn u8_slice(slice: &[Self]) -> Option<&[u8]>
    where
        Self: Sized,
    {
        let _ = slice;
        None
    }
}

#[async_trait::async_trait]
impl BorshSerialize for u8 {
    #[inline]
    async fn serialize<W: AsyncWrite + Send + Unpin>(&self, writer: &mut W) -> Result<()> {
        writer.write_all(core::slice::from_ref(self)).await
    }

    #[inline]
    fn u8_slice(slice: &[Self]) -> Option<&[u8]> {
        Some(slice)
    }
}

[hamidrezakp]

Send and Unpin is needed because in some types like Vec<T> we may not be able to serialize the whole thing in one go and have to store Future which contains the writer for later pooling.

[frol]

I was also thinking about introducing a separate trait for async methods and keep it under a feature-flag. This way it is not going to be a breaking change.

@iho It is a nice-to-have, and we currently don't have an urgency for it, so let's wait for @hamidrezakp to experiment with it. Just for context, there is an issue in serde where reader methods are much slower than from_slice which is another consideration point when we work on more features in that direction.

[hamidrezakp]

@iho It is a nice-to-have, and we currently don't have an urgency for it, so let's wait for @hamidrezakp to experiment with it.

Sure, i will continue writing the whole thing and see if it works and benchmark it.
@iho if you have any idea to make my code better, i highly appreciate it.
My code is in this repo.

Just for context, there is an issue in serde where reader methods are much slower than from_slice which is another consideration point when we work on more features in that direction.

Good point, that's why we always support both reading from a Reader and Slice or writing to Writer and Vec.
This way we have the benefit of both of them.

[hamidrezakp]

for reference, I'm following this method that Jon mentioned here

[frol]

@hamidrezakp I assigned this issue to you for now as I am on-boarding new contributors and I don't want them to work on this issue now. I don't expect you to deliver the PR, I just want to indicate that it is "taken" for now (let's say, 1 month). If someone in the future (1+ months from now) finds this issue, and wants to address it, feel free to reignite the conversation.

[hamidrezakp]

I have made a basic working version with derive macros 😄 .
It's not ready, just some PoC.
#150