datrs / random-access-storage

Abstract interface to implement random-access instances.
Apache License 2.0
13 stars 7 forks source link

Async IO access #2

Open yoshuawuyts opened 6 years ago

yoshuawuyts commented 6 years ago

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 commented 6 years ago

Working example: https://github.com/yoshuawuyts/spec-rust/blob/ee3bdc39b06bdf691f8904f0f8146917db90c36a/examples/futures.rs

yoshuawuyts commented 5 years ago

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 commented 5 years ago

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

yoshuawuyts commented 5 years ago

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.