Skip to content

Fix MEMORY USAGE command#5

Open
tomerqodo wants to merge 2 commits intocoderabbit_full_base_fix_memory_usage_command_pr5from
coderabbit_full_head_fix_memory_usage_command_pr5
Open

Fix MEMORY USAGE command#5
tomerqodo wants to merge 2 commits intocoderabbit_full_base_fix_memory_usage_command_pr5from
coderabbit_full_head_fix_memory_usage_command_pr5

Conversation

@tomerqodo
Copy link
Copy Markdown

@tomerqodo tomerqodo commented Jan 30, 2026

Benchmark PR from agentic-review-benchmarks#5

Summary by CodeRabbit

Release Notes

  • Tests
    • Added comprehensive test coverage for memory usage calculations across string encodings, validating memory bounds for embedded strings, raw strings, and integer-encoded keys.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

The changes refactor memory size estimation functionality from robj-based to kvobj-based computation. A new function kvobjComputeSize is introduced with updated accounting logic for various object encodings. The OBJECT USAGE invocation path is updated to use this new function. Test comments are updated to reflect the new function name, and a new test validates memory usage behavior for string-encoded keys.

Changes

Cohort / File(s) Summary
Core Size Computation
src/object.c
Introduced kvobjComputeSize function for kvobj-based size estimation, removing explicit robj header accounting in favor of malloc_usable_size and incremental asize accumulation. Updated OBJECT USAGE command to call the new function instead of the original objectComputeSize.
Test Comment Updates
tests/unit/type/hash.tcl, tests/unit/type/list.tcl
Updated test comments from "coverage for objectComputeSize" to "coverage for kvobjComputeSize" to reflect the refactored function name. No functional changes to test logic.
String Memory Usage Tests
tests/unit/type/string.tcl
Added new "MEMORY USAGE - STRINGS" test block that validates memory usage for various key and value sizes, covering EMBSTR/RAW and OBJ_ENCODING_INT encodings with explicit bounds assertions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A kvobj takes the stage so new,
where malloc's magic makes sizes true.
From robj's chains we're breaking free,
accounting every byte we see.
Hop along, the code's redrawn!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main objective of the PR, which is to fix the MEMORY USAGE command by refactoring it to use the new kvobj-based computation function.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch coderabbit_full_head_fix_memory_usage_command_pr5

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

2 participants