Lift SharedCow::to_mut out of if let branches (#13524)

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 be guaranteed to take the short path in `Arc::into_mut`).
Lifting it get's rid of this.
This commit is contained in:
Stefan Holderbach 2024-08-03 00:26:48 +02:00 committed by GitHub
parent ff1ad77130
commit 63f00e78d1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -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 => {