From f9c15de1fd271032ff44288fa2f0599edd9dd204 Mon Sep 17 00:00:00 2001 From: Daniel Micay Date: Sun, 10 Feb 2013 20:37:21 -0500 Subject: [PATCH 1/4] treemap: use an &mut parameter for skew and split results in a small performance improvement and reduces the compiled code size --- src/libstd/treemap.rs | 47 ++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/src/libstd/treemap.rs b/src/libstd/treemap.rs index d8deea60725..c3cb45a2bfb 100644 --- a/src/libstd/treemap.rs +++ b/src/libstd/treemap.rs @@ -576,29 +576,25 @@ pure fn each_reverse(node: &r/Option<~TreeNode>, } // Remove left horizontal link by rotating right -fn skew(mut node: ~TreeNode) -> ~TreeNode { +fn skew(node: &mut ~TreeNode) { if node.left.map_default(false, |x| x.level == node.level) { let mut save = node.left.swap_unwrap(); node.left <-> save.right; // save.right now None - save.right = Some(node); - save - } else { - node // nothing to do + *node <-> save; + node.right = Some(save); } } // Remove dual horizontal link by rotating left and increasing level of // the parent -fn split(mut node: ~TreeNode) -> ~TreeNode { +fn split(node: &mut ~TreeNode) { if node.right.map_default(false, |x| x.right.map_default(false, |y| y.level == node.level)) { let mut save = node.right.swap_unwrap(); node.right <-> save.left; // save.left now None - save.left = Some(node); save.level += 1; - save - } else { - node // nothing to do + *node <-> save; + node.left = Some(save); } } @@ -611,11 +607,15 @@ fn insert(node: &mut Option<~TreeNode>, key: K, let mut save = node.swap_unwrap(); if key < save.key { let inserted = insert(&mut save.left, key, value); - *node = Some(split(skew(save))); // re-balance, if necessary + skew(&mut save); + split(&mut save); + *node = Some(save); // re-balance, if necessary inserted } else if save.key < key { let inserted = insert(&mut save.right, key, value); - *node = Some(split(skew(save))); // re-balance, if necessary + skew(&mut save); + split(&mut save); + *node = Some(save); // re-balance, if necessary inserted } else { save.key = key; @@ -684,15 +684,24 @@ fn remove(node: &mut Option<~TreeNode>, key: &K) -> bool { do save.right.mutate |mut x| { x.level = save.level; x } } - save = skew(save); + skew(&mut save); - do save.right.mutate |mut right| { - right = skew(right); - right.right.mutate(skew); - right + match save.right { + Some(ref mut right) => { + skew(right); + match right.right { + Some(ref mut x) => { skew(x) }, + None => () + } + } + None => () + } + + split(&mut save); + match save.right { + Some(ref mut x) => { split(x) }, + None => () } - save = split(save); - save.right.mutate(split); } *node = Some(save); From 195a969bb3e42e82e647e6ffead557f29884ff41 Mon Sep 17 00:00:00 2001 From: Daniel Micay Date: Sun, 10 Feb 2013 20:44:15 -0500 Subject: [PATCH 2/4] treemap: avoid swap_unwrap in insert Performance before: std::treemap::TreeMap sequential_ints 0.151877 s random_ints 0.160926 s delete_ints 0.08694 s sequential_strings 0.316458 s random_strings 0.290778 s delete_strings 0.169892 s Performance after: std::treemap::TreeMap sequential_ints 0.083971 s random_ints 0.095861 s delete_ints 0.083931 s sequential_strings 0.278272 s random_strings 0.240286 s delete_strings 0.173581 s --- src/libstd/treemap.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/libstd/treemap.rs b/src/libstd/treemap.rs index c3cb45a2bfb..4b02a13583f 100644 --- a/src/libstd/treemap.rs +++ b/src/libstd/treemap.rs @@ -600,29 +600,28 @@ fn split(node: &mut ~TreeNode) { fn insert(node: &mut Option<~TreeNode>, key: K, value: V) -> bool { - if node.is_none() { - *node = Some(~TreeNode::new(key, value)); - true - } else { - let mut save = node.swap_unwrap(); + match *node { + Some(ref mut save) => { if key < save.key { let inserted = insert(&mut save.left, key, value); - skew(&mut save); - split(&mut save); - *node = Some(save); // re-balance, if necessary + skew(save); + split(save); inserted } else if save.key < key { let inserted = insert(&mut save.right, key, value); - skew(&mut save); - split(&mut save); - *node = Some(save); // re-balance, if necessary + skew(save); + split(save); inserted } else { save.key = key; save.value = value; - *node = Some(save); false } + } + None => { + *node = Some(~TreeNode::new(key, value)); + true + } } } From b0f58f6e684ca9076fac99e8f3917d01eeb5775e Mon Sep 17 00:00:00 2001 From: Daniel Micay Date: Sun, 10 Feb 2013 21:11:33 -0500 Subject: [PATCH 3/4] avoid explicit reborrow in heir_swap --- src/libstd/treemap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/treemap.rs b/src/libstd/treemap.rs index 4b02a13583f..c100d8ca85d 100644 --- a/src/libstd/treemap.rs +++ b/src/libstd/treemap.rs @@ -631,7 +631,7 @@ fn remove(node: &mut Option<~TreeNode>, key: &K) -> bool { // *could* be done without recursion, but it won't borrow check do child.mutate |mut child| { if child.right.is_some() { - heir_swap(&mut *node, &mut child.right); + heir_swap(node, &mut child.right); } else { node.key <-> child.key; node.value <-> child.value; From f9c7ba009b51f39629d74ac67781c034643e74e8 Mon Sep 17 00:00:00 2001 From: Daniel Micay Date: Sun, 10 Feb 2013 22:03:26 -0500 Subject: [PATCH 4/4] treemap: cut down on swap_unwrap in remove Performance before: std::treemap::TreeMap sequential_ints 0.083971 s random_ints 0.095861 s delete_ints 0.083931 s sequential_strings 0.278272 s random_strings 0.240286 s delete_strings 0.173581 s Performance after: std::treemap::TreeMap sequential_ints 0.083297 s random_ints 0.097644 s delete_ints 0.052602 s sequential_strings 0.287326 s random_strings 0.242372 s delete_strings 0.142269 s --- src/libstd/treemap.rs | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/libstd/treemap.rs b/src/libstd/treemap.rs index c100d8ca85d..d3583828f9a 100644 --- a/src/libstd/treemap.rs +++ b/src/libstd/treemap.rs @@ -626,7 +626,7 @@ fn insert(node: &mut Option<~TreeNode>, key: K, } fn remove(node: &mut Option<~TreeNode>, key: &K) -> bool { - fn heir_swap(node: &mut TreeNode, + fn heir_swap(node: &mut ~TreeNode, child: &mut Option<~TreeNode>) { // *could* be done without recursion, but it won't borrow check do child.mutate |mut child| { @@ -640,15 +640,15 @@ fn remove(node: &mut Option<~TreeNode>, key: &K) -> bool { } } - if node.is_none() { + match *node { + None => { return false // bottom of tree - } else { - let mut save = node.swap_unwrap(); - - let removed = if save.key < *key { - remove(&mut save.right, key) + } + Some(ref mut save) => { + let (removed, this) = if save.key < *key { + (remove(&mut save.right, key), false) } else if *key < save.key { - remove(&mut save.left, key) + (remove(&mut save.left, key), false) } else { if save.left.is_some() { if save.right.is_some() { @@ -662,16 +662,22 @@ fn remove(node: &mut Option<~TreeNode>, key: &K) -> bool { save.left = Some(left); remove(&mut save.left, key); } else { - save = save.left.swap_unwrap(); + *save = save.left.swap_unwrap(); } + (true, false) } else if save.right.is_some() { - save = save.right.swap_unwrap(); + *save = save.right.swap_unwrap(); + (true, false) } else { - return true // leaf + (true, true) } - true }; + if this { + *node = None; + return true; + } + let left_level = save.left.map_default(0, |x| x.level); let right_level = save.right.map_default(0, |x| x.level); @@ -683,7 +689,7 @@ fn remove(node: &mut Option<~TreeNode>, key: &K) -> bool { do save.right.mutate |mut x| { x.level = save.level; x } } - skew(&mut save); + skew(save); match save.right { Some(ref mut right) => { @@ -696,15 +702,15 @@ fn remove(node: &mut Option<~TreeNode>, key: &K) -> bool { None => () } - split(&mut save); + split(save); match save.right { Some(ref mut x) => { split(x) }, None => () } } - *node = Some(save); removed + } } }