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 18, 2023
1 parent 24c3220 commit 6c56507
Show file tree
Hide file tree
Showing 10 changed files with 181 additions and 61 deletions.
2 changes: 1 addition & 1 deletion cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,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 @@ -60,16 +60,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 @@ -78,10 +78,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 @@ -22,6 +22,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
);
}
46 changes: 39 additions & 7 deletions config/base/derive/src/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,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 @@ -194,11 +197,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 {
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 @@ -227,6 +226,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 @@ -256,12 +271,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 @@ -276,6 +296,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 @@ -347,3 +347,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 @@ -86,6 +86,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 @@ -99,7 +117,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
62 changes: 34 additions & 28 deletions config/src/iroha.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ view! {
pub disable_panic_terminal_colors: bool,
/// `Kura` configuration
#[config(inner)]
pub kura: kura::Configuration,
pub kura: Box<kura::Configuration>,
/// `Sumeragi` configuration
#[config(inner)]
#[view(into = sumeragi::ConfigurationView)]
pub sumeragi: sumeragi::Configuration,
#[view(into = Box<sumeragi::ConfigurationView>)]
pub sumeragi: Box<sumeragi::Configuration>,
/// `Torii` configuration
#[config(inner)]
pub torii: torii::Configuration,
pub torii: Box<torii::Configuration>,
/// `BlockSynchronizer` configuration
#[config(inner)]
pub block_sync: block_sync::Configuration,
Expand All @@ -40,23 +40,23 @@ view! {
pub queue: queue::Configuration,
/// `Logger` configuration
#[config(inner)]
pub logger: logger::Configuration,
pub logger: Box<logger::Configuration>,
/// `GenesisBlock` configuration
#[config(inner)]
#[view(into = genesis::ConfigurationView)]
pub genesis: genesis::Configuration,
#[view(into = Box<genesis::ConfigurationView>)]
pub genesis: Box<genesis::Configuration>,
/// `WorldStateView` configuration
#[config(inner)]
pub wsv: wsv::Configuration,
pub wsv: Box<wsv::Configuration>,
/// Network configuration
#[config(inner)]
pub network: network::Configuration,
/// Telemetry configuration
#[config(inner)]
pub telemetry: telemetry::Configuration,
pub telemetry: Box<telemetry::Configuration>,
/// SnapshotMaker configuration
#[config(inner)]
pub snapshot: snapshot::Configuration,
pub snapshot: Box<snapshot::Configuration>,
/// LiveQueryStore configuration
#[config(inner)]
pub live_query_store: live_query_store::Configuration,
Expand All @@ -69,17 +69,17 @@ impl Default for ConfigurationProxy {
public_key: None,
private_key: None,
disable_panic_terminal_colors: Some(bool::default()),
kura: Some(kura::ConfigurationProxy::default()),
sumeragi: Some(sumeragi::ConfigurationProxy::default()),
torii: Some(torii::ConfigurationProxy::default()),
kura: Some(Box::default()),
sumeragi: Some(Box::default()),
torii: Some(Box::default()),
block_sync: Some(block_sync::ConfigurationProxy::default()),
queue: Some(queue::ConfigurationProxy::default()),
logger: Some(logger::ConfigurationProxy::default()),
genesis: Some(genesis::ConfigurationProxy::default()),
wsv: Some(wsv::ConfigurationProxy::default()),
logger: Some(Box::default()),
genesis: Some(Box::default()),
wsv: Some(Box::default()),
network: Some(network::ConfigurationProxy::default()),
telemetry: Some(telemetry::ConfigurationProxy::default()),
snapshot: Some(snapshot::ConfigurationProxy::default()),
telemetry: Some(Box::default()),
snapshot: Some(Box::default()),
live_query_store: Some(live_query_store::ConfigurationProxy::default()),
}
}
Expand Down Expand Up @@ -208,17 +208,17 @@ mod tests {
fn arb_proxy()(
(public_key, private_key) in arb_keys(),
disable_panic_terminal_colors in prop::option::of(Just(true)),
kura in prop::option::of(kura::tests::arb_proxy()),
sumeragi in prop::option::of(sumeragi::tests::arb_proxy()),
torii in prop::option::of(torii::tests::arb_proxy()),
kura in prop::option::of(kura::tests::arb_proxy().prop_map(Box::new)),
sumeragi in (prop::option::of(sumeragi::tests::arb_proxy().prop_map(Box::new))),
torii in (prop::option::of(torii::tests::arb_proxy().prop_map(Box::new))),
block_sync in prop::option::of(block_sync::tests::arb_proxy()),
queue in prop::option::of(queue::tests::arb_proxy()),
logger in prop::option::of(logger::tests::arb_proxy()),
genesis in prop::option::of(genesis::tests::arb_proxy()),
wsv in prop::option::of(wsv::tests::arb_proxy()),
logger in prop::option::of(logger::tests::arb_proxy().prop_map(Box::new)),
genesis in prop::option::of(genesis::tests::arb_proxy().prop_map(Box::new)),
wsv in prop::option::of(wsv::tests::arb_proxy().prop_map(Box::new)),
network in prop::option::of(network::tests::arb_proxy()),
telemetry in prop::option::of(telemetry::tests::arb_proxy()),
snapshot in prop::option::of(snapshot::tests::arb_proxy()),
telemetry in prop::option::of(telemetry::tests::arb_proxy().prop_map(Box::new)),
snapshot in prop::option::of(snapshot::tests::arb_proxy().prop_map(Box::new)),
live_query_store in prop::option::of(live_query_store::tests::arb_proxy()),
) -> ConfigurationProxy {
ConfigurationProxy { public_key, private_key, disable_panic_terminal_colors, kura, sumeragi, torii, block_sync, queue,
Expand All @@ -227,8 +227,7 @@ mod tests {
}

proptest! {
#[test]
fn iroha_proxy_build_fails_on_none(proxy in arb_proxy()) {
fn __iroha_proxy_build_fails_on_none(proxy in arb_proxy()) {
let cfg = proxy.build();
let example_cfg = ConfigurationProxy::from_path(CONFIGURATION_PATH).build().expect("Failed to build example Iroha config");
if cfg.is_ok() {
Expand All @@ -237,6 +236,13 @@ mod tests {
}
}

#[test]
fn iroha_proxy_build_fails_on_none() {
// Using `stacker` because test generated by `proptest!` takes too much stack space.
// Allocating 3MB.
stacker::grow(3 * 1024 * 1024, __iroha_proxy_build_fails_on_none)
}

#[test]
fn parse_example_json() {
let cfg_proxy = ConfigurationProxy::from_path(CONFIGURATION_PATH);
Expand Down
Loading

0 comments on commit 6c56507

Please sign in to comment.