From 82c62f06c7b48230ea612b439e87c7a2fee57e0e Mon Sep 17 00:00:00 2001 From: Kacy Fortner Date: Tue, 17 Feb 2026 18:21:18 -0500 Subject: [PATCH] refactor(core, protocol): extract memory diff helper and add clarity comments extract `adjust_memory` helper in `ConcurrentKeyspace` to replace four copies of the signed-diff atomic update pattern in set, incr_by, incr_by_float, and append. add inline comments: - keyspace.rs: note that Value::String clone is a cheap Bytes refcount increment - command.rs: explain why command_name() uses an explicit match instead of a derive macro (the string mappings are the thing worth seeing at a glance) --- crates/ember-core/src/concurrent.rs | 53 +++++++++++----------------- crates/ember-core/src/keyspace.rs | 1 + crates/ember-protocol/src/command.rs | 4 +++ 3 files changed, 25 insertions(+), 33 deletions(-) diff --git a/crates/ember-core/src/concurrent.rs b/crates/ember-core/src/concurrent.rs index cec23a87..8f2cd90a 100644 --- a/crates/ember-core/src/concurrent.rs +++ b/crates/ember-core/src/concurrent.rs @@ -150,15 +150,8 @@ impl ConcurrentKeyspace { // Update memory tracking if let Some(old) = self.data.insert(key.clone(), entry) { - // Replace: adjust memory - let old_size = old.size(key.len()); - let diff = entry_size as isize - old_size as isize; - if diff > 0 { - self.memory_used.fetch_add(diff as usize, Ordering::Relaxed); - } else { - self.memory_used - .fetch_sub((-diff) as usize, Ordering::Relaxed); - } + // Replace: adjust memory for the size difference + self.adjust_memory(old.size(key.len()), entry_size); } else { self.memory_used.fetch_add(entry_size, Ordering::Relaxed); } @@ -255,14 +248,7 @@ impl ConcurrentKeyspace { let key_len = entry.key().len(); let old_size = entry.size(key_len); entry.value = new_bytes; - let new_size = entry.size(key_len); - let diff = new_size as isize - old_size as isize; - if diff > 0 { - self.memory_used.fetch_add(diff as usize, Ordering::Relaxed); - } else if diff < 0 { - self.memory_used - .fetch_sub((-diff) as usize, Ordering::Relaxed); - } + self.adjust_memory(old_size, entry.size(key_len)); return Ok(new_val); } } @@ -301,14 +287,7 @@ impl ConcurrentKeyspace { let key_len = entry.key().len(); let old_size = entry.size(key_len); entry.value = new_bytes; - let new_size = entry.size(key_len); - let diff = new_size as isize - old_size as isize; - if diff > 0 { - self.memory_used.fetch_add(diff as usize, Ordering::Relaxed); - } else if diff < 0 { - self.memory_used - .fetch_sub((-diff) as usize, Ordering::Relaxed); - } + self.adjust_memory(old_size, entry.size(key_len)); return Ok(new_val); } } @@ -337,14 +316,7 @@ impl ConcurrentKeyspace { let key_len = entry.key().len(); let old_size = entry.size(key_len); entry.value = Bytes::from(new_data); - let new_size = entry.size(key_len); - let diff = new_size as isize - old_size as isize; - if diff > 0 { - self.memory_used.fetch_add(diff as usize, Ordering::Relaxed); - } else if diff < 0 { - self.memory_used - .fetch_sub((-diff) as usize, Ordering::Relaxed); - } + self.adjust_memory(old_size, entry.size(key_len)); return new_len; } // expired — remove and fall through to create @@ -535,6 +507,21 @@ impl ConcurrentKeyspace { self.memory_used.store(0, Ordering::Relaxed); } + /// Adjusts `memory_used` after an in-place value replacement. + /// + /// Computes the signed difference between old and new sizes and applies it + /// atomically. Called wherever a key's value changes without removing the key. + #[inline] + fn adjust_memory(&self, old_size: usize, new_size: usize) { + let diff = new_size as isize - old_size as isize; + if diff > 0 { + self.memory_used.fetch_add(diff as usize, Ordering::Relaxed); + } else if diff < 0 { + self.memory_used + .fetch_sub((-diff) as usize, Ordering::Relaxed); + } + } + /// Simple eviction: remove approximately `needed` bytes worth of entries. fn evict_entries(&self, needed: usize) { let mut freed = 0usize; diff --git a/crates/ember-core/src/keyspace.rs b/crates/ember-core/src/keyspace.rs index a2da8eca..6b4c531e 100644 --- a/crates/ember-core/src/keyspace.rs +++ b/crates/ember-core/src/keyspace.rs @@ -435,6 +435,7 @@ impl Keyspace { Some(e) => match &e.value { Value::String(_) => { e.touch(); + // Value::String wraps Bytes — clone is a cheap refcount increment. Ok(Some(e.value.clone())) } _ => Err(WrongType), diff --git a/crates/ember-protocol/src/command.rs b/crates/ember-protocol/src/command.rs index 3238e164..a0480965 100644 --- a/crates/ember-protocol/src/command.rs +++ b/crates/ember-protocol/src/command.rs @@ -487,6 +487,10 @@ impl Command { /// /// Used for metrics labels and slow log entries. Zero allocation — /// returns a `&'static str` for every known variant. + /// + /// The match is explicit rather than derive-generated: a proc macro would + /// obscure the string mappings, which are the thing most worth seeing at + /// a glance when auditing command names. pub fn command_name(&self) -> &'static str { match self { Command::Ping(_) => "ping",