Skip to content

Commit

Permalink
[fix]: Reduce configuration stack size
Browse files Browse the repository at this point in the history
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
  • Loading branch information
Arjentix committed Oct 11, 2023
1 parent f7c2272 commit 693da37
Show file tree
Hide file tree
Showing 13 changed files with 203 additions and 62 deletions.
14 changes: 14 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ duct = "0.13.6"
criterion = "0.3.6"
proptest = "1.0.0"
expect-test = "1.4.1"
stacker = "0.1.15"

eyre = "0.6.8"
color-eyre = "0.6.2"
Expand Down
2 changes: 1 addition & 1 deletion cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ impl Iroha {
|error| {
iroha_logger::warn!(%error, "Failed to load wsv from snapshot, creating empty wsv");
WorldStateView::from_configuration(
config.wsv,
*config.wsv,
world,
Arc::clone(&kura),
live_query_store_handle.clone(),
Expand Down
12 changes: 6 additions & 6 deletions cli/src/samples.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,16 @@ pub fn get_config_proxy(peers: UniqueVec<PeerId>, key_pair: Option<KeyPair>) ->
ConfigurationProxy {
public_key: Some(public_key.clone()),
private_key: Some(private_key.clone()),
sumeragi: Some(iroha_config::sumeragi::ConfigurationProxy {
sumeragi: Some(Box::new(iroha_config::sumeragi::ConfigurationProxy {
max_transactions_in_block: Some(2),
trusted_peers: Some(TrustedPeers { peers }),
..iroha_config::sumeragi::ConfigurationProxy::default()
}),
torii: Some(iroha_config::torii::ConfigurationProxy {
})),
torii: Some(Box::new(iroha_config::torii::ConfigurationProxy {
p2p_addr: Some(DEFAULT_TORII_P2P_ADDR.clone()),
api_url: Some(DEFAULT_API_ADDR.clone()),
..iroha_config::torii::ConfigurationProxy::default()
}),
})),
block_sync: Some(iroha_config::block_sync::ConfigurationProxy {
block_batch_size: Some(1),
gossip_period_ms: Some(500),
Expand All @@ -79,10 +79,10 @@ pub fn get_config_proxy(peers: UniqueVec<PeerId>, key_pair: Option<KeyPair>) ->
queue: Some(iroha_config::queue::ConfigurationProxy {
..iroha_config::queue::ConfigurationProxy::default()
}),
genesis: Some(iroha_config::genesis::ConfigurationProxy {
genesis: Some(Box::new(iroha_config::genesis::ConfigurationProxy {
account_private_key: Some(Some(private_key)),
account_public_key: Some(public_key),
}),
})),
..ConfigurationProxy::default()
}
}
Expand Down
2 changes: 1 addition & 1 deletion client/tests/integration/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ fn get_config() {
assert_eq!(cfg_proxy.network.unwrap().build().unwrap(), test.network);
assert_eq!(
cfg_proxy.telemetry.unwrap().build().unwrap(),
test.telemetry
*test.telemetry
);
}
1 change: 1 addition & 0 deletions config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ once_cell = { workspace = true }

[dev-dependencies]
proptest = { workspace = true }
stacker = { workspace = true }

[features]
default = []
Expand Down
47 changes: 39 additions & 8 deletions config/base/derive/src/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,11 @@ pub fn impl_load_from_env(ast: &StructWithFields) -> TokenStream {
.transpose()?;
};
if field.has_inner {
let maybe_map_box = gen_maybe_map_box(inner_ty);
set_field.extend(quote! {
let inner_proxy = <#inner_ty as #env_trait>::from_env(#env_fetcher_ident)?;
let inner_proxy = <#inner_ty as #env_trait>::from_env(#env_fetcher_ident)
#maybe_map_box
?;
let #ident = if let Some(old_inner) = #ident {
Some(<#inner_ty as ::iroha_config_base::proxy::Override>::override_with(old_inner, inner_proxy))
} else {
Expand Down Expand Up @@ -195,12 +198,7 @@ fn gen_proxy_struct(mut ast: StructWithFields) -> StructWithFields {
// For fields of `Configuration` that have an inner config, the corresponding
// proxy field should have a `..Proxy` type there as well
if field.has_inner {
#[allow(clippy::expect_used)]
if let Type::Path(path) = &mut field.ty {
let old_ident = &path.path.segments.last().expect("Can't be empty").ident;
let new_ident = format_ident!("{}Proxy", old_ident);
path.path.segments.last_mut().expect("Can't be empty").ident = new_ident;
}
proxify_field_type(&mut field.ty);
}
let ty = &field.ty;
field.ty = parse_quote! {
Expand Down Expand Up @@ -229,6 +227,22 @@ fn gen_proxy_struct(mut ast: StructWithFields) -> StructWithFields {
ast
}

#[allow(clippy::expect_used)]
pub fn proxify_field_type(field_ty: &mut syn::Type) {
if let Type::Path(path) = field_ty {
let last_segment = path.path.segments.last_mut().expect("Can't be empty");
if last_segment.ident == "Box" {
let box_generic = utils::extract_box_generic(last_segment);
// Recursion
proxify_field_type(box_generic)
} else {
// TODO: Wouldn't it be better to get it as an associated type?
let new_ident = format_ident!("{}Proxy", last_segment.ident);
last_segment.ident = new_ident;
}
}
}

pub fn impl_build(ast: &StructWithFields) -> TokenStream {
let checked_fields = gen_none_fields_check(ast);
let proxy_name = &ast.ident;
Expand Down Expand Up @@ -258,12 +272,17 @@ fn gen_none_fields_check(ast: &StructWithFields) -> proc_macro2::TokenStream {
if field.has_inner {
let inner_ty = get_inner_type("Option", &field.ty);
let builder_trait = quote! { ::iroha_config_base::proxy::Builder };

let maybe_map_box = gen_maybe_map_box(inner_ty);

quote! {
#ident: <#inner_ty as #builder_trait>::build(
self.#ident.ok_or(
#missing_field{field: stringify!(#ident), message: ""}
)?
)?
)
#maybe_map_box
?
}
} else {
quote! {
Expand All @@ -278,6 +297,18 @@ fn gen_none_fields_check(ast: &StructWithFields) -> proc_macro2::TokenStream {
}
}

fn gen_maybe_map_box(inner_ty: &syn::Type) -> proc_macro2::TokenStream {
if let Type::Path(path) = &inner_ty {
let last_segment = path.path.segments.last().expect("Can't be empty");
if last_segment.ident == "Box" {
return quote! {
.map(Box::new)
};
}
}
quote! {}
}

/// Helper function to be used as an empty fallback for [`impl LoadFromEnv`] or [`impl LoadFromDisk`].
/// Only meant for proxy types usage.
fn gen_none_fields_proxy(ast: &StructWithFields) -> proc_macro2::TokenStream {
Expand Down
16 changes: 16 additions & 0 deletions config/base/derive/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,3 +350,19 @@ pub fn get_parent_ty(ast: &StructWithFields) -> Type {
.map(|builder| builder.parent)
.expect("Should not be called on structs with no `#[builder(..)]` attribute")
}

pub fn extract_box_generic(box_seg: &mut syn::PathSegment) -> &mut syn::Type {
let syn::PathArguments::AngleBracketed(generics) = &mut box_seg.arguments else {
panic!("`Box` should have explicit generic");
};

assert!(
generics.args.len() == 1,
"`Box` should have exactly one generic argument"
);
let syn::GenericArgument::Type(generic_type) = generics.args.first_mut().expect("Can't be empty") else {
panic!("`Box` should have type as a generic argument")
};

generic_type
}
20 changes: 19 additions & 1 deletion config/base/derive/src/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,24 @@ mod gen {
})
.collect::<Vec<_>>();

let field_froms: Vec<_> = fields
.iter()
.map(|field| {
let field_ident = &field.ident;
if let syn::Type::Path(syn::TypePath { path, .. }) = &field.ty {
let last_segment = path.segments.last().expect("Not empty");
if last_segment.ident == "Box" {
return quote! {
#field_ident: Box::new(core::convert::From::<_>::from(*#field_ident)),
};
}
}
quote! {
#field_ident: core::convert::From::<_>::from(#field_ident),
}
})
.collect();

quote! {
impl #impl_generics core::convert::From<#original_ident> for #view_ident #ty_generics #where_clause {
fn from(config: #original_ident) -> Self {
Expand All @@ -100,7 +118,7 @@ mod gen {
Self {
#(
#(#field_cfg_attrs)*
#field_idents: core::convert::From::<_>::from(#field_idents),
#field_froms
)*
}
}
Expand Down
48 changes: 48 additions & 0 deletions config/base/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,32 @@ pub mod proxy {
) -> Result<Option<String>, Self::Error>;
}

impl<T: Documented> Documented for Box<T> {
type Error = T::Error;

fn get_docs() -> Value {
T::get_docs()
}

fn get_inner_docs() -> String {
T::get_inner_docs()
}

fn get_recursive<'tl, U>(&self, inner_field: U) -> Result<Value, Self::Error>
where
U: AsRef<[&'tl str]> + Send + 'tl,
{
T::get_recursive(self, inner_field)
}

#[allow(single_use_lifetimes)] // False-positive
fn get_doc_recursive<'tl>(
field: impl AsRef<[&'tl str]>,
) -> Result<Option<String>, Self::Error> {
T::get_doc_recursive(field)
}
}

/// Trait for combining two configuration instances
pub trait Override: Serialize + DeserializeOwned + Sized {
/// If any of the fields in `other` are filled, they
Expand All @@ -509,6 +535,12 @@ pub mod proxy {
fn override_with(self, other: Self) -> Self;
}

impl<T: Override> Override for Box<T> {
fn override_with(self, other: Self) -> Self {
Box::new(T::override_with(*self, *other))
}
}

/// Trait for configuration loading and deserialization from
/// the environment
pub trait LoadFromEnv: Sized {
Expand Down Expand Up @@ -540,6 +572,14 @@ pub mod proxy {
}
}

impl<T: LoadFromEnv> LoadFromEnv for Box<T> {
type ReturnValue = T::ReturnValue;

fn from_env<F: FetchEnv>(fetcher: &F) -> Self::ReturnValue {
T::from_env(fetcher)
}
}

/// Abstraction over the actual implementation of how env variables are gotten
/// from the environment. Necessary for mocking in tests.
pub trait FetchEnv {
Expand Down Expand Up @@ -576,6 +616,14 @@ pub mod proxy {
fn build(self) -> Self::ReturnValue;
}

impl<T: Builder> Builder for Box<T> {
type ReturnValue = T::ReturnValue;

fn build(self) -> Self::ReturnValue {
T::build(*self)
}
}

/// Deserialization helper for proxy fields that wrap an `Option`
///
/// # Errors
Expand Down
Loading

0 comments on commit 693da37

Please sign in to comment.