Jemalloc sneaking in even with force_alloc_system

[vorner]

Hello

I'm experimenting with some cross-compilation, using xargo. I have my own target description and a script that downloads SDK for the device and uses that to build the corresponding std.

If I enable jemalloc, I can compile a binary (with some other hacks) and it works. But I'd like to strip the size of the binary, so I'd prefer to use the system allocator instead. When I switch to use force_alloc_system and "exe-allocation-crate": "alloc_system" in the target description, it fails to link with bunch of missing references, like __rde_dealloc. Grepping the sources, these come from liballoc_jemalloc crate. So it seems, no matter what I do, some part of jemalloc (the rust part) gets pulled in, but the C part is not compiled.

Experimenting with older nightlies, the first broken version is ‒ the compilation with system allocator works in older ones (and produces ~130kB smaller binary):

rustc 1.20.0-nightly (696412de7 2017-07-06)
binary: rustc
commit-hash: 696412de7e4e119f8536686c643621115b90c775
commit-date: 2017-07-06
host: x86_64-unknown-linux-gnu
release: 1.20.0-nightly
LLVM version: 4.0

I have the code in this repository: https://github.com/vorner/xcompile. There's a build script that, when run, downloads the SDK (that one is a bit largish), copies current rust sources (yes, this needs rustup component add rust-src), patches them a little bit (I had some problems with libunwind) and compiles it. It doesn't clean things, so I always delete the sysroot and the sources between experiments (rm -rf ~/.xargo target rust-src).

There are two relevant (very similar) branches, with_jemalloc and with_sys_alloc.

I think this might be somewhat related to this report: #43510, but the observable effects are quite different.

I noticed there's that new #[global_allocator] attribute, and I'll read its documentation, but the force_alloc_system feature is still available, so I believe it wasn't the intention to deprecate it.

[aidanhs]

I left a comment in that linked issue, but in short - force_alloc_system is not what you're looking for, it's a hack to work around issues with building a jemalloc-less rust with another jemalloc-less rust (effectively nobody does this, you'd need to be compiling rust with a rust you compiled yourself).

exe-allocation-crate should 'just work' I think.

[vorner]

Thanks for the hints. After some experimentation I discovered it works when:

• force_alloc_system is not used
• The exe-allocation-crate is set to null (not "alloc_system"). That one is a bit surprising.

So, the question is, should it also work with "alloc_system", or is this behaviour OK and should the issue be closed?

[aidanhs]

It should probably work with alloc_system, I'd leave this issue open.

[aidanhs]

#43589 does not fix this.

It looks like the only allocation crate able to be pulled in explicitly as the default exe crate is jemalloc, since it's the only crate containing functions with the __rde prefix. I see no reason in theory why liballoc_system couldn't contain these too (in practice, jemalloc depends on liballoc_system for the OOM handler so the symbols would conflict) or generate them automatically if necessary.

By setting exe allocator to null, you're defaulting to the library allocator, liballoc_system.

Thoughts @alexcrichton?

[alexcrichton]

Right now the compiler isn't equipped to deal with "exe-allocation-crate": "alloc_system", but if you set it to null should work. Does that not though?

[vorner]

Yes, setting it to null works. But our impression here was that it probably should (as in „it's surprising it doesn't and users might expect it“, not „there's code that is supposed to do it“) work with "exe-allocation-crate": "alloc_system".

As I said above, my real problem was solved by the null, so I'm Ok with either closing this, burying the issue deep in history as low priority or making it work with the "alloc_system". I just don't feel like the one to decide that.

[alexcrichton]

Yeah in some sense this is deep within the internals of the compiler and is defacto pretty unstable. I'm fine fixing "bugs" like this but I don't think they'll be proactively fixed unless you'd like to endeavour to do so

[vorner]

I actually want to start contributing code into the compiler and this could be a nice first one, for two reasons. Nobody really cares much how long this takes to fix and fixing it could be a fitting punishment for me opening such a useless and stupid issue 😇

However, as the code base is quite large, I'd use a pointer or two into the general direction in which to start digging.

I think the solution to hijack the value somewhere on its way, special case it and replace it with None would work and would be reasonably easy to do. But it also feels a bit hacky. Would that be deemed OK, or should I look for a more systematic solution?

[alexcrichton]

Oh that'd be awesome! Right now there's three kinds of allocators each with their own prefix and the session's allocator kind is set in this function. We'd probably just want a way to configure the "allocator kind" by looking at the attributes in the crate rather than inferring based on the custom target spec.

[vorner]

OK, so I've read all the relevant code I could find (librustc/middle/allocator.rs, liballoc/heap.rs, liballoc_*, librustc_trans/allocator.rs) and the RFCs that seem related (1398, 1974). I probably don't understand all the details but I have some idea.

Now I see why I got the errors above and what they meant. However, I don't see an obvious next course of action. I understand the function you linked (inject_allocator_crate) will eventually get either completely rewritten or dropped, as well as the exe-allocator-crate and the magic with prefixes from the librustc/middle/allocator.rs.

So, I have a few questions:

• Am I right in assuming that the prefixes exist because sometimes both liballoc_system and liballoc_jemalloc get linked into the binary? Therefore, unifying the prefixes (or variants) of DefaultLib and DefaultExe is not an option.
• What do you mean by „We'd probably just want a way to configure the "allocator kind" by looking at the attributes in the crate rather than inferring based on the custom target spec.“? Is that what the RFC 1974 is about? Or should the compiler look into liballoc_system or liballoc_jemalloc, detect that one is able to be a lib allocator while the other an exe allocator and set the allocator kind based on that? Is possible? Is it worth it (assuming these kinds will just go away eventually)?
• If the answer above is no, would it make sense to add a special logic to inject_allocator_crate to error on seeing the exe allocator being set to alloc_system (probably the only lib allocator in existence now), since it knows only how to be a lib allocator and will definitely fail, or silently turning it into None? It would be only to prevent having really unhelpful linker errors later on.
• What is the status around the RFC 1974? The checkbox on its tracking issue Tracking issue for changing the global, default allocator (RFC 1974) #27389 says it is implemented, but I don't think it is completely. The prefix __rg_ exists only in the one place in librustc/middle/allocator.rs and there's no part that provides the functions (or constructs them). Is it magic of some kind, or just missing part? Or a leftover from before? The System allocator is still present in the alloc_system crate, not (as the RFC says) in std::heap. It seems to work, but I haven't yet tracked how exactly.
• When I've already read parts of the code around the allocators, is there something I might help with? Is this a good place to ask all this, or should I move elsewhere? IRC?

[alexcrichton]

The various issues and RFCs tend to not go much into the implementation details, so for that you're mostly just relegated to code and comments to figure that out.

Here what's happening is that the compiler just needs to select the right prefix for specifically the alloc_system and alloc_jemalloc crates. There's no other crate we need to worry about here. The prefix there is selected by the "kind" and we just need to ensure that rustc picks the right kind when it links to one of those two crates, which could be encoded in an attribute.