diff --git a/docs/adr/0008-wrapper-code.md b/docs/adr/0008-wrapper-code.md new file mode 100644 index 0000000000..6fdba28897 --- /dev/null +++ b/docs/adr/0008-wrapper-code.md @@ -0,0 +1,58 @@ +# Wrapper code + +* Status: draft +* Deciders: +* Date: ??? + +## Context and Problem Statement + +Application-services components currently consist of a Rust core that gets wrapped by Swift and Kotlin wrappers. +In the past, these wrappers were strictly necessary because of deficiencies in our FFI strategy. +However, UniFFI is reaching the point where it can support all our requirements and it's possible to remove the wrapper code. + +So... should we? + +### Scope + +This ADR discusses what our general policy on wrapper code should be. +It does not cover how we should plan our work. +If we decide to reject wrapper code, we do not need to commit to any particular timeline for actually removing it. + +## Decision Drivers + +## Considered Options + +* **(A) Embrace wrapper code and continue to use it** +* **(B) Reject wrapper code and remove it** +* **(C) Use wrapper code for async wrapping only** + +## Decision Outcome + +## Pros and Cons of the Options + +### (A) Embrace wrapper code and continue to use it + +* Good, because wrapper code allows us to wrap sync Rust code in async wrapper functions and this is a low-risk approach to async. +* Good, because wrapper code allows us to mitigate breaking changes in the Rust code. +* Bad, because there are better ways to handle breaking changes, like Rust feature flags. + We could introduce breaking changes, and large changes in general, behind a feature flag. + We would wait to enable the feature flag on the megazord until consumer application was ready. +* Good, because it allows us to present APIs using idiomatic, native, types. + For example, UniFFI callback interfaces may meet our requirements, but the Swift `NotificationCenter` may provide a more ergonomic API. + +### (B) Reject wrapper code and remove it + +* Good, because it simplifies our documentation strategy. + There is active work in UniFFI to autogenerate the bindings documentation from the Rust docstrings (https://github.com/mozilla/uniffi-rs/pull/1498, https://github.com/mozilla/uniffi-rs/pull/1493). + If there are no wrappers, then we could potentially use this to auto-generate a high-level documentation site and/or docstrings in the generated bindings code. + If there are wrappers, then this probably isn't going to work. + In general, wrappers mean we are have multiple public APIs which makes documentation harder. +* Good, because a "vanilla" API may be easier to integrate with multiple consumer apps. + `NotificationCenter` might be the preferred choice for firefox-ios, but another Swift app may want to use a different system. + By only using UniFFI-supported types we can be fairly sure that our code will work with any system. + +### (C) Use wrapper code for async wrapping only + +* We must chose this one over (B) if we decide to avoid async Rust in `ADR-0009` +* Good, because a "vanilla" API may be easier to integrate with multiple consumer apps. +* Good, because it somewhat simplifies our documentation strategy, although not as much as (B). diff --git a/docs/adr/0009-async-rust.md b/docs/adr/0009-async-rust.md new file mode 100644 index 0000000000..1b0377bb2c --- /dev/null +++ b/docs/adr/0009-async-rust.md @@ -0,0 +1,111 @@ +# Using Async Rust + +* Status: draft +* Deciders: +* Date: ??? + +## Context and Problem Statement + +Our Rust components are currently written as using synchronous Rust. +The components are then wrapped in Kotlin to present an async interface. +Swift also wraps them to present an async-style interface, although it currently uses `DispatchQueue` and completion handlers rather than `async` functions. + +UniFFI has been adding async capabilities in the last year and it seems possible to switch to using async Rust and not having an async wrapper. + +So... should we? + +### Scope + +This ADR discusses what our general policy on wrapper code should be. +It does not cover how we should plan our work. +If we decide to embrace async Rust, we do not need to commit to any particular timeline for actually switching to it. + +### Desktop and gecko-js + +On desktop, we can't write async wrappers because it's not possible in Javascript. +Instead we use a strategy where every function is automatically wrapped as async in the C++ layer. +Using a config file, it's possible to opt-out of this strategy for particular functions/methods. + +This seems to be working okay although it feels slightly weird. +It's not completely clear that it would scale with more complex components. + +### Android-components + +In Kotlin, the async wrapper layer currently lives in `android-components`. +For the purposes of this ADR, it doesn't really matter, and this ADR will not make a distinction between wrapper code in our repo and `android-components`. + +## How it would work + +### SQLite queries + +One of the reasons our code currently blocks is to run SQLite queries. +https://github.com/mozilla/uniffi-rs/pull/1837 has a system to run blocking code inside an async function. +It would basically mean replacing code like this: + +```kotlin + override suspend fun wipeLocal() = withContext(coroutineContext) { + conn.getStorage().wipeLocal() + } +``` + +with code like this: +```rust + + async fn wipe_local() { + self.queue.run_blocking(|| self.db.wipe_local()).await + } +``` + +We would need to merge this code, which is currently planed for the end of 2023. + +### Locks + +Another reason our code blocks is to wait on a `Mutex` or `RWLock`. +There are a few ways we could handle this: + +* The simplest is to continue using regular locks, inside a `run_blocking()` call, which would be very similar to our current system. +* We could also consider switching to `async_lock` and reversing the order: lock first, then make a `run_blocking()` call. + This may be more efficient since the async task would suspend while waiting for the lock rather than blocking a thread +* We could also ditch locks and use [actors and channels](https://ryhl.io/blog/actors-with-tokio/) to protect our resources. + It's probably not worth rewriting our current components to do this, but this approach might be useful for new components. + +### Network requests + +The last reason we block is for network requests. +To support that we would probably need some sort of "async viaduct" that would allow consumer applications to choose either: +- Use async functions from the `reqwest` library. + This matches what we currently do for `firefox-ios`. +- Use the foreign language's network stack via an async callback interface. + This matches what we currently do for `firefox-android`. + This would require implemnenting https://github.com/mozilla/uniffi-rs/issues/1729, which is currently planed for the end of 2023. + +## Decision Drivers + +## Considered Options + +* **(A) Embrace Async Rust** +* **(B) Avoid Async Rust** + +## Decision Outcome + +## Pros and Cons of the Options + +### (A) Embrace Async Rust + +* Good, if we decide to avoid wrappers in `ADR-0008` because it allows us to remove the async wrappers. +* Bad, because there's a risk that the UniFFI async code will cause issues and our current async strategy is working okay. +* Good because it allows us to be more effecient with our thread usage. + When an async task is waiting on a lock or network request, it can suspend itself and release the thread for other async work. + Currently, we need to block a thread while we are waiting for this. + However, it's not clear this would meaningfully affect our consumers since we don't run that many blocking operations. + We would be saving maybe 1-2 threads at certain points. +* Good, because it makes it easier to integrate with new languages that expect async. + For example, WASM integration libraries usually returns `Future` objects to Rust which can only be evaluated in an async context. + Note: this is a separate issue from UniFFI adding WASM support. + If we switched our component code to using async Rust, it's possible that we could use `wasm-bindgen` instead. +* Bad, because it makes it harder to provide bindings on new languages that don't support async, like C and C++. + Maybe we could bridge the gap with some sort of callback-based async system, but it's not clear how that would work. + +### (B) Avoid Async Rust + +This is basically just the inverse of the last section.