From ce581a80a66f4b3acac9eb8c2a2340896bc3fb81 Mon Sep 17 00:00:00 2001 From: Stefan Holderbach Date: Sat, 30 Mar 2024 14:03:31 +0100 Subject: [PATCH] Elide clone in `V::follow_cell_path` for record (#12325) # Description This clone is not necessary and tanks the performance of deep nested access. As soon as we found the value, we know we discard the old value, so can `std::mem::take` the inner (`impl Default for Value` to the rescue) We may be able to further optimize this but not having to clone the value is vital. --- crates/nu-protocol/src/value/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 473221a721..385a2b0681 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -1160,16 +1160,16 @@ impl Value { let span = current.span(); match current { - Value::Record { val, .. } => { + Value::Record { mut val, .. } => { // Make reverse iterate to avoid duplicate column leads to first value, actually last value is expected. - if let Some(found) = val.iter().rev().find(|x| { + if let Some(found) = val.iter_mut().rev().find(|x| { if insensitive { x.0.eq_ignore_case(column_name) } else { x.0 == column_name } }) { - current = found.1.clone(); // TODO: avoid clone here + current = std::mem::take(found.1); } else if *optional { return Ok(Value::nothing(*origin_span)); // short-circuit } else if let Some(suggestion) = @@ -1222,15 +1222,15 @@ impl Value { .map(|val| { let val_span = val.span(); match val { - Value::Record { val, .. } => { - if let Some(found) = val.iter().rev().find(|x| { + Value::Record { mut val, .. } => { + if let Some(found) = val.iter_mut().rev().find(|x| { if insensitive { x.0.eq_ignore_case(column_name) } else { x.0 == column_name } }) { - Ok(found.1.clone()) // TODO: avoid clone here + Ok(std::mem::take(found.1)) } else if *optional { Ok(Value::nothing(*origin_span)) } else if let Some(suggestion) =