From 15687b45f77fa55477898603f2b9ef2e9a701f72 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 11 Oct 2017 22:00:55 +0200 Subject: [PATCH] Use the jemallocator crate from crates.io instead of alloc_jemalloc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This compiles a new copy of jemalloc in addition to the one brought by the Rust standard library, but: * Only one of those ends up being linked * The increase in compile times is small (20 seconds on my laptop) This commit also adds an use of `#[global_allocator]` to select `jemallocator` as the default. `extern crate alloc_jemalloc;` used to decide the default, but it hasn’t since the `#[global_allocator]` attribute was introduced. `mallctl_set(b"epoch\0", 0_u64)` ends up calling `mallctl` with null pointers for the "old" value and its length, which isn’t the same as what we were doing before (passing the same pointer to both "old" and "new" parameters). My understanding is that this works too, since the important part is passing a new value: http://jemalloc.net/jemalloc.3.html#epoch > epoch (uint64_t) rw > > If a value is passed in, refresh the data from which > the mallctl*() functions report values, and increment the epoch. > Return the current epoch. This is useful for detecting whether > another thread caused a refresh. --- Cargo.lock | 27 +++++++++++++++ components/profile/Cargo.toml | 1 + components/profile/lib.rs | 18 +++++----- components/profile/mem.rs | 63 +++++++++-------------------------- 4 files changed, 52 insertions(+), 57 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ee8a17753dd5..f2a61c828c9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -341,6 +341,11 @@ dependencies = [ "unicode-normalization 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "cc" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "cexpr" version = "0.2.0" @@ -1444,6 +1449,24 @@ name = "itoa" version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "jemalloc-sys" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "cc 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "jemallocator" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "jemalloc-sys 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "jpeg-decoder" version = "0.1.13" @@ -2359,6 +2382,7 @@ dependencies = [ "heartbeats-simple 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "influent 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "ipc-channel 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)", + "jemallocator 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "profile_traits 0.0.1", @@ -3784,6 +3808,7 @@ dependencies = [ "checksum byteorder 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "c40977b0ee6b9885c9013cd41d9feffdd22deb3bb4dc3a71d901cc7a77de18c8" "checksum bytes 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "c129aff112dcc562970abb69e2508b40850dd24c274761bb50fb8a0067ba6c27" "checksum caseless 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "8950b075cff75cdabadee97148a8b5816c7cf62e5948a6005b5255d564b42fe7" +"checksum cc 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7db2f146208d7e0fbee761b09cd65a7f51ccc38705d4e7262dad4d73b12a76b1" "checksum cexpr 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "393a5f0088efbe41f9d1fcd062f24e83c278608420e62109feb2c8abee07de7d" "checksum cfg-if 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "d4c819a1287eb618df47cc647173c5c4c66ba19d888a6e50d605672aed3140de" "checksum cgl 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "86765cb42c2a2c497e142af72517c1b4d7ae5bb2f25dfa77a5c69642f2342d89" @@ -3876,6 +3901,8 @@ dependencies = [ "checksum ipc-channel 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "a38ad662f104525ac57012e0b79aad07507e3fc11e3bae668bf416264e760ebb" "checksum itertools 0.5.10 (registry+https://github.com/rust-lang/crates.io-index)" = "4833d6978da405305126af4ac88569b5d71ff758581ce5a987dbfa3755f694fc" "checksum itoa 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "eb2f404fbc66fd9aac13e998248505e7ecb2ad8e44ab6388684c5fb11c6c251c" +"checksum jemalloc-sys 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "94fb624d7e8345e5c42caab8d1db6ec925fdadff3fd0cb7dd781b41be8442828" +"checksum jemallocator 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "a2a0bf981c8cff179d52c6ca1784ec15e2a61a2f76a4f5319dda0a18f158f975" "checksum jpeg-decoder 0.1.13 (registry+https://github.com/rust-lang/crates.io-index)" = "2805ccb10ffe4d10e06ef68a158ff94c255211ecbae848fbde2146b098f93ce7" "checksum js 0.1.6 (git+https://github.com/servo/rust-mozjs)" = "" "checksum kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7507624b29483431c0ba2d82aece8ca6cdba9382bff4ddd0f7490560c056098d" diff --git a/components/profile/Cargo.toml b/components/profile/Cargo.toml index 7fbe3db8bcac..e8bd1d1c4df6 100644 --- a/components/profile/Cargo.toml +++ b/components/profile/Cargo.toml @@ -27,4 +27,5 @@ task_info = {path = "../../support/rust-task_info"} regex = "0.2" [target.'cfg(not(target_os = "windows"))'.dependencies] +jemallocator = "0.1" libc = "0.2" diff --git a/components/profile/lib.rs b/components/profile/lib.rs index 382edcf31c6c..0739acbebfc3 100644 --- a/components/profile/lib.rs +++ b/components/profile/lib.rs @@ -2,14 +2,10 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#![cfg_attr(not(target_os = "windows"), feature(alloc_jemalloc))] +#![cfg_attr(not(target_os = "windows"), feature(global_allocator))] #![feature(box_syntax)] -#![deny(unsafe_code)] - -#[allow(unused_extern_crates)] -#[cfg(not(target_os = "windows"))] -extern crate alloc_jemalloc; +#[cfg(not(target_os = "windows"))] extern crate jemallocator; extern crate heartbeats_simple; extern crate influent; extern crate ipc_channel; @@ -29,9 +25,11 @@ extern crate servo_config; extern crate task_info; extern crate time as std_time; -#[allow(unsafe_code)] mod heartbeats; -#[allow(unsafe_code)] pub mod mem; -pub mod time; -pub mod trace_dump; +#[deny(unsafe_code)] pub mod time; +#[deny(unsafe_code)] pub mod trace_dump; + +#[cfg(not(target_os = "windows"))] +#[global_allocator] +static ALLOC: jemallocator::Jemalloc = jemallocator::Jemalloc; diff --git a/components/profile/mem.rs b/components/profile/mem.rs index 6046d84145c1..175cda58f721 100644 --- a/components/profile/mem.rs +++ b/components/profile/mem.rs @@ -354,14 +354,8 @@ impl ReportsForest { mod system_reporter { #[cfg(not(target_os = "windows"))] - use libc::{c_char, c_int, c_void, size_t}; + use libc::{c_int, size_t}; use profile_traits::mem::{Report, ReportKind, ReporterRequest}; - #[cfg(not(target_os = "windows"))] - use std::ffi::CString; - #[cfg(not(target_os = "windows"))] - use std::mem::size_of; - #[cfg(not(target_os = "windows"))] - use std::ptr::null_mut; use super::{JEMALLOC_HEAP_ALLOCATED_STR, SYSTEM_HEAP_ALLOCATED_STR}; #[cfg(target_os = "macos")] use task_info::task_basic_info::{virtual_size, resident_size}; @@ -397,17 +391,17 @@ mod system_reporter { // directly from the jemalloc documentation. // "Total number of bytes allocated by the application." - report(path![JEMALLOC_HEAP_ALLOCATED_STR], jemalloc_stat("stats.allocated")); + report(path![JEMALLOC_HEAP_ALLOCATED_STR], jemalloc_stat("stats.allocated\0")); // "Total number of bytes in active pages allocated by the application. // This is a multiple of the page size, and greater than or equal to // |stats.allocated|." - report(path!["jemalloc-heap-active"], jemalloc_stat("stats.active")); + report(path!["jemalloc-heap-active"], jemalloc_stat("stats.active\0")); // "Total number of bytes in chunks mapped on behalf of the application. // This is a multiple of the chunk size, and is at least as large as // |stats.active|. This does not include inactive chunks." - report(path!["jemalloc-heap-mapped"], jemalloc_stat("stats.mapped")); + report(path!["jemalloc-heap-mapped"], jemalloc_stat("stats.mapped\0")); } request.reports_channel.send(reports); @@ -457,47 +451,22 @@ mod system_reporter { None } - #[cfg(not(target_os = "windows"))] - extern { - #[cfg_attr(any(target_os = "macos", target_os = "android"), link_name = "je_mallctl")] - fn mallctl(name: *const c_char, oldp: *mut c_void, oldlenp: *mut size_t, - newp: *mut c_void, newlen: size_t) -> c_int; - } - #[cfg(not(target_os = "windows"))] fn jemalloc_stat(value_name: &str) -> Option { - // Before we request the measurement of interest, we first send an "epoch" - // request. Without that jemalloc gives cached statistics(!) which can be - // highly inaccurate. - let epoch_name = "epoch"; - let epoch_c_name = CString::new(epoch_name).unwrap(); - let mut epoch: u64 = 0; - let epoch_ptr = &mut epoch as *mut _ as *mut c_void; - let mut epoch_len = size_of::() as size_t; - - let value_c_name = CString::new(value_name).unwrap(); - let mut value: size_t = 0; - let value_ptr = &mut value as *mut _ as *mut c_void; - let mut value_len = size_of::() as size_t; - - // Using the same values for the `old` and `new` parameters is enough - // to get the statistics updated. - let rv = unsafe { - mallctl(epoch_c_name.as_ptr(), epoch_ptr, &mut epoch_len, epoch_ptr, - epoch_len) - }; - if rv != 0 { - return None; - } + unsafe { + // Before we request the measurement of interest, we first send an "epoch" + // request. Without that jemalloc gives cached statistics(!) which can be + // highly inaccurate. + if ::jemallocator::mallctl_set(b"epoch\0", 0_u64).is_err() { + return None + } - let rv = unsafe { - mallctl(value_c_name.as_ptr(), value_ptr, &mut value_len, null_mut(), 0) - }; - if rv != 0 { - return None; + let mut value: size_t = 0; + if ::jemallocator::mallctl_fetch(value_name.as_bytes(), &mut value).is_err() { + return None + } + Some(value as usize) } - - Some(value as usize) } #[cfg(target_os = "windows")]