Skip to content

Commit 7105c15

Browse files
committed
graph: Avoid unnecessary entity deep clone in as_modifications
Use Arc::try_unwrap to move the entity out of the Arc without cloning when the refcount is 1 (the common case after remove() from the LFU cache). Also make merge_remove_null_fields return whether it changed anything, replacing the post-merge full entity comparison.
1 parent b28a337 commit 7105c15

File tree

2 files changed

+21
-8
lines changed

2 files changed

+21
-8
lines changed

graph/src/components/store/entity_cache.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -518,12 +518,14 @@ impl EntityCache {
518518
}
519519
// Entity may have been changed
520520
(Some(current), EntityOp::Update(updates)) => {
521-
let mut data = current.as_ref().clone();
522-
data.merge_remove_null_fields(updates)
521+
let mut data =
522+
Arc::try_unwrap(current).unwrap_or_else(|arc| arc.as_ref().clone());
523+
let changed = data
524+
.merge_remove_null_fields(updates)
523525
.map_err(|e| key.unknown_attribute(e))?;
524526
let data = Arc::new(data);
525527
self.current.insert(key.clone(), Some(data.cheap_clone()));
526-
if current != data {
528+
if changed {
527529
Some(Overwrite {
528530
key,
529531
data,

graph/src/data/store/mod.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,14 +1014,25 @@ impl Entity {
10141014
/// If a key exists in both entities, the value from `update` is chosen.
10151015
/// If a key only exists on one entity, the value from that entity is chosen.
10161016
/// If a key is set to `Value::Null` in `update`, the key/value pair is removed.
1017-
pub fn merge_remove_null_fields(&mut self, update: Entity) -> Result<(), InternError> {
1017+
pub fn merge_remove_null_fields(&mut self, update: Entity) -> Result<bool, InternError> {
1018+
let mut changed = false;
10181019
for (key, value) in update.0.into_iter() {
10191020
match value {
1020-
Value::Null => self.0.remove(&key),
1021-
_ => self.0.insert(&key, value)?,
1022-
};
1021+
Value::Null => {
1022+
if self.0.remove(&key).is_some() {
1023+
changed = true;
1024+
}
1025+
}
1026+
_ => {
1027+
let different = self.0.get(key.as_str()).is_none_or(|old| *old != value);
1028+
self.0.insert(&key, value)?;
1029+
if different {
1030+
changed = true;
1031+
}
1032+
}
1033+
}
10231034
}
1024-
Ok(())
1035+
Ok(changed)
10251036
}
10261037

10271038
/// Remove all entries with value `Value::Null` from `self`

0 commit comments

Comments
 (0)