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

WIP: Add tokio async support #150

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hamidrezakp
Copy link

@hamidrezakp hamidrezakp commented Jun 10, 2023

This PR will add support for tokio by adding two traits AsyncBorshDeserialize and AsyncBorshSerialize and their respective derive macros to be used in places that we have async reader and writer.

Feature gate = "tokio"

Here is works that needs to be done:

  • Update documentation to address new trait and it's usage.
  • Fix tests and write them for async pair of traits.
  • Organize and rename new trait and modules to be more generic over async runtimes. (e.g. we may want to support async-std too)
  • Disable feature gate as a default and make it optional

Anyway, here is the driver code for current working version:

use borsh::tokio::{AsyncBorshDeserialize, AsyncBorshSerialize};
use borsh_derive::{AsyncBorshDeserialize, AsyncBorshSerialize};

#[derive(AsyncBorshDeserialize, AsyncBorshSerialize, PartialEq, Debug)]
struct Custom {
    a: u8,
    b: Vec<String>,
}

#[tokio::main]
async fn main() -> std::io::Result<()> {
    let value = Custom {
        a: 12,
        b: vec!["Hello".to_string(), "From Tokio".to_string()],
    };

    let serialized = value.try_to_vec().await?;

    let expected = Custom::try_from_slice(&serialized).await?;

    assert_eq!(value, expected);

    Ok(())
}

Cargo.toml:

[dependencies]
async-trait = "0.1.68"
borsh = { path = "../../borsh-rs/borsh" }
borsh-derive = { path = "../../borsh-rs/borsh-derive" }
tokio = { version = "1.28", features = ["macros", "rt", "rt-multi-thread"] }

@hamidrezakp hamidrezakp changed the title Add tokio async support WIP: Add tokio async support Jun 10, 2023
Copy link
Collaborator

@frol frol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This a massive! Thanks for the effort. It would be great if we can come up with some way to avoid code duplication as currently there seem to be 2 versions of code living side by side.

Comment on lines +52 to +57
/// 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)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgive my ignorance, but what is the point of using an async version of try_to_vec? It feels like a pointless method to me.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are totally right!
we don't need that. i will remove it.

@hamidrezakp
Copy link
Author

Thank you, yeah exactly, this is what i'm afraid of.

@frol
Copy link
Collaborator

frol commented Jun 16, 2023

It is going to be really hard to maintain and I fear that it will inevitably drift away, so I would keep it in PR for now to see if anyone will be able to come up with a better plan or maybe Rust async? syntax will mature.

@hamidrezakp
Copy link
Author

It is going to be really hard to maintain and I fear that it will inevitably drift away, so I would keep it in PR for now to see if anyone will be able to come up with a better plan or maybe Rust async? syntax will mature.

Good idea,
I don't like the current plan tbh.

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

Successfully merging this pull request may close these issues.

2 participants