Use tikv-jemallocator in rustc/rustdoc in addition to jemalloc-sys when enabled. #83152

guswynn · 2021-03-15T16:18:35Z

In #81782 it was mentioned that one reason rustc may benefit from minimalloc is it doesn't use the sdallocx api from jemalloc.

Currently, on unix, rust uses jemalloc by importing its symbols to use them with the default, System (libc) global allocator.
This PR switches its global alloc to tikv-jemallocator, which correctly uses sized deallocation (https://docs.rs/tikv-jemallocator/0.4.1/src/tikv_jemallocator/lib.rs.html#121-126). tikv-jemallocator, as far as I can tell, is a more up-to-date set of bindings to jemalloc than jemallocator

The perf results of this pr are in large part due to the version upgrade of jemalloc, but sized deallocation has a non-trivial improvement, particularly to rustdoc.

This pr also includes changes to bootstrap to correctly pass the jemalloc feature through to the rustdoc build

rust-highfive · 2021-03-15T16:18:39Z

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

Mark-Simulacrum · 2021-03-15T16:26:11Z

@bors try @rust-timer queue

rust-timer · 2021-03-15T16:26:12Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-03-15T16:26:20Z

⌛ Trying commit 1b45a29fdf54f0ffae814c700149fd8f18b36442 with merge 0bcde55ea93243c9ca7a70e1c0a4ae9e5a8d5034...

guswynn · 2021-03-15T16:37:40Z

ci failure was just a tidy mistake (I ALWAYS forget to run that locally...)

jyn514 · 2021-03-15T16:40:11Z

Do we have perf tests for error paths? My hunch is that, while the clean path of rustc is heavily arena-driven, the error paths are more alloc-heavy

I don't think so, historically we haven't worried much about compile time for errors (see also https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/perfbot/near/229630033). I'd expect rustdoc to show any changes though, it does a lot of allocations.

guswynn · 2021-03-15T16:42:58Z

@jyn514 rustdoc doesn't appear to depend on this crate? In that case it may be using the libc allocator in all cases?

jyn514 · 2021-03-15T17:21:38Z

Oh huh, I think you're right. Do you mind doing that here? You don't have to add jemalloc to cargo.toml, just load it from the sysroot (see the comment at the top of librustdoc/lib.rs).

guswynn · 2021-03-15T18:05:09Z

(I also testing with "jemalloc = true" in config.toml locally as even tho im on osx, its ci that turns that on, as well as used the more-correct jemalloc feature in rustc_driver)

src/librustdoc/lib.rs

guswynn · 2021-03-15T18:44:00Z

Does bor's need to re-queue with the new commits for rust-timer to catch up?

jyn514 · 2021-03-15T19:26:38Z

Does bors need to re-queue with the new commits for rust-timer to catch up?

Yes, otherwise rust-timer will use the build artifacts from the old commit.

@bors try @rust-timer queue

rust-timer · 2021-03-15T19:26:39Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2021-03-15T19:26:47Z

⌛ Trying commit dad03db36b50c9cc8e935a48d0d8bc5aec5f83c2 with merge 4f4b85262ea011b219ebd71d3da9ac82605df32e...

LifeIsStrange · 2021-03-15T20:52:48Z

It would probably be better to use this fork which is more up to date with Jemalloc:
https://github.com/tikv/jemallocator

guswynn · 2021-03-15T20:55:11Z

@LifeIsStrange I'll take a look!

bors · 2021-03-15T21:12:31Z

💔 Test failed - checks-actions

bors · 2021-04-05T23:40:12Z

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing d322385 to master...

LifeIsStrange · 2021-04-22T23:13:38Z

@guswynn hi, jemalloc is working on an unofficial release channel which would update every 1-3 months.
They promise to do a less extensive but best effort performance and stability testing, for the x86-64 Linux platform.
So for this platform it could make sense to use such pre-releases.

jemalloc/jemalloc#2060

3023: perf: switch global alloc to tikv-jemallocator r=doitian,driftluo a=zhangsoledad  ### What problem does this PR solve? gnzlbg/jemallocator#168 jemalloc/jemalloc#1497 see also: rust-lang/rust#83152 ### What is changed and how it works? Proposal: switch global alloc to tikv-jemallocator which is more up-to-date ### Related changes ### Check List  Side effects - Performance regression - Breaking backward compatibility ### Release note  ```release-note Title Only: Include only the PR title in the release note. ``` Co-authored-by: zhangsoledad <787953403@qq.com>

This was added in rust-lang#83152, which has several errors in its comments. This commit also fix up the comments, which are quite wrong and misleading.

nnethercote · 2021-12-23T06:33:14Z

Currently, on unix, rust uses jemalloc by importing its symbols to use them with the default, System (libc) global allocator. This PR switches its global alloc to tikv-jemallocator, which correctly uses sized deallocation (https://docs.rs/tikv-jemallocator/0.4.1/src/tikv_jemallocator/lib.rs.html#121-126). tikv-jemallocator, as far as I can tell, is a more up-to-date set of bindings to jemalloc than jemallocator

I know I'm late to the party, but: the first and third sentences here are correct, but the second sentence is mostly incorrect. Although this PR added a #[global_allocator], it has no effect, and the mechanism described in the first sentence is still the one that causes jemalloc to be used.

The perf results of this pr are in large part due to the version upgrade of jemalloc, but sized deallocation has a non-trivial improvement, particularly to rustdoc.

Sized deallocation doesn't come into the picture; the improvement was all because tikv-jemalloc-sys uses a newer version of jemalloc than jemalloc-sys. See #92222 for more details.

guswynn · 2021-12-23T07:15:31Z

@nnethercote One of the earlier perf runs up above in this pr used jemallocator (non-tikv) and more closely captured the much smaller, but still measurable effect of sized deallocation. The larger wins were the upgrade, as you mentioned; the pr description is likely outdated. Hearing that those sized dealloc apis are not being used, which led to #92222 Is very concerning to me, and I would need to dig deeper into why the #[global_allocator] attribute is not causing the sized apis to be used, if that is the case.

nnethercote · 2021-12-23T08:58:02Z

@nnethercote One of the earlier perf runs up above in this pr used jemallocator (non-tikv) and more closely captured the much smaller, but still measurable effect of sized deallocation.

That perf run is here and you'll see that almost every significant change is a doc build. (The two non-doc ones are likely noise.) You did add the use of jemalloc to rustdoc, but it's the extern stuff in main that has the effect of incorporating jemalloc, not the #[global_allocator].

I would need to dig deeper into why the #[global_allocator] attribute is not causing the sized apis to be used, if that is the case.

#92222's comment points to #81782 (comment), which has an explanation.

nnethercote · 2021-12-23T09:01:59Z

BTW, this was still a great PR! Switching to tikv-jemallocator (well, tikv-jemalloc-sys, because tikv-jemallocator is bypassed) was a huge win, and adding it to rustdoc was also good. It's just that what actually happened and what everyone thought happened didn't quite match 😀

…-rustdoc, r=alexcrichton Remove useless `#[global_allocator]` from rustc and rustdoc. This was added in rust-lang#83152, which has several errors in its comments. This commit also fix up the comments, which are quite wrong and misleading. r? `@alexcrichton`

guswynn · 2021-12-27T16:32:15Z

@nnethercote oh WOW did not know about the complexity of how rustc is build

is decoupling librustc_driver.so and libstd.so something that anyone is looking at?

nnethercote · 2022-01-03T21:19:11Z

is decoupling librustc_driver.so and libstd.so something that anyone is looking at?

Not that I know of. And given that sized deallocation doesn't appear to be a win, there's not much motivation for it.

Switch to maintained version of jemallocator like rustc did in: rust-lang/rust#83152

* Update clap to 3.0 and improve argument parsing and help output. Update clap to 3.0 and improve argument parsing and help output: - Update clap from 3.0.0-beta4 to 3.0. - Update help output text. - Allow easy specifying of "-1" for -O option (allow_hyphen_values(true)). - Make -v var=value as last argument before program work without need for "--" or other argument after. - Make -i and -F mutually exclusive as specified field separator as -F is not used when parsing input files as CSV or TSV. * Update itoa from 0.4 to 1.0. Update itoa from 0.4 to 1.0. itoa::Buffer::new() allocates the necessary space on the stack automatically, and usage is similar to ryu now. * Update other dependencies. Update other dependencies. * Switch to maintained version of jemallocator. Switch to maintained version of jemallocator like rustc did in: rust-lang/rust#83152

refs: - gnzlbg/jemallocator#173 - rust-lang/rust#83152

rust-lang/rust#83152

rust-highfive assigned Mark-Simulacrum Mar 15, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 15, 2021

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 15, 2021

This comment has been minimized.

Sign in to view

guswynn force-pushed the jemallocator_part2 branch from 1b45a29 to 1bf6efa Compare March 15, 2021 16:36

jyn514 added I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 15, 2021

guswynn force-pushed the jemallocator_part2 branch from 1bf6efa to 0549375 Compare March 15, 2021 16:37

guswynn commented Mar 15, 2021

View reviewed changes

src/librustdoc/lib.rs Show resolved Hide resolved

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 15, 2021

This comment has been minimized.

Sign in to view

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 5, 2021

bors merged commit d322385 into rust-lang:master Apr 5, 2021

rustbot added this to the 1.53.0 milestone Apr 5, 2021

matklad mentioned this pull request Sep 1, 2021

Replace outdated jemalloc near/nearcore#4766

Closed

0xdeafbeef mentioned this pull request Sep 3, 2021

Replace jemalloc rust-rocksdb/rust-rocksdb#554

Merged

ordian mentioned this pull request Sep 3, 2021

[parity-util-mem] Consider using tikv-jemallocator instead of jemallocator paritytech/parity-common#586

Closed

zhangsoledad mentioned this pull request Sep 8, 2021

perf: switch global alloc to tikv-jemallocator nervosnetwork/ckb#3023

Merged

tgnottingham mentioned this pull request Sep 24, 2021

Indicate that keccak benchmark has high max-rss variance rust-lang/rustc-perf#800

Closed

guswynn deleted the jemallocator_part2 branch September 26, 2021 18:51

thaodt mentioned this pull request Sep 28, 2021

Suggest using the updated jemalloc lib SaitoTech/saito-rust#259

Closed

nnethercote mentioned this pull request Dec 23, 2021

Remove useless #[global_allocator] from rustc and rustdoc. #92222

Merged

ghuls added a commit to ghuls/frawk that referenced this pull request Jan 13, 2022

Switch to maintained version of jemallocator.

e2177db

Switch to maintained version of jemallocator like rustc did in: rust-lang/rust#83152

ghuls added a commit to ghuls/frawk that referenced this pull request Jan 13, 2022

Switch to maintained version of jemallocator.

e8a6333

Switch to maintained version of jemallocator like rustc did in: rust-lang/rust#83152

edjroz mentioned this pull request Apr 5, 2022

Make allocator configurable CosmWasm/wasmvm#264

Open

joseluisq added a commit to static-web-server/static-web-server that referenced this pull request Apr 21, 2022

refactor: move to maintained jemalloc (tikv-jemallocator)

024531c

refs: - gnzlbg/jemallocator#173 - rust-lang/rust#83152

Haraade mentioned this pull request May 24, 2022

Memory problem ? DNSCrypt/encrypted-dns-server#86

Closed

msmouse added a commit to aptos-labs/aptos-core that referenced this pull request Aug 2, 2022

[storage] use tikv-jemallocator

cd5b53a

rust-lang/rust#83152

cijiugechu mentioned this pull request Jul 22, 2023

chore: switch to tikv-jemallocator oxc-project/oxc#583

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use tikv-jemallocator in rustc/rustdoc in addition to jemalloc-sys when enabled. #83152

Use tikv-jemallocator in rustc/rustdoc in addition to jemalloc-sys when enabled. #83152

guswynn commented Mar 15, 2021 •

edited

Loading

rust-highfive commented Mar 15, 2021

Mark-Simulacrum commented Mar 15, 2021

rust-timer commented Mar 15, 2021

bors commented Mar 15, 2021

This comment has been minimized.

guswynn commented Mar 15, 2021

jyn514 commented Mar 15, 2021

guswynn commented Mar 15, 2021

jyn514 commented Mar 15, 2021

guswynn commented Mar 15, 2021

guswynn commented Mar 15, 2021

jyn514 commented Mar 15, 2021

rust-timer commented Mar 15, 2021

bors commented Mar 15, 2021

LifeIsStrange commented Mar 15, 2021

guswynn commented Mar 15, 2021

bors commented Mar 15, 2021

This comment has been minimized.

bors commented Apr 5, 2021

LifeIsStrange commented Apr 22, 2021

nnethercote commented Dec 23, 2021

guswynn commented Dec 23, 2021

nnethercote commented Dec 23, 2021

nnethercote commented Dec 23, 2021 •

edited

Loading

guswynn commented Dec 27, 2021

nnethercote commented Jan 3, 2022

Use tikv-jemallocator in rustc/rustdoc in addition to jemalloc-sys when enabled. #83152

Use tikv-jemallocator in rustc/rustdoc in addition to jemalloc-sys when enabled. #83152

Conversation

guswynn commented Mar 15, 2021 • edited Loading

rust-highfive commented Mar 15, 2021

Mark-Simulacrum commented Mar 15, 2021

rust-timer commented Mar 15, 2021

bors commented Mar 15, 2021

This comment has been minimized.

guswynn commented Mar 15, 2021

jyn514 commented Mar 15, 2021

guswynn commented Mar 15, 2021

jyn514 commented Mar 15, 2021

guswynn commented Mar 15, 2021

guswynn commented Mar 15, 2021

jyn514 commented Mar 15, 2021

rust-timer commented Mar 15, 2021

bors commented Mar 15, 2021

LifeIsStrange commented Mar 15, 2021

guswynn commented Mar 15, 2021

bors commented Mar 15, 2021

This comment has been minimized.

bors commented Apr 5, 2021

LifeIsStrange commented Apr 22, 2021

nnethercote commented Dec 23, 2021

guswynn commented Dec 23, 2021

nnethercote commented Dec 23, 2021

nnethercote commented Dec 23, 2021 • edited Loading

guswynn commented Dec 27, 2021

nnethercote commented Jan 3, 2022

guswynn commented Mar 15, 2021 •

edited

Loading

nnethercote commented Dec 23, 2021 •

edited

Loading