Skip to content
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

switch jemalloc-sys back to tikv-jemalloc-sys, and update to 0.6.0 #133792

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

lqd
Copy link
Member

@lqd lqd commented Dec 3, 2024

Some context:

A perf run showed some improvements: jemalloc enables stats reporting by default and version 0.6.0 of the bindings now correctly enables/disables this setting when compiling the library depending on the stats cargo feature actually being activated. So, this PR switches back to tikv-jemalloc-sys to update to 0.6.0.

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 3, 2024
@lqd
Copy link
Member Author

lqd commented Dec 3, 2024

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 3, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 3, 2024
switch `jemalloc-sys` back to `tikv-jemalloc-sys`, and update to 0.6.0

Some context:
- we used to use jemalloc bindings from https://github.com/gnzlbg/jemallocator, since rust-lang#55238
- that crate was abandoned, picked up as a fork in https://github.com/tikv/jemallocator, so we switched to that in rust-lang#83152.
- then they were able to publish to the original `jemalloc-sys` bindings crate, and `jemalloc-sys` and `tikv-jemalloc-sys` became the same thing -- so I switched back to the OG crate in rust-lang#96790
- they're now having publishing problems again: I've been waiting for tikv/jemallocator#96 for the `jemalloc-sys` 0.6.0 update for a few months, but `tikv-jemalloc-sys` is already updated to 0.6.0.

A perf run showed some improvements, so this PR switches back to `tikv-jemalloc-sys` to update to 0.6.0.

r? ghost
@bors
Copy link
Contributor

bors commented Dec 3, 2024

⌛ Trying commit a69fe84 with merge c4063b417f80744aa7e0f240e2c7be86d706c158...

@bors
Copy link
Contributor

bors commented Dec 3, 2024

☀️ Try build successful - checks-actions
Build commit: c4063b4 (c4063b417f80744aa7e0f240e2c7be86d706c158)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c4063b4): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.2% [-0.3%, -0.1%] 77
Improvements ✅
(secondary)
-0.2% [-0.2%, -0.1%] 31
All ❌✅ (primary) -0.2% [-0.3%, -0.1%] 77

Max RSS (memory usage)

Results (primary -1.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.6% [-1.6%, -1.6%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -1.6% [-1.6%, -1.6%] 1

Cycles

Results (primary 1.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.9% [0.9%, 2.5%] 7
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.9% [0.9%, 2.5%] 7

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 767.333s -> 766.603s (-0.10%)
Artifact size: 332.08 MiB -> 330.99 MiB (-0.33%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 3, 2024
@Mark-Simulacrum
Copy link
Member

r=me

@lqd lqd marked this pull request as ready for review December 3, 2024 13:32
@rustbot
Copy link
Collaborator

rustbot commented Dec 3, 2024

The Miri subtree was changed

cc @rust-lang/miri

The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging.

cc @davidtwco, @wesleywiser

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@lqd
Copy link
Member Author

lqd commented Dec 3, 2024

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Dec 3, 2024

📌 Commit a69fe84 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 3, 2024
@bors
Copy link
Contributor

bors commented Dec 3, 2024

⌛ Testing commit a69fe84 with merge 490b2cc...

@bors
Copy link
Contributor

bors commented Dec 3, 2024

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 3, 2024
@bors bors merged commit 490b2cc into rust-lang:master Dec 3, 2024
7 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 3, 2024
@lqd lqd deleted the jemallocup branch December 3, 2024 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants