From e036b980570d70aa9c7c2807cfe5769a37a10a0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Br=C3=BCnig?= Date: Sun, 9 Jun 2024 12:38:27 +0200 Subject: [PATCH 1/4] improve documentation of tree_reduce --- src/lib.rs | 53 +++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 018f877e5..1861360bf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2528,24 +2528,26 @@ pub trait Itertools: Iterator { /// /// If `f` is associative you should also decide carefully: /// - /// - if `f` is a trivial operation like `u32::wrapping_add`, prefer the normal - /// [`Iterator::reduce`] instead since it will most likely result in the generation of simpler - /// code because the compiler is able to optimize it - /// - otherwise if `f` is non-trivial like `format!`, you should use `tree_reduce` since it - /// reduces the number of operations from `O(n)` to `O(ln(n))` + /// For an iterator producing `n` elements, both [`Iterator::reduce`] and `tree_reduce` will + /// call `f` `n - 1` times. However, `tree_reduce` will call `f` on earlier intermediate + /// results, which is beneficial for `f` that allocate and produce longer results for longer + /// arguments. For example if `f` combines arguments using `format!`, then `tree_reduce` will + /// operate on average on shorter arguments resulting in less bytes being allocated overall. /// - /// Here "non-trivial" means: - /// - /// - any allocating operation - /// - any function that is a composition of many operations + /// If 'f' does not benefit from such a reordering, like `u32::wrapping_add`, prefer the + /// normal [`Iterator::reduce`] instead since it will most likely result in the generation of + /// simpler code because the compiler is able to optimize it. /// /// ``` /// use itertools::Itertools; /// + /// let f = |a: String, b: String| { + /// format!("f({a}, {b})") + /// }; + /// /// // The same tree as above - /// let num_strings = (1..8).map(|x| x.to_string()); - /// assert_eq!(num_strings.tree_reduce(|x, y| format!("f({}, {})", x, y)), - /// Some(String::from("f(f(f(1, 2), f(3, 4)), f(f(5, 6), 7))"))); + /// assert_eq!((1..8).map(|x| x.to_string()).tree_reduce(f), + /// Some(String::from("f(f(f(1, 2), f(3, 4)), f(f(5, 6), 7))"))); /// /// // Like fold1, an empty iterator produces None /// assert_eq!((0..0).tree_reduce(|x, y| x * y), None); @@ -2553,9 +2555,36 @@ pub trait Itertools: Iterator { /// // tree_reduce matches fold1 for associative operations... /// assert_eq!((0..10).tree_reduce(|x, y| x + y), /// (0..10).fold1(|x, y| x + y)); + /// /// // ...but not for non-associative ones /// assert_ne!((0..10).tree_reduce(|x, y| x - y), /// (0..10).fold1(|x, y| x - y)); + /// + /// + /// let mut total_capacity_reduce = 0; + /// let reduce_res = (1..100).map(|x| x.to_string()) + /// .reduce(|a, b| { + /// let r = f(a, b); + /// total_capacity_reduce += r.capacity(); + /// r + /// }) + /// .unwrap(); + /// + /// let mut total_capacity_tree_reduce = 0; + /// let tree_reduce_res = (1..100).map(|x| x.to_string()) + /// .tree_reduce(|a, b| { + /// let r = f(a, b); + /// total_capacity_tree_reduce += r.capacity(); + /// r + /// }) + /// .unwrap(); + /// + /// dbg!(total_capacity_reduce, total_capacity_tree_reduce, + /// reduce_res.len(), tree_reduce_res.len()); + /// // total_capacity_reduce = 65630 + /// // total_capacity_tree_reduce = 7284 + /// // reduce_res.len() = 679 + /// // tree_reduce_res.len() = 679 /// ``` fn tree_reduce(mut self, mut f: F) -> Option where From 40d02858a8c8188a81d6d84eef9c346bc9f2781a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Br=C3=BCnig?= Date: Sun, 16 Jun 2024 11:37:27 +0200 Subject: [PATCH 2/4] tree_reduce doc: use len() instead of capacity() --- src/lib.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1861360bf..7ca6e58a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2560,31 +2560,27 @@ pub trait Itertools: Iterator { /// assert_ne!((0..10).tree_reduce(|x, y| x - y), /// (0..10).fold1(|x, y| x - y)); /// - /// - /// let mut total_capacity_reduce = 0; + /// let mut total_len_reduce = 0; /// let reduce_res = (1..100).map(|x| x.to_string()) /// .reduce(|a, b| { /// let r = f(a, b); - /// total_capacity_reduce += r.capacity(); + /// total_len_reduce += r.len(); /// r /// }) /// .unwrap(); /// - /// let mut total_capacity_tree_reduce = 0; + /// let mut total_len_tree_reduce = 0; /// let tree_reduce_res = (1..100).map(|x| x.to_string()) /// .tree_reduce(|a, b| { /// let r = f(a, b); - /// total_capacity_tree_reduce += r.capacity(); + /// total_len_tree_reduce += r.len(); /// r /// }) /// .unwrap(); /// - /// dbg!(total_capacity_reduce, total_capacity_tree_reduce, - /// reduce_res.len(), tree_reduce_res.len()); - /// // total_capacity_reduce = 65630 - /// // total_capacity_tree_reduce = 7284 - /// // reduce_res.len() = 679 - /// // tree_reduce_res.len() = 679 + /// assert_eq!(total_len_reduce, 33299); + /// assert_eq!(total_len_tree_reduce, 4228); + /// assert_eq!(reduce_res.len(), tree_reduce_res.len()); /// ``` fn tree_reduce(mut self, mut f: F) -> Option where From cc4c503605e0d2272492d7f2d8d28cdf32f1bc90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Br=C3=BCnig?= Date: Sun, 16 Jun 2024 11:45:06 +0200 Subject: [PATCH 3/4] tree_reduce doc: explain benefit when building binary search tree --- src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 7ca6e58a3..c05e28266 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2534,6 +2534,11 @@ pub trait Itertools: Iterator { /// arguments. For example if `f` combines arguments using `format!`, then `tree_reduce` will /// operate on average on shorter arguments resulting in less bytes being allocated overall. /// + /// Moreover, the output of `tree_reduce` is preferable to that of [`Iterator::reduce`] in + /// certain cases. For example, building a binary search tree using `tree_reduce` will result in + /// a balanced tree with height O(ln(n)), while [`Iterator::reduce`] will output a tree with + /// height O(n), essentially a linked list. + /// /// If 'f' does not benefit from such a reordering, like `u32::wrapping_add`, prefer the /// normal [`Iterator::reduce`] instead since it will most likely result in the generation of /// simpler code because the compiler is able to optimize it. From 44a73c5742500eee6751107d24570b7cd56d6ea1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Br=C3=BCnig?= Date: Mon, 17 Jun 2024 12:39:12 +0200 Subject: [PATCH 4/4] tree_reduce doc: fix formatting --- src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c05e28266..0e7e68fb8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2536,10 +2536,10 @@ pub trait Itertools: Iterator { /// /// Moreover, the output of `tree_reduce` is preferable to that of [`Iterator::reduce`] in /// certain cases. For example, building a binary search tree using `tree_reduce` will result in - /// a balanced tree with height O(ln(n)), while [`Iterator::reduce`] will output a tree with - /// height O(n), essentially a linked list. + /// a balanced tree with height `O(ln(n))`, while [`Iterator::reduce`] will output a tree with + /// height `O(n)`, essentially a linked list. /// - /// If 'f' does not benefit from such a reordering, like `u32::wrapping_add`, prefer the + /// If `f` does not benefit from such a reordering, like `u32::wrapping_add`, prefer the /// normal [`Iterator::reduce`] instead since it will most likely result in the generation of /// simpler code because the compiler is able to optimize it. ///