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

Added mem-dbg as optional feature #19

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

LucaCappelletti94
Copy link

Mem-dbg is a crate that allows to compute the size of a struct. I have added the derives through the crate as an optional feature, so as to use it to compare this implementation with others easily.

Cheers!

@LucaCappelletti94
Copy link
Author

FYI, the errors are caused by the CI configuration being old. Consider updating it.

@alecmocatta
Copy link
Owner

Thanks for this! Let me know when done and will merge despite the outdated CI.

@LucaCappelletti94
Copy link
Author

Hi @alecmocatta, sure - I am running some benchmarks so I may add some other small edits.

@LucaCappelletti94
Copy link
Author

Why are you checking whether the estimates are three-sorted? I see that you run a binary search but that requires the estimates to be completely sorted, and I recently discovered that the estimates provided from the HLL++ paper are not sorted (they nearly are). I will be providing a fix for that shortly.

fn is_three_sorted<T>(data: &[T]) -> bool
where
T: PartialOrd + Debug,
{
data.windows(3).all(|w| {
let ret = w[0].partial_cmp(&w[2]).unwrap() != Ordering::Greater;
if !ret {
println!("{:?}", w);
}
ret
})
}
}

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