Public/nushell
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Lift SharedCow::to_mut out of if let branches

Browse Source 
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.
This commit is contained in:
sholderbach 2024-08-03 00:06:38 +02:00
parent af34d5c062
commit 3255eef1ed
1 changed files with 12 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
crates/nu-protocol/src/value/mod.rs
View File

@ -1223,7 +1223,8 @@ impl Value {
for val in vals.iter_mut() { for val in vals.iter_mut() {
match val { match val {
Value::Record { val: record, .. } => { 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())?; val.upsert_data_at_cell_path(path, new_val.clone())?;
} else { } else {
let new_col = if path.is_empty() { let new_col = if path.is_empty() {
@ -1235,7 +1236,7 @@ impl Value {
.upsert_data_at_cell_path(path, new_val.clone())?; .upsert_data_at_cell_path(path, new_val.clone())?;
new_col new_col
}; };
record.to_mut().push(col_name, new_col); record.push(col_name, new_col);
} }
} }
Value::Error { error, .. } => return Err(*error.clone()), Value::Error { error, .. } => return Err(*error.clone()),
@ -1250,7 +1251,8 @@ impl Value {
} }
} }
Value::Record { val: record, .. } => { 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)?; val.upsert_data_at_cell_path(path, new_val)?;
} else { } else {
let new_col = if path.is_empty() { 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.upsert_data_at_cell_path(path, new_val)?;
new_col new_col
}; };
record.to_mut().push(col_name, new_col); record.push(col_name, new_col);
} }
} }
Value::Error { error, .. } => return Err(*error.clone()), Value::Error { error, .. } => return Err(*error.clone()),
@ -1591,7 +1593,8 @@ impl Value {
let v_span = val.span(); let v_span = val.span();
match val { match val {
Value::Record { val: record, .. } => { 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() { if path.is_empty() {
return Err(ShellError::ColumnAlreadyExists { return Err(ShellError::ColumnAlreadyExists {
col_name: col_name.clone(), col_name: col_name.clone(),
@ -1618,7 +1621,7 @@ impl Value {
)?; )?;
new_col new_col
}; };
record.to_mut().push(col_name, new_col); record.push(col_name, new_col);
} }
} }
Value::Error { error, .. } => return Err(*error.clone()), Value::Error { error, .. } => return Err(*error.clone()),
@ -1634,7 +1637,8 @@ impl Value {
} }
} }
Value::Record { val: record, .. } => { 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() { if path.is_empty() {
return Err(ShellError::ColumnAlreadyExists { return Err(ShellError::ColumnAlreadyExists {
col_name: col_name.clone(), 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.insert_data_at_cell_path(path, new_val, head_span)?;
new_col new_col
}; };
record.to_mut().push(col_name, new_col); record.push(col_name, new_col);
} }
} }
other => { other => {