Skip to content

Redirect __rust_dealloc to sdallocx #122329

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

Closed
wants to merge 1 commit into from
Closed

Redirect __rust_dealloc to sdallocx #122329

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions compiler/rustc/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@
// https://github.com/rust-lang/rust/commit/b90cfc887c31c3e7a9e6d462e2464db1fe506175#diff-43914724af6e464c1da2171e4a9b6c7e607d5bc1203fa95c0ab85be4122605ef
// for an example of how to do so.

/// This redirects `__rust_dealloc` to jemalloc so it can make use of the size of the allocation.
#[cfg(feature = "jemalloc-sys")]
#[no_mangle]
pub unsafe extern "C" fn __rust_dealloc(ptr: *mut u8, size: usize, _align: usize) {
unsafe { jemalloc_sys::sdallocx(ptr.cast(), size, 0) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Zoxc I think you closed this PR to try using a #[global_allocator] here with the statically linked std, but just as a heads up:

  • I had quickly tried using jemalloc as a global allocator in the rustc driver and it segfaulted while building std
  • otherwise, don't we also need to take alignment into account in the __rust_dealloc code above in the general case, so that it matches what System.alloc does for big alignments and allocations w/ a smaller size than the alignment? That being said, in the context of rustc it shouldn't matter, these values should be coming from types that wouldn't need non-zero jemalloc flags. I did a perf run with both 0, and flags computed by checking size/alignment, and it seemed neither made any improvement over the master branch. Weird, after seeing wins in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I had quickly tried using jemalloc as a global allocator in the rustc driver and it segfaulted while building std

Do you have a branch for that?

  • otherwise, don't we also need to take alignment into account in the __rust_dealloc code above

My understanding is that the alignment argument is just a performance hint for sdallocx.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that it's just a performance hint, because it modifies the actual size (jemalloc/jemalloc-cmake@4cfe551#diff-a4cb09e38cfec8141b07c291f731a8e01a17412568a852884fd921e8e521766bR1850 - this is some old code, the new one is more complicated, but does the same thing). Sadly, it also seems like sdallocx takes a slow path when flags != 0, although that might not happen that often.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a branch for that?

yes, some wip work here lqd@6819e3c

rustc-main symbols override are still there as their presence/absence didn't seem to impact the segfault, but maybe it does and llvm would need to be setup differently.

#[used]
static _USED: unsafe extern "C" fn(*mut u8, usize, usize) = __rust_dealloc;
}

#[unix_sigpipe = "sig_dfl"]
fn main() {
// See the comment at the top of this file for an explanation of this.
Expand Down