-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add differential memory usage tracking #51
Open
michaelsproul
wants to merge
7
commits into
main
Choose a base branch
from
mem-usage
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
9b2f055
Add differential memory usage tracking
michaelsproul 6dcf56e
Track total
michaelsproul 07a6a7f
Revert "Track total"
michaelsproul 2b54b1e
Fix bugs and test using DHAT
michaelsproul 3bfde1b
Docs
michaelsproul b13b9e3
Use capacity rather than length for packed leaf
michaelsproul 7b42785
Abstract over UpdateMap more completely
michaelsproul File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
use crate::{Arc, List, Tree, UpdateMap, Value, Vector}; | ||
use std::collections::HashMap; | ||
use typenum::Unsigned; | ||
|
||
pub trait MemorySize { | ||
/// The memory address of this item. | ||
fn self_pointer(&self) -> usize; | ||
|
||
/// Subtrees (Arcs) for this type's fields that consume memory. | ||
fn subtrees(&self) -> Vec<&dyn MemorySize>; | ||
|
||
/// Memory consumed by this type's non-recursive fields. | ||
fn intrinsic_size(&self) -> usize; | ||
} | ||
|
||
/// Memory usage (RAM) analysis for Milhouse data structures. | ||
#[derive(Default)] | ||
pub struct MemoryTracker { | ||
// Map from pointer to size of subtree referenced by that pointer. | ||
subtree_sizes: HashMap<usize, usize>, | ||
} | ||
|
||
#[derive(Debug)] | ||
pub struct ItemStats { | ||
/// Total size of this item ignorning structural sharing. | ||
pub total_size: usize, | ||
/// Amount of memory used by this item in addition to memory that was already tracked. | ||
pub differential_size: usize, | ||
} | ||
|
||
impl MemoryTracker { | ||
pub fn track_item<T: MemorySize + ?Sized>(&mut self, item: &T) -> ItemStats { | ||
let ptr = item.self_pointer(); | ||
|
||
// If this item is already tracked, then its differential size is 0. | ||
if let Some(&total_size) = self.subtree_sizes.get(&ptr) { | ||
return ItemStats { | ||
total_size, | ||
differential_size: 0, | ||
}; | ||
} | ||
|
||
// Otherwise, calculate the intrinsic size of this item, and recurse into its subtrees. | ||
let intrinsic_size = item.intrinsic_size(); | ||
|
||
let subtrees = item.subtrees(); | ||
|
||
let mut total_size = intrinsic_size; | ||
let mut differential_size = intrinsic_size; | ||
|
||
for subtree in subtrees { | ||
let subtree_stats = self.track_item(subtree); | ||
total_size += subtree_stats.total_size; | ||
differential_size += subtree_stats.differential_size; | ||
} | ||
|
||
self.subtree_sizes.insert(ptr, total_size); | ||
|
||
ItemStats { | ||
total_size, | ||
differential_size, | ||
} | ||
} | ||
} | ||
|
||
impl<T: Value> MemorySize for Arc<Tree<T>> { | ||
fn self_pointer(&self) -> usize { | ||
self.as_ptr() as usize | ||
} | ||
|
||
fn subtrees(&self) -> Vec<&dyn MemorySize> { | ||
match &**self { | ||
Tree::Leaf(_) | Tree::PackedLeaf(_) | Tree::Zero(_) => vec![], | ||
Tree::Node { left, right, .. } => { | ||
vec![left, right] | ||
} | ||
} | ||
} | ||
|
||
fn intrinsic_size(&self) -> usize { | ||
let leaf_size = match &**self { | ||
// This is the T allocated behind the Arc in `Leaf::value`. | ||
Tree::Leaf(_) => std::mem::size_of::<T>(), | ||
// This is the Vec<T> allocated inside `PackedLeaf::values`. | ||
Tree::PackedLeaf(packed) => packed.values.capacity() * std::mem::size_of::<T>(), | ||
Tree::Node { .. } | Tree::Zero(..) => 0, | ||
}; | ||
std::mem::size_of::<Self>() + std::mem::size_of::<Tree<T>>() + leaf_size | ||
} | ||
} | ||
|
||
impl<T: Value, N: Unsigned, U: UpdateMap<T>> MemorySize for List<T, N, U> { | ||
fn self_pointer(&self) -> usize { | ||
self as *const _ as usize | ||
} | ||
|
||
fn subtrees(&self) -> Vec<&dyn MemorySize> { | ||
vec![&self.interface.backing.tree] | ||
} | ||
|
||
fn intrinsic_size(&self) -> usize { | ||
// This approximates the size of the UpdateMap, and assumes that `T` is not recursive. | ||
// We could probably add a `T: MemorySize` bound? In most practical cases the update map | ||
// should be empty anyway. | ||
std::mem::size_of::<Self>() + self.interface.updates.len() * std::mem::size_of::<T>() | ||
} | ||
} | ||
|
||
impl<T: Value, N: Unsigned, U: UpdateMap<T>> MemorySize for Vector<T, N, U> { | ||
fn self_pointer(&self) -> usize { | ||
self as *const _ as usize | ||
} | ||
|
||
fn subtrees(&self) -> Vec<&dyn MemorySize> { | ||
vec![&self.interface.backing.tree] | ||
} | ||
|
||
fn intrinsic_size(&self) -> usize { | ||
// This approximates the size of the UpdateMap, and assumes that `T` is not recursive. | ||
// We could probably add a `T: MemorySize` bound? In most practical cases the update map | ||
// should be empty anyway. | ||
std::mem::size_of::<Self>() + self.interface.updates.len() * std::mem::size_of::<T>() | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
use crate::{mem::MemoryTracker, Vector}; | ||
use typenum::U1024; | ||
|
||
#[test] | ||
fn vector_mutate_last() { | ||
let v1 = Vector::<u64, U1024>::new(vec![1; 1024]).unwrap(); | ||
let mut v2 = v1.clone(); | ||
*v2.get_mut(1023).unwrap() = 2; | ||
v2.apply_updates().unwrap(); | ||
|
||
let mut tracker = MemoryTracker::default(); | ||
let v1_stats = tracker.track_item(&v1); | ||
let v2_stats = tracker.track_item(&v2); | ||
|
||
// Total size is equal. | ||
assert_eq!(v1_stats.total_size, v2_stats.total_size); | ||
|
||
// Differential size for v1 is equal to its total size (nothing to diff against). | ||
assert_eq!(v1_stats.total_size, v1_stats.differential_size); | ||
|
||
// The differential size of the second list should be less than 2% of the total size. | ||
assert!(50 * v2_stats.differential_size < v2_stats.total_size); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
|
||
mod builder; | ||
mod iterator; | ||
mod mem; | ||
mod packed; | ||
mod pop_front; | ||
mod proptest; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
use milhouse::{mem::MemoryTracker, List}; | ||
use typenum::U1024; | ||
|
||
#[global_allocator] | ||
static ALLOC: dhat::Alloc = dhat::Alloc; | ||
|
||
#[test] | ||
fn memory_tracker_accuracy() { | ||
let _profiler = dhat::Profiler::builder().testing().build(); | ||
|
||
// Take a snapshot at the start so we can ignore "background allocations" from e.g. the test | ||
// runner and the process starting up. | ||
let pre_stats = dhat::HeapStats::get(); | ||
|
||
// We box the list because the MemorySize implementation for List includes the fields of the | ||
// list, and we want to allocate them on the heap so that they are visible to DHAT. | ||
let list = Box::new(List::<u64, U1024>::new(vec![1; 1024]).unwrap()); | ||
|
||
// Calculate the size of the list using Milhouse tools, and then drop the tracker so it isn't | ||
// consuming any heap space (which would interfere with our measurements). | ||
let mut mem_tracker = MemoryTracker::default(); | ||
let stats = mem_tracker.track_item(&*list); | ||
drop(mem_tracker); | ||
|
||
// Calculate total size according to DHAT by subtracting the starting allocations from the | ||
// current amount allocated. | ||
let post_stats = dhat::HeapStats::get(); | ||
let dhat_total_size = post_stats.curr_bytes - pre_stats.curr_bytes; | ||
|
||
dhat::assert_eq!(dhat_total_size, stats.total_size); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Really neat test! Have you tried nested Lists?
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.
no, because I know it won't work 😳