Skip to content

Commit

Permalink
Use trait objects internally to avoid generic instantiation (#60)
Browse files Browse the repository at this point in the history
Keep the external interface to async transactions *mostly* unchanged, but delegate to internal functions that (as much as possible) avoid using generic functions.

This is intended to reduce code generation. For a debug build of `omicron-nexus`, this reduces the (debug build, Linux) binary size from 514 MiB to 497 MiB.

Source for idea: https://matklad.github.io/2021/09/04/fast-rust-builds.html#Keeping-Instantiations-In-Check

> A... common case here are closures: by default, prefer &dyn Fn() over impl Fn(). Similarly to paths, an impl-based nice API might be a thin wrapper around dyn-based implementation which does the bulk of the work.
  • Loading branch information
smklein authored Dec 7, 2023
1 parent ed7ab5e commit 3126feb
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 4 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ default = [ "cockroach" ]
bb8 = "0.8"
async-trait = "0.1.73"
diesel = { version = "2.1.0", default-features = false, features = [ "r2d2" ] }
futures = "0.3"
thiserror = "1.0"
tokio = { version = "1.32", default-features = false, features = [ "rt-multi-thread" ] }

Expand Down
76 changes: 72 additions & 4 deletions src/async_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ use diesel::{
},
result::Error as DieselError,
};
use futures::future::BoxFuture;
use futures::future::FutureExt;
use std::any::Any;
use std::future::Future;
use std::sync::Arc;
use std::sync::MutexGuard;
Expand Down Expand Up @@ -164,12 +167,45 @@ where
retry: RetryFunc,
) -> Result<R, DieselError>
where
R: Send + 'static,
Fut: Future<Output = Result<R, DieselError>> + Send,
Func: Fn(SingleConnection<Conn>) -> Fut + Send + Sync,
RetryFut: Future<Output = bool> + Send,
R: Any + Send + 'static,
Fut: FutureExt<Output = Result<R, DieselError>> + Send,
Func: (Fn(SingleConnection<Conn>) -> Fut) + Send + Sync,
RetryFut: FutureExt<Output = bool> + Send,
RetryFunc: Fn() -> RetryFut + Send + Sync,
{
// This function sure has a bunch of generic parameters, which can cause
// a lot of code to be generated, and can slow down compile-time.
//
// This API intends to provide a convenient, generic, shim over the
// dynamic "transaction_async_with_retry_inner" function below, which
// should avoid being generic. The goal here is to instantiate only one
// significant "body" of this function, while retaining flexibility for
// clients using this library.

// Box the functions, and box the return value.
let f = |conn| {
f(conn)
.map(|result| result.map(|r| Box::new(r) as Box<dyn Any + Send>))
.boxed()
};
let retry = || retry().boxed();

// Call the dynamically dispatched function, then retrieve the return
// value out of a Box.
self.transaction_async_with_retry_inner(&f, &retry)
.await
.map(|v| *v.downcast::<R>().expect("Should be an 'R' type"))
}

// NOTE: This function intentionally avoids all generics!
#[cfg(feature = "cockroach")]
async fn transaction_async_with_retry_inner(
&self,
f: &(dyn Fn(SingleConnection<Conn>) -> BoxFuture<'_, Result<Box<dyn Any + Send>, DieselError>>
+ Send
+ Sync),
retry: &(dyn Fn() -> BoxFuture<'_, bool> + Send + Sync),
) -> Result<Box<dyn Any + Send>, DieselError> {
// Check out a connection once, and use it for the duration of the
// operation.
let conn = Arc::new(self.get_owned_connection());
Expand Down Expand Up @@ -253,6 +289,38 @@ where
E: From<DieselError> + Send + 'static,
Fut: Future<Output = Result<R, E>> + Send,
Func: FnOnce(SingleConnection<Conn>) -> Fut + Send,
{
// This function sure has a bunch of generic parameters, which can cause
// a lot of code to be generated, and can slow down compile-time.
//
// This API intends to provide a convenient, generic, shim over the
// dynamic "transaction_async_with_retry_inner" function below, which
// should avoid being generic. The goal here is to instantiate only one
// significant "body" of this function, while retaining flexibility for
// clients using this library.

let f = Box::new(move |conn| {
f(conn)
.map(|result| result.map(|r| Box::new(r) as Box<dyn Any + Send>))
.boxed()
});

self.transaction_async_inner(f)
.await
.map(|v| *v.downcast::<R>().expect("Should be an 'R' type"))
}

// NOTE: This function intentionally avoids as many generic parameters as possible
async fn transaction_async_inner<'a, E>(
&'a self,
f: Box<
dyn FnOnce(SingleConnection<Conn>) -> BoxFuture<'a, Result<Box<dyn Any + Send>, E>>
+ Send
+ 'a,
>,
) -> Result<Box<dyn Any + Send>, E>
where
E: From<DieselError> + Send + 'static,
{
// Check out a connection once, and use it for the duration of the
// operation.
Expand Down

0 comments on commit 3126feb

Please sign in to comment.