-
Notifications
You must be signed in to change notification settings - Fork 33
feat: add manual bucket release to prevent memory waste #386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
|
|
|
|
74613ac
to
641624a
Compare
641624a
to
acb1eb6
Compare
Co-authored-by: Copilot <[email protected]>
…ble-structures into maksym/free-buckets-on-clear
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this 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 introduces manual bucket release functionality to the MemoryManager to prevent memory waste during data migration scenarios. When stable structures like BTreeMap or Vec are cleared, their allocated memory buckets remain tied to the memory ID and cannot be reused by other memories, leading to memory growth.
- Adds
release_virtual_memory_buckets()
method to manually reclaim buckets after clearing structures - Implements bucket reuse pool to allow freed buckets to be allocated to other memories
- Provides comprehensive test coverage demonstrating migration scenarios and bucket reuse efficiency
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
src/memory_manager/bucket_release_tests.rs | Comprehensive test suite validating bucket release functionality and migration scenarios |
src/memory_manager.rs | Core implementation of bucket release mechanism and free bucket pool management |
benchmarks/io_chunks/canbench_results.yml | Performance benchmark results showing minimal instruction count changes |
benchmarks/btreemap/canbench_results.yml | BTreeMap benchmark results demonstrating performance impact |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This PR adds
release_virtual_memory_buckets()
to enable manual bucket reclamation when stable structures (BTreeMap
,Vec
) are cleared.Problem: Stable structures and MemoryManager operate independently - the memory manager allocates buckets but doesn't know when structures are cleared. When migrating data (A→B), clearing structure A doesn't automatically release its memory buckets, preventing structure B from reusing them and causing memory waste.
Solution: After clearing a stable structure, manually call
release_virtual_memory_buckets(memory_id)
to release its buckets for reuse by other structures.Safety: Users must ensure the structure is fully cleared before releasing buckets. The memory manager provides no safety checks - releasing active buckets causes memory corruption.
Includes comprehensive tests demonstrating migration scenarios and verifying correct implementation.