Skip to content

refactor(core, protocol): extract memory diff helper and add clarity comments - #163

Merged
kacy merged 1 commit into
mainfrom
refactor/idiomatic-rust-pass-pr1
Feb 17, 2026
Merged

refactor(core, protocol): extract memory diff helper and add clarity comments#163
kacy merged 1 commit into
mainfrom
refactor/idiomatic-rust-pass-pr1

Conversation

@kacy

@kacy kacy commented Feb 17, 2026

Copy link
Copy Markdown
Owner

summary

this pass targets two areas flagged in a readability review of emberkv-core and ember-protocol.

ConcurrentKeyspace — memory diff helper

the signed-diff atomic update pattern appeared four times in set, incr_by, incr_by_float, and append:

let diff = new_size as isize - old_size as isize;
if diff > 0 {
    self.memory_used.fetch_add(diff as usize, ...);
} else if diff < 0 {
    self.memory_used.fetch_sub((-diff) as usize, ...);
}

extracted into fn adjust_memory(&self, old_size: usize, new_size: usize). no logic change — same atomics, same ordering, same behavior on diff == 0 (no-op).

keyspace.rs — clone comment

get() returns Ok(Some(e.value.clone())) where the value is Value::String(Bytes). added a brief inline note that this is a cheap refcount increment so readers don't mistake it for a deep copy.

command.rscommand_name() comment

the doc comment now explains why 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.

what was tested

  • cargo build -p emberkv-core -p ember-protocol — clean
  • cargo test -p emberkv-core -p ember-protocol — 330 tests pass, 0 failures
  • cargo fmt --check -p emberkv-core -p ember-protocol — no formatting issues

…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)
@kacy
kacy merged commit 5450c50 into main Feb 17, 2026
7 checks passed
@kacy
kacy deleted the refactor/idiomatic-rust-pass-pr1 branch February 17, 2026 23:23
kacy added a commit that referenced this pull request Feb 19, 2026
…comments (#163)

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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant