-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Tracking issue for alloc_system/alloc_jemalloc #33082
Comments
Whatever the behaviour of I think a |
Just to be clear, the only thing that would be stabilized here would be the fact that a crate named |
I think it's reasonable to fast-track stabilization of some way of getting pure malloc-based allocations. I'm slightly uneasy with |
To be clear, I'm thinking we could stabilize only the name Also to be clear, it is not clear to me that this is actually what Gecko wants and that it would suffice for them.
Yeah we've always had a nice property that you can basically concatenate any crates together, but unfortunately allocators and panic runtimes erodes this down a little more :( |
It seems reasonable to have the idea of "global language services" that can be required and which can admit only one implementation. I wouldn't want this to be an expandable, user-defined set, since it's kind of anti-modular (unlike say a trait), but we do need SOME answer that permits someone to say "gimme memory" or "panic" without having to have the implementor in scope. Tying it to very special crates seems... ok to me. At least I too cannot directly think of an alternative. I guess I could imagine that merely adding the crate as an extern crate isn't enough, the app or somebody has to "bless it" for it to serve in its special role as a provider of that mechanism. But I'm not convincced that's valuable and it requires us to think up how it gets declared. In any case, total composability of crates isn't really possible. For example, we already have the notion of |
To make things a bit more explicit, we could take a similar approach as to what we do now for plugins: #![allocator(alloc_malloc)] That approach would also be nice in that you could have one allocator crate that calls into another and e.g. adds some instrumentation or whatever. |
The proposed changes here would work for us in the short term. We'd still probably write our own allocator when that API was stabilized so we could take advantage of |
Talking with @aturon offline, some thoughts we have are:
|
To be clear, the thing to deprecated would be |
Indeed! |
This is not absolutely necessary for Gecko currently; as you say, the amount of Rust code is rather small and doesn't allocate much memory anyway. But at some point, this will start to become a problem. I guess it's a question of how soon custom allocator support is likely to stabilize, versus providing this stopgap, with some considerations to how much Rust code is going to get integrated into Gecko in the intervening timeframe. (I'm assuming Gecko can only use stable Rust, and not something like the beta releases, which may not be a great assumption.) Is that stabilization liable to be sometime in 2016, or is it more likely coming in 2017? |
The earliest this can be stable (mentioned above) is August 2016, and I'm relatively confident we can shoot for that date. |
To be clear, I was asking about full-blown custom allocator stabilization, not merely an |
Aha yes indeed! I suspect that would be 2017 at the earliest, but even that would be generous most likely as it's not a super high priority just yet. We can always shift some priorities, however, to move it up. Does that mean, though, that an allocator which always calls malloc isn't what Gecko wants in the long run either? |
@froydnj By "full-blown custom allocator", do you mean:
|
It is what Gecko wants; my thought on asking when custom allocators were going to be ready was whether they'd be ready soon enough that they'd converge with a raft of Rust code being added to Gecko. In that case, we could just deal with the It is still possible that a raft of Rust code and custom allocators being ready would converge, even with a 2017 schedule, though! I'm not aware of what timeframe everybody wants their Rust code landing in Gecko. I should go find out. @aturon I meant changing the global/default allocator. I'm not familiar with the per-data structure allocator work; folks mentioned it in #29802 for possibly providing fallible operations on containers, but it's not clear to me whether the per-data structure allocator work addresses the concerns in #29802. |
Ok, thanks for the clarification @froydnj! |
cc @nnethercote |
I'm late to the party here and I might be missing some context and details. But it might be helpful to stake out a maximalist position in terms of Firefox requirements, independent of specific Rust mechanisms.
As I said, it's a maximalist position :) I understand that hitting all these requirements may be challenging. cc @glandium in case he has anything to add. |
@nnethercote just as another data point, how many of your requirements are also true for third-party components that Gecko includes? I'm just curious in the sense of figuring out how different the requirements are from "Rust is some extra stuff linked in" vs "Rust is the exact same as C++". The basic gist is that with custom global allocators we can hit all your requirements except for the stable version of rustc (which seems like a pretty good requirement!). I'm not sure if the "route everything to malloc on all platforms" style allocation hits your requirements, though. |
As far as I know all existing third-party components are C or C++ and just use
It sounds like it would, but I know the devil is in the details with this stuff. The tricks we have to play to get mozjemalloc as the heap allocator are non-trivial, and vary from platform to platform. @glandium is the expert there. |
Ok! It'd be helpful for me at least to learn some of these details about how malloc/new are wired up to mozjemalloc. It may also be helpful for understanding whether "route to malloc" will be sufficient or not. I'll see what @glandium has to say though :) |
I think I wrote about this in some other related rust issue. Anyways, from the perspective of linking static libraries into gecko and getting them to use mozjemalloc like everything else, it's only a matter of the static libraries having (in ELF-speak) undefined symbol references to malloc, free, calloc, realloc. That's valid on all platforms Gecko builds on. Now, there's another concern from Gecko perspective: malloc is fallible, and most allocations in Gecko are infallible. Being that if they fail, we MOZ_CRASH(). For C++, that happens because we have an inline definition of operator new that uses moz_xmalloc, which does malloc and MOZ_CRASH() if malloc returns NULL. (C++ code can also opt-in to fallible allocation with So, in practice, we have malloc, free, calloc, realloc, moz_xmalloc, moz_xcalloc, moz_xrealloc. I'd expect most rust code in Gecko would want the infallible-malloc type of behavior. So this raises the question: what does rust code do when malloc/calloc/realloc returns NULL to it? And the corollary: what can rust code do to gracefully handle memory allocation failure? |
Ok, thanks for the explanation @glandium! Out of curiosity, could the same treatment apply to
For us any situation where an allocation returns NULL will end up calling If it's necessary to route that to something else we can also look into stabilizing a method to handle OOM (basically something with this signature, although it'd likely be unsafe. Is that also a requirement for Gecko?
Currently, in stable Rust, nothing. All collections and pointers in the standard library end up calling that |
We could, and in fact, maybe we should... last time I looked there are a couple third-party things in Gecko that end up using HeadAlloc/HeapFree...
Whatever we end up doing for rust "exceptions" (was that called panic? I don't remember), we may want to have kind of the same for OOM. At the very least, we need to ensure this triggers the crash reporter like OOM in Gecko.
I haven't touched rust in a long while, but what does this mean for code that would, for example, try to allocate a multi-hundred megabytes buffer to decode a large image or something? Is there any way it can gracefully fail on its own or would allocating that buffer call the oom routine in the first place because it's handled by something in the standard library that does? |
@nnethercote Yes, it's RSS. I have no idea how to interpret the maps, but I put them in a gist in case they're useful for you. I though that higher RAM usage is a pretty well-known problem with jemalloc tbh :P |
The contents of |
I'm going to go out on a limb here, but I guess your application is doing large allocations. The system allocator is likely unmapping them when they're deallocated, while jemalloc keeps them around for later reuse. It also doesn't necessarily clean them up straight away. |
@glandium Thanks, that's quite helpful. For now it's much easier to just use |
It seems that the tor project would really like to be able to use their own allocators in stable Rust: https://www.reddit.com/r/rust/comments/62o9rx/tordev_tor_in_a_safer_languagerust_network_team/dfovrkx/ |
Currently the feature(alloc_system) prevents us from building on stable and beta The tracking issue is here: rust-lang/rust#33082
We're unlikely to really do anything to fix this, so I'm going to close this in favor of #27389. It's highly likely that all programs will start to link to |
We’ve been bitten before by symbol names changing: servo/heapsize#46 and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults.
We’ve been bitten before by symbol names changing: servo/heapsize#46 and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults.
We’ve been bitten before by symbol names changing: servo/heapsize#46 and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults.
We’ve been bitten before by symbol names changing: servo/heapsize#46 and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults.
We’ve been bitten before by symbol names changing: servo/heapsize#46 and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults.
Stop relying on linking details of std’s default allocator We’ve been bitten before by symbol names changing: servo/heapsize#46, and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18944) <!-- Reviewable:end -->
Stop relying on linking details of std’s default allocator We’ve been bitten before by symbol names changing: servo/heapsize#46, and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18944) <!-- Reviewable:end -->
…t allocator (from servo:jemallocator2); r=nox We’ve been bitten before by symbol names changing: servo/heapsize#46, and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults. Source-Repo: https://github.com/servo/servo Source-Revision: 07e9794306d597afe5d90d192fd32a99572c3cc3 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : c773f809c4f783e63c42218220e7c8c190727e6e
…t allocator (from servo:jemallocator2); r=nox We’ve been bitten before by symbol names changing: servo/heapsize#46, and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults. Source-Repo: https://github.com/servo/servo Source-Revision: 07e9794306d597afe5d90d192fd32a99572c3cc3
…t allocator (from servo:jemallocator2); r=nox We’ve been bitten before by symbol names changing: servo/heapsize#46, and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults. Source-Repo: https://github.com/servo/servo Source-Revision: 07e9794306d597afe5d90d192fd32a99572c3cc3
We’ve been bitten before by symbol names changing: servo/heapsize#46 and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults.
…t allocator (from servo:jemallocator2); r=nox We’ve been bitten before by symbol names changing: servo/heapsize#46, and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults. Source-Repo: https://github.com/servo/servo Source-Revision: 07e9794306d597afe5d90d192fd32a99572c3cc3 UltraBlame original commit: 921526150768f9a2e9ba1586a350583f9ad025c9
…t allocator (from servo:jemallocator2); r=nox We’ve been bitten before by symbol names changing: servo/heapsize#46, and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults. Source-Repo: https://github.com/servo/servo Source-Revision: 07e9794306d597afe5d90d192fd32a99572c3cc3 UltraBlame original commit: 921526150768f9a2e9ba1586a350583f9ad025c9
…t allocator (from servo:jemallocator2); r=nox We’ve been bitten before by symbol names changing: servo/heapsize#46, and upstream is planning to stop using jemalloc by default: rust-lang/rust#33082 (comment) So use the (relatively) new `#[global_allocator]` attribute to explicitly select the system allocator on Windows and jemalloc (now in an external crate) on other platforms. This choice matches current defaults. Source-Repo: https://github.com/servo/servo Source-Revision: 07e9794306d597afe5d90d192fd32a99572c3cc3 UltraBlame original commit: 921526150768f9a2e9ba1586a350583f9ad025c9
Introduced in RFC 1183 these two crates lie underneath the facade with their tracking issue hooked up to #27783. I'd like to open a separate issue for just these two crates.
We have a request in Gecko to stabilize the ability to route allocations to specifically
malloc
(on all platforms). This includes Windows, where currentlyalloc_system
routes to functions likeHeapAlloc
.Essentially, one of these crates, or perhaps a new crate like them, may want to have an accelerated path to stabilization before allocators themselves. For example I wouldn't want to stabilize
#![allocator]
or#![needs_allocator]
, but we could stabilize the semantics ofextern crate alloc_malloc
redirects all allocations to malloc (for example).Nominating for discussion!
cc @rust-lang/libs
cc @froydnj
The text was updated successfully, but these errors were encountered: