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

enable jemalloc assertions when configured to do so #52906

Merged
merged 1 commit into from
Aug 2, 2018

Conversation

RalfJung
Copy link
Member

This is essentially a re-submission of the functional part of #43648. I was unable to reproduce the issue I had back then, maybe something changed somewhere to no longer trigger the assertion.

Fixes #44152

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 31, 2018
@RalfJung
Copy link
Member Author

RalfJung commented Jul 31, 2018

@gnzlbg says nothing should have changed. I tried these settings:

# Whether or not jemalloc is built and enabled
use-jemalloc = true

# Whether or not jemalloc is built with its debug option set
debug-jemalloc = true

That should be enough to properly enable this, right? Then I did ./x.py build, and it went through all the way.

I verified that the relevant code in build.rs gets executed by making it panic.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 31, 2018

Yes, this change should be enough to configure jemalloc with debug assertions enabled when compiled with the debug cargo feature.

If you want to be extra sure that it is being compiled with debug assertions enabled, building it with -vv (verbose output, so that you see the output of the build.rs; no idea how to enable this via x.py) and then inspecting the output of ./configure should be enough.

This line of the output: https://travis-ci.org/jemalloc/jemalloc/jobs/408750420#L346 should contain debug : 1 (1 means enabled, 0 disabled).

@alexcrichton
Copy link
Member

@bors: r+

Ok! Let's see if it gets past CI

@bors
Copy link
Contributor

bors commented Jul 31, 2018

📌 Commit 872395a has been approved by alexcrichton

@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 Jul 31, 2018
@bors
Copy link
Contributor

bors commented Aug 2, 2018

⌛ Testing commit 872395a with merge 76aeeef...

bors added a commit that referenced this pull request Aug 2, 2018
enable jemalloc assertions when configured to do so

This is essentially a re-submission of the functional part of #43648. I was unable to reproduce the issue I had back then, maybe something changed somewhere to no longer trigger the assertion.

Fixes #44152
@bors
Copy link
Contributor

bors commented Aug 2, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 76aeeef to master...

@bors bors merged commit 872395a into rust-lang:master Aug 2, 2018
@RalfJung RalfJung deleted the jemalloc branch August 16, 2018 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants