-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Allow disabling jemalloc as the memory allocator #14888
Conversation
use libc; | ||
|
||
#[inline] | ||
pub unsafe fn allocate(size: uint, _align: uint) -> *mut u8 { |
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.
This isn't a correct implementation if it's ignoring the align
parameter. It either needs to call posix_memalign
instead or manually implement a header to enforce alignment. On Windows, _aligned_malloc
, _aligned_realloc
and _aligned_free
can be used, potentially special casing a low alignment requirement and calling through to malloc
(and doing the same branch in realloc
and free
).
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.
This is correct when considering the last commit of this pull request: alexcrichton@0b38ba7.
This is precisely what I and brson were saying in #14006.
Systems have used malloc/free for decades. There's no reason that rust shouldn't be compatible with these systems.
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.
There's no point of having the alignment parameter if respecting it is optional. It's necessary if the language is going to support SIMD types. Existing systems have also use functions like strcpy
for decades but it doesn't mean Rust's strings should be null-terminated and compatible with those functions. Both POSIX and Windows support making aligned allocations so I can't see the logic in dropping support for SIMD types without gaining anything in return.
.. .and this interface isn't enough, we have to kludge around it. its a nice opportunity for Rust to take a step forward, mandating a better interface to allocation by default; its easy to wrap something around malloc/free supporting the extra fidelity if thats all a platform gives. malloc should take an alignment, and free should take a size, IMO. |
+1, very easy to manually manage alignment around malloc/free. I don't 2014年6月16日 上午7:22于 "dobkeratops" [email protected]写道:
|
but if the basic interface Rust mandates allows more fidelity, you can implement a better foundation than malloc/free. You needn't be constrained by choices made 30+ years ago. if something better isn't isn't available, Rust can just supply an inefficient wrapper around malloc/free, providing the alignment, and ignoring the size on free. malloc/free were designed for a language which didn't have collection classes; Collection classes make it easy for the client to keep the information of allocation size - e.g. inferred by the size of a type for unique_ptr or Box, or vector /Vec; so there's no need for the underlying allocator to replicate this information aswell. |
It could also just consider asking for an alignment higher than it supports to be an out-of-memory error. It really isn't that hard to branch into code using an alignment header though, and it doesn't need to have overhead for the low alignment case since |
Updated with aligned allocation on a per-platform basis, I suppose we should use it so long as it exists. |
It would be nice to special case low alignments by calling through to |
Prioritize threads affected by user typing To this end I’ve introduced a new custom thread pool type which can spawn threads using each QoS class. This way we can run latency-sensitive requests under one QoS class and everything else under another QoS class. The implementation is very similar to that of the `threadpool` crate (which is currently used by rust-analyzer) but with unused functionality stripped out. I’ll have to rebase on master once rust-lang#14859 is merged but I think everything else is alright :D
Requiring jemalloc as the allocator for all platforms hamstrings rust to being as portable as jemalloc is, with no exceptions. It is often more difficult to get jemalloc working in various situations, such as working with emscripten. With this motivation, a new configure option,
--disable-jemalloc
is provided to build liballoc to use the system allocator by default rather than jemalloc. The default is still to enable jemalloc.