Disable the background_threads jemallocator feature

[carols10cents]

It's not supported on macOS :(

When I try to cargo run --bin server on macOS Catalina 10.15.1, I get:

$ cargo run --bin server
    Finished dev [unoptimized + debuginfo] target(s) in 0.51s
     Running `target/debug/server`
<jemalloc>: option background_thread currently supports pthread only
memory allocation of 40 bytes failedAbort trap: 6

Leave the upgrade but turn off the background_threads feature for now. Someday we might be able to enable features per-target, but that day is not today.

r? @jtgeibel

[jtgeibel]

@carols10cents I'm a bit torn between how we should handle this. I definitely wasn't expecting that moving the configuration flag from runtime to build-time would result in a change to runtime behavior. Maybe setting this at runtime is ignored on macOS? The docs say "spawns background worker threads on initialization (first jemalloc call) if this option is enabled". So maybe std is sneaking in an allocation before main is called, meaning our runtime enable never worked?

Does MALLOC_CONF="background_thread:false" cargo run --bin server work for you on master? That should override the build time configuration and I'm guessing that works on your machine.

Does running MALLOC_CONF="background_thread:true" cargo run --bin server on an older version of master (before #1953) fail for you at runtime? If so, then I expect that we haven't been properly enabling background threads (ever since rustc defaulted to system malloc in 1.32).

In summary, I guess my suggestions are:

• r+ on this
• Set MALLOC_CONF to background_thread:true on production
  • (Although I always liked seeing this explicitly mentioned in our codebase. We can add jemalloc-ctl back in. Or we add to the docs saying to set the variable in production.)
• If our runtime logic isn't working from within main, work with upstream to improve docs, and maybe add a background_threads_on_supported_platforms feature to the crate.

[jtgeibel]

To clarify on "a bit torn" above, the other option is to (for now) revert my PR outright. I was thinking this might be necessary, so that we could do an atomic style deploy on this change, but since we can configure at runtime with MALLOC_CONF that doesn't seem necessary.

[jtgeibel]

Using gdb I was able to determine when the background threads are enabled on my Linux system:

• For current master, the default is enabled but this can disabled via MALLOC_CONF=background_thread:false.
• For this PR, the default is disabled but can be enabled via MALLOC_CONF=background_thread:true.
• For an older version of master, on my machine background threads are always enabled and is is not possible to disable them via MALLOC_CONF. So it appears the runtime option at the start of main was working as expected.

[carols10cents]

Does MALLOC_CONF="background_thread:false" cargo run --bin server work for you on master? That should override the build time configuration and I'm guessing that works on your machine.

No :(

$ git show --format=oneline
e2e7f65300b0068a64647c7d4fa6f7fa7ce4e84e (HEAD -> master, upstream/master) Auto merge of #1975 - rust-lang:dependabot/npm_and_yarn/eslint-config-prettier-6.7.0, r=locks
$ MALLOC_CONF="background_thread:false" cargo run --bin server
    Finished dev [unoptimized + debuginfo] target(s) in 0.25s
     Running `target/debug/server`
<jemalloc>: option background_thread currently supports pthread only
memory allocation of 40 bytes failedAbort trap: 6

Does running MALLOC_CONF="background_thread:true" cargo run --bin server on an older version of master (before #1953) fail for you at runtime? If so, then I expect that we haven't been properly enabling background threads (ever since rustc defaulted to system malloc in 1.32).

Nope, it works:

$ git checkout 68c5a8e
$ MALLOC_CONF="background_thread:true" cargo run --bin server
   Compiling cargo-registry v0.2.2 (/Users/carolnichols/rust/crates.io)
    Finished dev [unoptimized + debuginfo] target(s) in 59.42s
     Running `target/debug/server`
Using local uploader, crate files will be in the local_uploads directory
Booting with a civet based server
listening on port 8888

Because my results aren't what you were expecting, does that change your thoughts on next steps?

[jtgeibel]

Because my results aren't what you were expecting, does that change your thoughts on next steps?

No, not really. I find the results rather curious though.

MALLOC_CONF=background_thread:true,abort_conf:true is set in production and staging already, and I've observed the expected behavior locally on Ubuntu 19.10 (vs 16.04 on heroku-16). I think this is fine to land on master and eventually deploy.

@bors r+

[bors]

📌 Commit 7101ff3 has been approved by jtgeibel

[bors]

⌛ Testing commit 7101ff3 with merge 4f2207f...

[bors]

☀️ Test successful - checks-travis
Approved by: jtgeibel
Pushing 4f2207f to master...