Skip to content
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

Implimented GetSize for Arc<str>, tokio sync and fixed size of the &str #17

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ categories = ["memory-management", "caching"]

[dependencies]
get-size-derive = { version = "^0.1.3", optional = true }
tokio = { version = "^1", optional = true , features = ["sync", "time", "io-util"]}
serde_json = { version = "^1", optional = true}

[features]
default = []
Expand Down
63 changes: 58 additions & 5 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ use std::time::{Instant, Duration, SystemTime};
pub use get_size_derive::*;




#[cfg(test)]
mod tests;

Expand Down Expand Up @@ -140,9 +138,12 @@ impl<T> GetSize for PhantomData<T> {}
impl GetSize for PhantomPinned {}

impl GetSize for Instant {}
impl GetSize for Duration {}
impl GetSize for SystemTime {}

impl GetSize for Duration {
fn get_stack_size() -> usize {
12
}
}
Comment on lines -143 to +146
Copy link

Choose a reason for hiding this comment

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

question: Why make this change for Duration?

It seems like the default implementation based on size_of should be correct.



impl<'a, T> GetSize for Cow<'a, T>
Expand Down Expand Up @@ -334,6 +335,18 @@ impl<T> GetSize for Arc<T> where T: GetSize {
}
}

impl GetSize for Arc<str> {
fn get_size(&self) -> usize {
std::mem::size_of::<usize>() + (&**self).get_size()
}
Comment on lines +339 to +341
Copy link

Choose a reason for hiding this comment

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

suggestion: Shouldn't this be get_heap_size?

I think your

Suggested change
fn get_size(&self) -> usize {
std::mem::size_of::<usize>() + (&**self).get_size()
}
fn get_heap_size(&self) -> usize {
std::mem::size_of::<usize>() + (&**self).get_size()
}

}

impl GetSize for Box<str> {
fn get_size(&self) -> usize {
std::mem::size_of::<usize>() + (&**self).get_size()
}
Comment on lines +345 to +347
Copy link

Choose a reason for hiding this comment

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

suggestion: As above, this should be get_heap_size

Suggested change
fn get_size(&self) -> usize {
std::mem::size_of::<usize>() + (&**self).get_size()
}
fn get_heap_size(&self) -> usize {
std::mem::size_of::<usize>() + (&**self).get_size()
}

}

impl<T> GetSize for Option<T> where T: GetSize {
fn get_heap_size(&self) -> usize {
match self {
Expand Down Expand Up @@ -375,7 +388,11 @@ impl GetSize for String {
}
}

impl GetSize for &str {}
impl GetSize for &str {
fn get_size(&self) -> usize {
std::mem::size_of_val(*self)
}
}

impl GetSize for std::ffi::CString {
fn get_heap_size(&self) -> usize {
Expand Down Expand Up @@ -448,3 +465,39 @@ impl<T> GetSize for Box<[T]> {
total
}
}

#[cfg(feature = "tokio")]
impl<T> GetSize for tokio::sync::mpsc::UnboundedSender<T> {}

#[cfg(feature = "tokio")]
impl<T> GetSize for tokio::sync::mpsc::WeakUnboundedSender<T> {}

#[cfg(feature = "tokio")]
impl GetSize for tokio::time::Instant {}

#[cfg(feature = "tokio")]
impl GetSize for tokio::time::Interval {
fn get_size(&self) -> usize {
std::mem::size_of_val(self)
}
}

#[cfg(feature = "tokio")]
impl <T: tokio::io::AsyncRead> GetSize for tokio::io::BufReader<T> {
fn get_size(&self) -> usize {
let inner = std::mem::size_of_val(self.get_ref());
let buf = self.buffer().len();
let pos = 1_usize;
let cap = 1_usize;
let seek_state = 9_usize;

inner + buf + pos + cap + seek_state
}
}

#[cfg(feature = "serde_json")]
impl GetSize for serde_json::Value {
fn get_size(&self) -> usize {
std::mem::size_of_val(self)
}
}
14 changes: 14 additions & 0 deletions src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,3 +244,17 @@ fn derive_newtype() {
let test = TestNewType(0);
assert_eq!(u64::get_stack_size(), test.get_size());
}

#[test]
fn test_arc_str_size(){
let str_text = "a";
let arc: Arc<str> = Arc::from(str_text);
assert_eq!(arc.get_size(), std::mem::size_of::<usize>() + std::mem::size_of_val(str_text));
Copy link

Choose a reason for hiding this comment

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

suggestion: This is incorrect.

The total size should include the stack size of the Arc itself. In total it needs to store the 1 byte for the "a" plus the count (a usize) plus the fat pointer, which would be another two usizes worth of space.

}

#[test]
fn test_boxed_str_size() {
let str_text = "a";
let boxed: Box<str> = Box::from(str_text);
assert_eq!(boxed.get_size(), std::mem::size_of::<usize>() + std::mem::size_of_val(str_text));
}