-
Notifications
You must be signed in to change notification settings - Fork 16
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
build compatibility and thread sharing #191
Conversation
improve compatibilty and build errors on build_cc/cmake for windows
null catches
Seems I need to recheck build_cc for other os versions. I'll update over next day |
FYI, FreeBSD is of LLVM-like toolchain. |
thanks so much, I am not used to this one! |
In testing I noticed an almost doubling of test time between build_cc and cmake, they are now unified to make sure there is reduced chance of difference between builds. also enabled ability to read build environment of allocator
@SchrodingerZhu how do you normally test on freebsd locally? do you have Dockerfile? |
This builder cleanup is something I have been always wanting to do. Thanks a lot! I will take a look tomorrow (EDT). |
I'm debating redoing again but with greater focus on compiler toolchain but this was already gnarly and more than suitable for my needs so I may wait a bit. At least now the builds are appearing the same regardless of build option unlike before. I also made changes to lib.rs in snmalloc_rs as I'm trying to share allocator itself over a new process or thread and hijack memory calls |
Can u illustrate more on why you have added special handling of zero-sized layout? The rust document says it is up to the user to make sure the layout is of nonzero size.
I think sentinel value is a good practice, but I personally prefer to provide another struct (Say SentineledSnMalloc) that wraps up the SnMalloc struct and adds additional check before calling into inner layer. |
yeah, I saw that in the rust manual but I looked further and found that by explicitly handling zero-sized layouts, we can prevent undefined behavior by 1) Returning an aligned non-null pointer for zero-sized allocations without actually allocating memory 2) Skipping deallocation calls for zero-sized types 3) Handling reallocation edge cases involving zero sizes. Overall, the major benefit should be from zero-sized types (ZSTs) which can be optimized to no ops without unnecessary allocation calls |
Solid. Given that in most allocation site the size info is statically known, I think LTO will remove this branch. @ryancinsight can you justify this? Oh, you actually mark the function as |
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.
Rest parts look good to me but please address @devnexen's comment. Thank you for the patch!
sorry for delay, didn't like the skipping of windows for LTO and linking differences between systems were giving some issues so now revised and working as far as I can tell |
Are you thinking something like this, still have some redundancies but can clean up: ``#![no_std] use core::{ #[derive(Debug, Copy, Clone)] unsafe impl Send for SnMalloc {} impl SnMalloc {
} unsafe impl GlobalAlloc for SnMalloc {
} /// Wrapper around SnMalloc to handle zero-sized layouts explicitly. impl SentineledSnMalloc {
} unsafe impl GlobalAlloc for SentineledSnMalloc {
} #[cfg(test)]
} |
changes to builds as cmake and cc were failing windows gnu, clang, ucrt, but now working