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

Async IO access #2

Open
yoshuawuyts opened this issue Jun 13, 2018 · 4 comments
Open

Async IO access #2

yoshuawuyts opened this issue Jun 13, 2018 · 4 comments

Comments

@yoshuawuyts
Copy link
Contributor

This is the closest we've gotten so far: https://gist.github.com/rust-play/cf5eb5d7e255957b69dcaf3b9297d9f7

It'd be good to figure out a way in which we can make this work. Probably depends on the new version of https://github.com/alexcrichton/futures-await landing, which will allow us to pass borrowed values into API.

@yoshuawuyts
Copy link
Contributor Author

@yoshuawuyts
Copy link
Contributor Author

Thinking we could structure this as:

// provided methods: read, write
trait RandomAccess: AsyncRead + AsyncWrite + Drop {
  type Error;

  fn open() -> Result<Self, Self::Error>

  async fn remove(&mut self, offset: u64, length: u64) -> Result<(), Self::Error>
}

We could implement the sync_* versions of methods by creating a one-off executor. Not the most efficient, but it would get the job done for compat. We could also choose to drop it entirely, and only focus on the async versions of methods (which I think would be reasonable!)

@yoshuawuyts
Copy link
Contributor Author

yoshuawuyts commented Oct 30, 2018

Possibly we should also require std::io::Seek to be implemented.

@yoshuawuyts
Copy link
Contributor Author

Okay, from talking to @substack, and considering #13, we could make this trait not require any methods, and make it require trait RandomAccessStorage: Read + Write + Seek.

We could then provide range variants of stdlib methods, while preserving the ability to call stdlib methods directly. The proposed shape of the trait would be (simplified):

trait RandomAccessStorage: Read + Write + Seek {
  fn read_range(range: Range, buf: &mut [u8]) -> Result<(), Error>;
  fn write_offset(offset: usize, buf: &mut [u8]) -> Result<(), Error>;
  fn remove_range(range: Range) -> Result<(), Error>;
}

Async

Once we're comfortable enough with futures we could add the AsyncRead and AsyncWrite traits too:

trait RandomAccessStorage: Read + Write + Seek {
  fn read_range(range: Range, buf: &mut [u8]) -> Result<(), Error>;
  fn write_offset(offset: usize, buf: &mut [u8]) -> Result<(), Error>;
  fn remove_range(range: Range) -> Result<(), Error>;
  async fn poll_read_range(range: Range, buf: &mut [u8]) -> Result<(), Error>;
  async fn poll_write_offset(offset: usize, buf: &mut [u8]) -> Result<(), Error>;
  async fn poll_remove_range(range: Range) -> Result<(), Error>;
}

We could experiment with creating a first version of our backends that does not rely on async IO libraries directly, and instead calls out to the regular .read() and .write() methods. And when the time is right we could switch it out to a better implementation.

Ergonomics

To comply with #13, we'd have to remove the core implementation from allocating. But we could add convenience methods to do the allocations for us. However considering there's also async methods in play, that matrix might grow rather large, and the names would be long. Therefor I think it might make most sense to drop them entirely.

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

No branches or pull requests

1 participant