From 1410e86508fb2c989ae3e8193407e466ea604832 Mon Sep 17 00:00:00 2001 From: Christian Hagemeier Date: Mon, 13 Jan 2025 18:43:03 +0000 Subject: [PATCH] This changes how we generate the trees by using the recursive strategy once and then mapping file-nodes to empty directories. This should be more performant than our old implementation, as that lead to the inner strategy (which defines # of nodes, etc ... for the whole tree) to be run multiple times, causing a degradation in performance of proptest generation. Signed-off-by: Christian Hagemeier --- mountpoint-s3/tests/reftests/generators.rs | 23 ++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/mountpoint-s3/tests/reftests/generators.rs b/mountpoint-s3/tests/reftests/generators.rs index 13e053689..1a5624c73 100644 --- a/mountpoint-s3/tests/reftests/generators.rs +++ b/mountpoint-s3/tests/reftests/generators.rs @@ -127,21 +127,24 @@ impl Debug for TreeNode { /// Leaves are always [TreeNode::File] or an empty [TreeNode::Directory]. /// Parents are always [TreeNode::Directory]. pub fn gen_tree(depth: u32, max_size: u32, max_items: u32, max_width: usize) -> impl Strategy { - let leaf = any::().prop_map(TreeNode::File); - - let strategy = leaf.prop_recursive( + let leaf = prop_oneof![any::().prop_map(TreeNode::File),]; + leaf.prop_recursive( depth, // levels max_size, // max number of nodes max_items, // number of items per collection move |inner| { - // Take the inner strategy and make the recursive cases. - // Since the size of the tree could be zero, this also introduces directories as leaves. - prop::collection::btree_map(any::(), inner, 0..max_width).prop_map(TreeNode::Directory) + prop_oneof![ + // Take the inner strategy and make the recursive cases. + // Since the size of the tree could be zero, this also introduces directories as leaves. + prop::collection::btree_map(any::(), inner, 0..max_width).prop_map(TreeNode::Directory), + ] }, - ); - - // Ensure the root is always a directory by wrapping the inner strategy - prop::collection::btree_map(any::(), strategy, 0..max_width).prop_map(TreeNode::Directory) + ) + // Ensure the root is always a directory by transforming invalid nodes (i.e. file as root) into empty directories + .prop_map(|x| match x { + TreeNode::File(_) => TreeNode::Directory(BTreeMap::from([])), + _ => x, + }) } /// Take a generated tree and create the corresponding S3 namespace (list of keys)