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.

Sign up for GitHub

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

Jump to bottom

Update jemalloc to v5.3 #96790

Merged
merged 1 commit into from May 27, 2022
Merged

Update jemalloc to v5.3 #96790

merged 1 commit into from May 27, 2022

Conversation

lqd
Copy link
Member

@lqd lqd commented May 6, 2022 •
edited

Now that jemalloc version 5.3 has been released, this PR updates tikv-jemalloc-sys to the corresponding release.

The crates.io publishing issue seems to have been resolved for the jemalloc-sys package, and version 5.3.0 is now also available under the historical name (and should become the preferred crate to be used). Therefore, this PR also switches back to using jemalloc-sys instead of tikv-jemalloc-sys.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 6, 2022
@lqd
Copy link
Member Author

lqd commented May 6, 2022

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 6, 2022
@bors
Copy link
Contributor

bors commented May 6, 2022

⌛ Trying commit 4a3f0b89c6d0733a6e84288e3f9aafdf8429875e with merge 7c1ccf45f83f6c96b0fa945d58b116bea4b726ad...

@bors
Copy link
Contributor

bors commented May 6, 2022

💔 Test failed - checks-actions

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 6, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@lqd
Copy link
Member Author

lqd commented May 6, 2022

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@bors
Copy link
Contributor

bors commented May 6, 2022

⌛ Trying commit e0907118f66e205a81a91c7e565ccd847e0ec58f with merge c236f4668dec3cbb6ae05f72d8339c85657b43ff...

@bors
Copy link
Contributor

bors commented May 7, 2022

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

@rust-timer
Copy link
Collaborator

Queued c236f4668dec3cbb6ae05f72d8339c85657b43ff with parent 77652b9, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c236f4668dec3cbb6ae05f72d8339c85657b43ff): comparison url.

Summary:

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: mixed results
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 0 1 219 173 219
mean2 N/A 0.3% -1.0% -1.1% -1.0%
max N/A 0.3% -6.3% -3.3% -6.3%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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-review -S-waiting-on-perf -perf-regression

Footnotes

  1. number of relevant changes ↩

  2. the arithmetic mean of the percent change ↩

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels May 7, 2022
@lqd lqd mentioned this pull request May 7, 2022
Merged
@lqd
Copy link
Member Author

lqd commented May 11, 2022

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 11, 2022
@bors
Copy link
Contributor

bors commented May 11, 2022

⌛ Trying commit e61073ef87eed391e23ae16ba5ff037031730411 with merge 41e4c1342cf9326b60d0500f5890059a3d4583fc...

@lqd lqd changed the title Test jemalloc update to 5.3RC Test jemalloc update to 5.3 May 11, 2022
@bors
Copy link
Contributor

bors commented May 11, 2022

☀️ Try build successful - checks-actions
Build commit: 41e4c1342cf9326b60d0500f5890059a3d4583fc (41e4c1342cf9326b60d0500f5890059a3d4583fc)

@rust-timer
Copy link
Collaborator

Queued 41e4c1342cf9326b60d0500f5890059a3d4583fc with parent 08b4f1b, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (41e4c1342cf9326b60d0500f5890059a3d4583fc): comparison url.

Summary:

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: 🎉 relevant improvements found
Regressions 😿
(primary)		 Regressions 😿
(secondary)		 Improvements 🎉
(primary)		 Improvements 🎉
(secondary)		 All 😿 🎉
(primary)
count1 0 2 218 187 218
mean2 N/A 0.2% -0.9% -1.1% -0.9%
max N/A 0.2% -6.3% -3.3% -6.3%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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-review -S-waiting-on-perf -perf-regression

Footnotes

  1. number of relevant changes ↩

  2. the arithmetic mean of the percent change ↩

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 11, 2022
@lqd
Copy link
Member Author

lqd commented May 25, 2022

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@bors
Copy link
Contributor

bors commented May 25, 2022

⌛ Trying commit adab135 with merge 9ca7ce57dc5f41875f4d4bf4a239170463384666...

@lqd lqd marked this pull request as ready for review May 25, 2022 07:45
@lqd
Copy link
Member Author

lqd commented May 25, 2022 •
edited

The perf.rlo queue has a few PRs already, so final perf results should be available in a while.

After this last sanity check, r? @Mark-Simulacrum maybe ? (feel free to re-roll if you don't have time)

@nnethercote
Copy link
Contributor

So jemallocator is effectively an alias for tikv-jemallocator now, is that right?

@nnethercote
Copy link
Contributor

@bors delegate=lqd

@bors
Copy link
Contributor

bors commented May 25, 2022

✌️ @lqd can now approve this pull request

@bors
Copy link
Contributor

bors commented May 25, 2022

☀️ Try build successful - checks-actions
Build commit: 9ca7ce57dc5f41875f4d4bf4a239170463384666 (9ca7ce57dc5f41875f4d4bf4a239170463384666)

@rust-timer
Copy link
Collaborator

Queued 9ca7ce57dc5f41875f4d4bf4a239170463384666 with parent 9fadabc, future comparison URL.

@lqd
Copy link
Member Author

lqd commented May 25, 2022

@nnethercote The situation is a bit unclear to me: the owners of the tikv-jemalloc* forked crates seem to have been granted publishing rights to some of the original packages now, but apparently not all of the jemallocator ones (I've gleaned this understanding from tikv/jemallocator#26). Maybe that's about jemalloc-ctl or jemallocator-global.

It seems that depending on which of these jemalloc crates one was using, you could now switch back to jemallocator (much like you can switch back to jemalloc-sys if you're only using that).

I'm under the impression that in the most common cases, the end result indeed behaves as an alias. And otherwise in the rarer cases where you depend on more than these main crates, you would see that it's the same code published under 2 different names, with the satellite crates likely depending on the tikv-jemalloc* ones only.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9ca7ce57dc5f41875f4d4bf4a239170463384666): comparison url.

Instruction count

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 0.2% 0.2% 1
Improvements 🎉
(primary)		 -0.9% -6.1% 214
Improvements 🎉
(secondary)		 -1.1% -3.4% 188
All 😿🎉 (primary) -0.9% -6.1% 214

Max RSS (memory usage)

Results
  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)		 1.0% 1.2% 4
Regressions 😿
(secondary)		 1.9% 3.9% 8
Improvements 🎉
(primary)		 -2.7% -5.6% 48
Improvements 🎉
(secondary)		 -7.7% -21.6% 60
All 😿🎉 (primary) -2.4% -5.6% 52

Cycles

Results
  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: 🎉 relevant improvements found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 -2.4% -3.4% 4
Improvements 🎉
(secondary)		 -2.7% -3.7% 8
All 😿🎉 (primary) -2.4% -3.4% 4

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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-review -S-waiting-on-perf -perf-regression

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2 ↩3

  2. number of relevant changes ↩ ↩2 ↩3

@nnethercote
Copy link
Contributor

Very nice improvements on max-rss.

@lqd
Copy link
Member Author

lqd commented May 25, 2022 •
edited

I'll let mark weigh in on whether we need to do some additional checks when updating such important dependencies, that's why I'm not going to r=you immediately, to give him some time to do so.

@Mark-Simulacrum
Copy link
Member

We're pretty early in the release cycle and I think this looks like a good win without major regressions, so I think we can move forward.

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented May 27, 2022

📌 Commit adab135 has been approved by Mark-Simulacrum

@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. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 27, 2022
@bors
Copy link
Contributor

bors commented May 27, 2022

⌛ Testing commit adab135 with merge ebbcbfc...

@bors
Copy link
Contributor

bors commented May 27, 2022

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 27, 2022
@bors bors merged commit ebbcbfc into rust-lang:master May 27, 2022
@rustbot rustbot added this to the 1.63.0 milestone May 27, 2022
@bors bors mentioned this pull request May 27, 2022
Merged
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ebbcbfc): comparison url.

Instruction count

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 0.3% 0.3% 1
Improvements 🎉
(primary)		 -0.9% -6.2% 212
Improvements 🎉
(secondary)		 -1.1% -3.2% 191
All 😿🎉 (primary) -0.9% -6.2% 212

Max RSS (memory usage)

Results
  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)		 1.2% 1.2% 2
Regressions 😿
(secondary)		 2.4% 4.3% 11
Improvements 🎉
(primary)		 -2.6% -5.5% 48
Improvements 🎉
(secondary)		 -7.7% -20.7% 53
All 😿🎉 (primary) -2.5% -5.5% 50

Cycles

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 4.5% 4.5% 1
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 -2.6% -3.0% 3
All 😿🎉 (primary) N/A N/A 0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2 ↩3

  2. number of relevant changes ↩ ↩2 ↩3

@lqd lqd deleted the update_jemalloc branch May 28, 2022 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.63.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@lqd @rust-timer @bors @rust-log-analyzer @nnethercote @Mark-Simulacrum @rustbot