From 3255eef1ed041b856e3456962841350234a23145 Mon Sep 17 00:00:00 2001
From: sholderbach <sholderbach@users.noreply.github.com>
Date: Sat, 3 Aug 2024 00:06:38 +0200
Subject: [PATCH] Lift `SharedCow::to_mut` out of if let branches

In some `if let`s we ran the `SharedCow::to_mut` for the test and to get
access to a mutable reference in the happy path. Internally
`Arc::into_mut` has to read atomics and if necessary clone.
For else branches, where we still want to modify the record we
previously called this again (not just in rust, confirmed in the asm).
This would have introduced a `call` instruction and its cost (even if it
would take the short path in `Arc::into_mut`). Lifting it get's rid of
this.
---
 crates/nu-protocol/src/value/mod.rs | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs
index fc618d6341..edd51d70a6 100644
--- a/crates/nu-protocol/src/value/mod.rs
+++ b/crates/nu-protocol/src/value/mod.rs
@@ -1223,7 +1223,8 @@ impl Value {
                         for val in vals.iter_mut() {
                             match val {
                                 Value::Record { val: record, .. } => {
-                                    if let Some(val) = record.to_mut().get_mut(col_name) {
+                                    let record = record.to_mut();
+                                    if let Some(val) = record.get_mut(col_name) {
                                         val.upsert_data_at_cell_path(path, new_val.clone())?;
                                     } else {
                                         let new_col = if path.is_empty() {
@@ -1235,7 +1236,7 @@ impl Value {
                                                 .upsert_data_at_cell_path(path, new_val.clone())?;
                                             new_col
                                         };
-                                        record.to_mut().push(col_name, new_col);
+                                        record.push(col_name, new_col);
                                     }
                                 }
                                 Value::Error { error, .. } => return Err(*error.clone()),
@@ -1250,7 +1251,8 @@ impl Value {
                         }
                     }
                     Value::Record { val: record, .. } => {
-                        if let Some(val) = record.to_mut().get_mut(col_name) {
+                        let record = record.to_mut();
+                        if let Some(val) = record.get_mut(col_name) {
                             val.upsert_data_at_cell_path(path, new_val)?;
                         } else {
                             let new_col = if path.is_empty() {
@@ -1260,7 +1262,7 @@ impl Value {
                                 new_col.upsert_data_at_cell_path(path, new_val)?;
                                 new_col
                             };
-                            record.to_mut().push(col_name, new_col);
+                            record.push(col_name, new_col);
                         }
                     }
                     Value::Error { error, .. } => return Err(*error.clone()),
@@ -1591,7 +1593,8 @@ impl Value {
                             let v_span = val.span();
                             match val {
                                 Value::Record { val: record, .. } => {
-                                    if let Some(val) = record.to_mut().get_mut(col_name) {
+                                    let record = record.to_mut();
+                                    if let Some(val) = record.get_mut(col_name) {
                                         if path.is_empty() {
                                             return Err(ShellError::ColumnAlreadyExists {
                                                 col_name: col_name.clone(),
@@ -1618,7 +1621,7 @@ impl Value {
                                             )?;
                                             new_col
                                         };
-                                        record.to_mut().push(col_name, new_col);
+                                        record.push(col_name, new_col);
                                     }
                                 }
                                 Value::Error { error, .. } => return Err(*error.clone()),
@@ -1634,7 +1637,8 @@ impl Value {
                         }
                     }
                     Value::Record { val: record, .. } => {
-                        if let Some(val) = record.to_mut().get_mut(col_name) {
+                        let record = record.to_mut();
+                        if let Some(val) = record.get_mut(col_name) {
                             if path.is_empty() {
                                 return Err(ShellError::ColumnAlreadyExists {
                                     col_name: col_name.clone(),
@@ -1652,7 +1656,7 @@ impl Value {
                                 new_col.insert_data_at_cell_path(path, new_val, head_span)?;
                                 new_col
                             };
-                            record.to_mut().push(col_name, new_col);
+                            record.push(col_name, new_col);
                         }
                     }
                     other => {