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

Wasm futures #72

Closed
wants to merge 3 commits into from
Closed

Conversation

feefladder
Copy link
Contributor

depends on #71
Make the futures of the trait wasm-compatible

This uses async_trait crate, but the function signatures remain largely untouched...
change is from

fn get_bytes(&self, range: Range<u64>) -> BoxFuture<'_, AsyncTiffResult<Bytes>>
// which afaik equals
fn get_bytes<'a>(&'a self, range: Range<u64>) -> BoxFuture<'a, AsyncTiffResult<Bytes>>

to

fn get_bytes<'life0, 'async_trait>(&'life0 self, range: Range<u64>) -> BoxFuture<'async_trait, AsyncTiffResult<Bytes>>
where:
  'life0: 'async_trait,
  Self: 'async_trait

So I think it's not that big of a change

This was referenced Mar 21, 2025
@kylebarron
Copy link
Member

Can you explain how the BoxFuture is not already wasm-compatible? The ObjectStore trait uses BoxFuture, and it is wasm32 compatible.

@feefladder
Copy link
Contributor Author

feefladder commented Mar 21, 2025

So there's wasm and wasm... In browser, where everything becomes a Javascript object, futures are ?Send

Also, reqwest responses are not Send. It seems like it boils down to the fetch api returning a js_sys::Promise, which is !Send

With some hacks, it is possible though: https://users.rust-lang.org/t/is-it-possible-to-take-a-jsfuture-as-send/97774

but yeah, I think maybe specifying it further as suggested here:

#[cfg_attr(all(target_family = "wasm", target_os = "unknown"), async_trait(?Send)]
#[cfg_attr(not(all(target_family = "wasm", target_os = "unknown")), async_trait]

I don't know why and how object_store works in wasm, I didnt dig there...

@kylebarron
Copy link
Member

This AsyncFileReader trait is identical to the parquet crate's AsyncFileReader, which I am using in production in wasm32-unknown-unknown in the browser: https://github.com/kylebarron/parquet-wasm

@feefladder
Copy link
Contributor Author

interesting...

@feefladder
Copy link
Contributor Author

feefladder commented Mar 21, 2025

Ok, you're right, I'm wrong...
seems wasm compatibility could be achieved with another middleware:

use wasm_bindgen::spawn_local; // added `spawn_local`

pub struct WasmReader {
  inner: Arc<dyn NotSendAsyncFileReader>
}

impl AsyncFileReader for WasmReader {
  async fn get_bytes(&self, range: Range<u64>) -> BoxFuture<'_, AsyncTiffResult<Bytes>> {
    let (rx, tx) = oneshot::new()
    let inner = self.inner.clone()
    spawn_local(async move {
      tx.send(inner.get_bytes(range).await) // this task is not Send, but that's ok since we don't use any references
    })
    rx.await // channel receive is Send!!!
  }
}

The trick then is to have the main functionality defined in some loose async fns that can be either Send or ?Send, depending on the target.

There's quite some weeds also, where wasm-bindgen plays a lot of magic tricks, but in the end it was indeed parquet-wasm that had the solution :D

anyways.... Would it maybe be nice to use the 1.75 announced trait_variant? That way it's easy here is dynamic dispatch...

Still there may be a nice solution, or does that need its own crate, like parquet-wasm?

feel free to close this PR or suggest a way forward. Thanks for the guidance :)

@kylebarron
Copy link
Member

It's not really a middleware because it's not wrapping anything. You just need this implementation: https://github.com/kylebarron/parquet-wasm/blob/e85322b592ab224938861895a06f4f3947a77802/src/reader_async.rs#L230-L291

@kylebarron
Copy link
Member

I think we established that we don't need the changes in this PR for wasm support, so I'll close this

@kylebarron kylebarron closed this Mar 21, 2025
@feefladder
Copy link
Contributor Author

feefladder commented Mar 22, 2025

It's not really a middleware because it's not wrapping anything. You just need this implementation: https://github.com/kylebarron/parquet-wasm/blob/e85322b592ab224938861895a06f4f3947a77802/src/reader_async.rs#L230-L291

The part that makes it wasm-compatible (turns a non-Send js future into a Sendable Rust future is in the make_range_request_with_client implementation (l436-L447), and it does the channel-trick. The thing that gets wrapped in my previous example is explicitly non-send, which it is makes send by the channel trick.

@feefladder feefladder mentioned this pull request Mar 22, 2025
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