Skip to content

Conversation

@maksymar
Copy link
Contributor

@maksymar maksymar commented Aug 19, 2025

This PR provides a better reclaim_memory name to the public API method.

API Improvement:

  • Rename release_virtual_memory_buckets()reclaim_memory() for better semantics
  • Return page count (u64) instead of bucket count (usize) for user-friendly values
// Before
let bucket_count = memory_manager.release_virtual_memory_buckets(memory_id);

// After  
let pages_reclaimed = memory_manager.reclaim_memory(memory_id);

Documentation:

  • Add comprehensive memory reclamation section to README and docs
  • Show migration scenarios with/without reclamation (2x → 1x memory usage)
  • Strengthen memory isolation warnings with critical formatting
  • Update examples for consistency (map_a/map_b, u8 values)

@github-actions
Copy link

github-actions bot commented Aug 19, 2025

canbench 🏋 (dir: ./benchmarks/io_chunks) 5a80cd8 2025-08-19 12:30:53 UTC

./benchmarks/io_chunks/canbench_results.yml is up to date
📦 canbench_results_io_chunks.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 18 | regressed 0 | improved 0 | new 0 | unchanged 18]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 18 | regressed 0 | improved 0 | new 0 | unchanged 18]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 18 | regressed 0 | improved 0 | new 0 | unchanged 18]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

@github-actions
Copy link

github-actions bot commented Aug 19, 2025

canbench 🏋 (dir: ./benchmarks/nns) 5a80cd8 2025-08-19 12:30:14 UTC

./benchmarks/nns/canbench_results.yml is up to date
📦 canbench_results_nns.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 16 | regressed 0 | improved 0 | new 0 | unchanged 16]
    change:   [max +64 | p75 +64 | median -46 | p25 -525.07K | min -8.27M]
    change %: [max +1.13% | p75 0.00% | median -0.04% | p25 -0.19% | min -0.23%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 16 | regressed 0 | improved 0 | new 0 | unchanged 16]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 16 | regressed 0 | improved 0 | new 0 | unchanged 16]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

@github-actions
Copy link

github-actions bot commented Aug 19, 2025

canbench 🏋 (dir: ./benchmarks/vec) 5a80cd8 2025-08-19 12:30:13 UTC

./benchmarks/vec/canbench_results.yml is up to date
📦 canbench_results_vec.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 16 | regressed 0 | improved 0 | new 0 | unchanged 16]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 16 | regressed 0 | improved 0 | new 0 | unchanged 16]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 16 | regressed 0 | improved 0 | new 0 | unchanged 16]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

@github-actions
Copy link

github-actions bot commented Aug 19, 2025

canbench 🏋 (dir: ./benchmarks/btreemap) 5a80cd8 2025-08-19 12:31:48 UTC

./benchmarks/btreemap/canbench_results.yml is up to date
📦 canbench_results_btreemap.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 303 | regressed 0 | improved 0 | new 0 | unchanged 303]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 303 | regressed 0 | improved 0 | new 0 | unchanged 303]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 303 | regressed 0 | improved 0 | new 0 | unchanged 303]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

@github-actions
Copy link

github-actions bot commented Aug 19, 2025

canbench 🏋 (dir: ./benchmarks/btreeset) 5a80cd8 2025-08-19 12:30:13 UTC

./benchmarks/btreeset/canbench_results.yml is up to date
📦 canbench_results_btreeset.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 100 | regressed 0 | improved 0 | new 0 | unchanged 100]
    change:   [max +1.03M | p75 +36 | median +36 | p25 0 | min 0]
    change %: [max +0.13% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 100 | regressed 0 | improved 0 | new 0 | unchanged 100]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 100 | regressed 0 | improved 0 | new 0 | unchanged 100]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

@github-actions
Copy link

github-actions bot commented Aug 19, 2025

canbench 🏋 (dir: ./benchmarks/memory_manager) 5a80cd8 2025-08-19 12:30:02 UTC

./benchmarks/memory_manager/canbench_results.yml is up to date
📦 canbench_results_memory-manager.csv available in artifacts

---------------------------------------------------

Summary:
  instructions:
    status:   No significant changes 👍
    counts:   [total 3 | regressed 0 | improved 0 | new 0 | unchanged 3]
    change:   [max +320.00K | p75 +160.76K | median +1.52K | p25 +760 | min 0]
    change %: [max +0.09% | p75 +0.05% | median 0.00% | p25 0.00% | min 0.00%]

  heap_increase:
    status:   No significant changes 👍
    counts:   [total 3 | regressed 0 | improved 0 | new 0 | unchanged 3]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

  stable_memory_increase:
    status:   No significant changes 👍
    counts:   [total 3 | regressed 0 | improved 0 | new 0 | unchanged 3]
    change:   [max 0 | p75 0 | median 0 | p25 0 | min 0]
    change %: [max 0.00% | p75 0.00% | median 0.00% | p25 0.00% | min 0.00%]

---------------------------------------------------
CSV results saved to canbench_results.csv

@maksymar maksymar changed the title docs: Maksym/docs docs: use reclaim_memory() name and update docs accordingly Aug 19, 2025
@maksymar maksymar requested a review from Copilot August 19, 2025 11:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves the memory management API by renaming release_virtual_memory_buckets() to reclaim_memory() and changing its return value from bucket count to page count for better user experience. The changes include comprehensive documentation updates across README and docs to demonstrate memory reclamation scenarios and strengthen memory isolation warnings.

  • Renamed API method for better semantics and user-friendly return values
  • Added comprehensive memory reclamation documentation with migration examples
  • Updated all test files and comments to use consistent "memory reclamation" terminology

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/memory_manager.rs Core API change: renamed method and updated return type from bucket count to page count
src/memory_manager/memory_reclaim_*_tests.rs Updated test files to use new method name and terminology throughout
src/btreemap.rs Updated safety documentation to reference new method name
src/btreeset.rs Updated safety documentation to reference new method name
docs/src/concepts/memory-manager.md Added comprehensive memory reclamation section with migration scenarios
README.md Enhanced with memory reclamation examples and improved formatting

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@maksymar maksymar marked this pull request as ready for review August 19, 2025 11:07
@maksymar maksymar requested a review from a team as a code owner August 19, 2025 11:07
@maksymar maksymar enabled auto-merge (squash) August 19, 2025 12:02
@maksymar maksymar merged commit d1fde89 into main Aug 19, 2025
17 checks passed
@maksymar maksymar deleted the maksym/docs branch August 19, 2025 12:33
maksymar added a commit that referenced this pull request Aug 25, 2025
maksymar added a commit that referenced this pull request Aug 25, 2025
This PR reverts adding `reclaim_memory` method.

Reverts several recent commits in a single change:

- `6e397bd` fix: use conservative bucket reuse that survives reload #394
- `00468e1` docs: update memory reclamation examples in the docs  #392 
- `b911479` docs: cleanup documentation  #391 
- `d1fde89` docs: use reclaim_memory() name and update docs accordingly
#388
- `a18917b` docs: add safety documentation and tests for manual bucket
release #387
- `73e96e8` feat: add manual bucket release to prevent memory waste #386

This PR restores the codebase to the state before these commits.

Done with `git revert -n 6e397bd 00468e1 b911479 d1fde89 a18917b
73e96e8`

The reason for reverting this approach was that it can reclaim unused
memory in theory but provides little benefit in real-world migrations.
All due to the requirement to keep buckets in ascending order in each
VM.

Example 1: Reuse works
```
A allocates: [0, 4, 5]
B allocates: [1, 2, 3]
A frees: [0, 4, 5]
B grows: can reuse bucket 4 (since 4 > max(B) = 3)
B after grow: [1, 2, 3, 4]
```
Example 2: Reuse fails
```
A allocates: [0, 1, 2]
B allocates: [4, 5, 6]
A frees: [0, 1, 2]
B grows: cannot reuse any freed bucket (all < max(B) = 6), so allocates new bucket 7
B after grow: [4, 5, 6, 7]
```

In real life when migrating state A to state B, state B created after
state A grown, so it's first bucket ID is already higher than any free
bucket in state A virtual memory, therefore can not be reused.
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