Skip to content

Commit

Permalink
Stop relying on linking details of std’s default allocator
Browse files Browse the repository at this point in the history
We’ve been bitten before by symbol names changing:
servo/heapsize#46
and upstream is planning to stop using jemalloc by default:
rust-lang/rust#33082 (comment)

So use the (relatively) new `#[global_allocator]` attribute
to explicitly select the system allocator on Windows
and jemalloc (now in an external crate) on other platforms.
This choice matches current defaults.
  • Loading branch information
SimonSapin committed Oct 18, 2017
1 parent 4c538b6 commit 608ad5b
Show file tree
Hide file tree
Showing 20 changed files with 157 additions and 62 deletions.
40 changes: 39 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions components/allocator/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[package]
name = "servo_allocator"
version = "0.0.1"
authors = ["The Servo Project Developers"]
license = "MPL-2.0"
publish = false

[lib]
path = "lib.rs"

[features]
unstable = ["kernel32-sys", "jemallocator"]

[target.'cfg(not(windows))'.dependencies]
jemallocator = { version = "0.1.3", optional = true }

[target.'cfg(windows)'.dependencies]
kernel32-sys = { version = "0.2.1", optional = true }
61 changes: 61 additions & 0 deletions components/allocator/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* 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/. */

//! Selecting the default global allocator for Servo

#![cfg_attr(feature = "unstable", feature(global_allocator))]

#[cfg(feature = "unstable")]
#[global_allocator]
static ALLOC: platform::Allocator = platform::Allocator;

pub use platform::usable_size;


#[cfg(all(feature = "unstable", not(windows)))]
mod platform {
extern crate jemallocator;

pub use self::jemallocator::Jemalloc as Allocator;
use std::os::raw::c_void;

/// Get the size of a heap block.
pub unsafe extern "C" fn usable_size(ptr: *const c_void) -> usize {
jemallocator::usable_size(ptr)
}
}

#[cfg(all(feature = "unstable", windows))]
mod platform {
extern crate alloc_system;
extern crate kernel32;

pub use self::alloc_system::System as Allocator;
use self::kernel32::{GetProcessHeap, HeapSize, HeapValidate};
use std::os::raw::c_void;

/// Get the size of a heap block.
pub unsafe extern "C" fn malloc_size_of(mut ptr: *const c_void) -> usize {
let heap = GetProcessHeap();

if HeapValidate(heap, 0, ptr) == 0 {
ptr = *(ptr as *const *const c_void).offset(-1);
}

HeapSize(heap, 0, ptr) as usize
}
}

#[cfg(not(feature = "unstable"))]
mod platform {
use std::os::raw::c_void;

/// Without `#[global_allocator]` we cannot be certain of what allocator is used
/// or how it is linked. We therefore disable memory reporting. (Return zero.)
pub unsafe extern "C" fn usable_size(_ptr: *const c_void) -> usize {
0
}
}


1 change: 1 addition & 0 deletions components/gfx/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ core-text = "7.0"

[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
freetype = "0.3"
servo_allocator = {path = "../allocator"}

[target.'cfg(target_os = "linux")'.dependencies]
servo-fontconfig = "0.2.1"
Expand Down
4 changes: 2 additions & 2 deletions components/gfx/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ extern crate fnv;
#[cfg(target_os = "linux")]
extern crate fontconfig;
extern crate fontsan;
#[cfg(any(target_os = "linux", target_os = "android"))]
extern crate freetype;
#[cfg(any(target_os = "linux", target_os = "android"))] extern crate freetype;
#[cfg(any(target_os = "linux", target_os = "android"))] extern crate servo_allocator;
extern crate gfx_traits;

// Eventually we would like the shaper to be pluggable, as many operating systems have their own
Expand Down
16 changes: 9 additions & 7 deletions components/gfx/platform/freetype/font_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use freetype::freetype::FT_Library;
use freetype::freetype::FT_Memory;
use freetype::freetype::FT_MemoryRec_;
use freetype::freetype::FT_New_Library;
use malloc_size_of::{malloc_size_of, MallocSizeOf, MallocSizeOfOps};
use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
use servo_allocator::usable_size;
use std::mem;
use std::os::raw::{c_long, c_void};
use std::ptr;
Expand All @@ -31,7 +32,7 @@ extern fn ft_alloc(mem: FT_Memory, req_size: c_long) -> *mut c_void {
mem::forget(vec);

unsafe {
let actual_size = malloc_size_of(ptr as *const _);
let actual_size = usable_size(ptr as *const _);
let user = (*mem).user as *mut User;
(*user).size += actual_size;
}
Expand All @@ -41,7 +42,7 @@ extern fn ft_alloc(mem: FT_Memory, req_size: c_long) -> *mut c_void {

extern fn ft_free(mem: FT_Memory, ptr: *mut c_void) {
unsafe {
let actual_size = malloc_size_of(ptr as *const _);
let actual_size = usable_size(ptr as *const _);
let user = (*mem).user as *mut User;
(*user).size -= actual_size;

Expand All @@ -50,13 +51,14 @@ extern fn ft_free(mem: FT_Memory, ptr: *mut c_void) {
}
}

extern fn ft_realloc(mem: FT_Memory, _cur_size: c_long, new_req_size: c_long,
extern fn ft_realloc(mem: FT_Memory, old_size: c_long, new_req_size: c_long,
old_ptr: *mut c_void) -> *mut c_void {
let old_actual_size;
let mut vec;
unsafe {
old_actual_size = malloc_size_of(old_ptr as *const _);
vec = Vec::<u8>::from_raw_parts(old_ptr as *mut u8, old_actual_size, old_actual_size);
old_actual_size = usable_size(old_ptr as *const _);
let old_size = old_size as usize;
vec = Vec::<u8>::from_raw_parts(old_ptr as *mut u8, old_size, old_size);
};

let new_req_size = new_req_size as usize;
Expand All @@ -71,7 +73,7 @@ extern fn ft_realloc(mem: FT_Memory, _cur_size: c_long, new_req_size: c_long,
mem::forget(vec);

unsafe {
let new_actual_size = malloc_size_of(new_ptr as *const _);
let new_actual_size = usable_size(new_ptr as *const _);
let user = (*mem).user as *mut User;
(*user).size += new_actual_size;
(*user).size -= old_actual_size;
Expand Down
1 change: 1 addition & 0 deletions components/layout_thread/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ script_layout_interface = {path = "../script_layout_interface"}
script_traits = {path = "../script_traits"}
selectors = { path = "../selectors" }
serde_json = "1.0"
servo_allocator = {path = "../allocator"}
servo_arc = {path = "../servo_arc"}
servo_atoms = {path = "../atoms"}
servo_config = {path = "../config"}
Expand Down
5 changes: 3 additions & 2 deletions components/layout_thread/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ extern crate script_layout_interface;
extern crate script_traits;
extern crate selectors;
extern crate serde_json;
extern crate servo_allocator;
extern crate servo_arc;
extern crate servo_atoms;
extern crate servo_config;
Expand Down Expand Up @@ -84,7 +85,7 @@ use layout::webrender_helpers::WebRenderDisplayListConverter;
use layout::wrapper::LayoutNodeLayoutData;
use layout_traits::LayoutThreadFactory;
use libc::c_void;
use malloc_size_of::{malloc_size_of, MallocSizeOf, MallocSizeOfOps};
use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
use metrics::{PaintTimeMetrics, ProfilerMetadataFactory};
use msg::constellation_msg::PipelineId;
use msg::constellation_msg::TopLevelBrowsingContextId;
Expand Down Expand Up @@ -775,7 +776,7 @@ impl LayoutThread {
let mut reports = vec![];
// Servo uses vanilla jemalloc, which doesn't have a
// malloc_enclosing_size_of function.
let mut ops = MallocSizeOfOps::new(malloc_size_of, None, None);
let mut ops = MallocSizeOfOps::new(::servo_allocator::usable_size, None, None);

// FIXME(njn): Just measuring the display tree for now.
let rw_data = possibly_locked_rw_data.lock();
Expand Down
3 changes: 0 additions & 3 deletions components/malloc_size_of/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ publish = false
[lib]
path = "lib.rs"

[target.'cfg(windows)'.dependencies]
kernel32-sys = "0.2.1"

[features]
servo = ["js", "string_cache", "url", "webrender_api", "xml5ever"]

Expand Down
31 changes: 0 additions & 31 deletions components/malloc_size_of/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ extern crate euclid;
extern crate hashglobe;
#[cfg(feature = "servo")]
extern crate js;
#[cfg(target_os = "windows")]
extern crate kernel32;
extern crate servo_arc;
extern crate smallbitvec;
extern crate smallvec;
Expand All @@ -63,8 +61,6 @@ extern crate webrender_api;
#[cfg(feature = "servo")]
extern crate xml5ever;

#[cfg(target_os = "windows")]
use kernel32::{GetProcessHeap, HeapSize, HeapValidate};
use std::hash::{BuildHasher, Hash};
use std::mem::size_of;
use std::ops::Range;
Expand Down Expand Up @@ -146,33 +142,6 @@ impl MallocSizeOfOps {
}
}

/// Get the size of a heap block.
#[cfg(not(target_os = "windows"))]
pub unsafe extern "C" fn malloc_size_of(ptr: *const c_void) -> usize {
// The C prototype is `je_malloc_usable_size(JEMALLOC_USABLE_SIZE_CONST void *ptr)`. On some
// platforms `JEMALLOC_USABLE_SIZE_CONST` is `const` and on some it is empty. But in practice
// this function doesn't modify the contents of the block that `ptr` points to, so we use
// `*const c_void` here.
extern "C" {
#[cfg_attr(any(prefixed_jemalloc, target_os = "macos", target_os = "ios", target_os = "android"),
link_name = "je_malloc_usable_size")]
fn malloc_usable_size(ptr: *const c_void) -> usize;
}
malloc_usable_size(ptr)
}

/// Get the size of a heap block.
#[cfg(target_os = "windows")]
pub unsafe extern "C" fn malloc_size_of(mut ptr: *const c_void) -> usize {
let heap = GetProcessHeap();

if HeapValidate(heap, 0, ptr) == 0 {
ptr = *(ptr as *const *const c_void).offset(-1);
}

HeapSize(heap, 0, ptr) as usize
}

/// Trait for measuring the "deep" heap usage of a data structure. This is the
/// most commonly-used of the traits.
pub trait MallocSizeOf {
Expand Down
3 changes: 2 additions & 1 deletion components/profile/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ name = "profile"
path = "lib.rs"

[features]
unstable = []
unstable = ["jemalloc-sys"]

[dependencies]
profile_traits = {path = "../profile_traits"}
Expand All @@ -31,3 +31,4 @@ regex = "0.2"

[target.'cfg(not(target_os = "windows"))'.dependencies]
libc = "0.2"
jemalloc-sys = {version = "0.1.3", optional = true}
Loading

0 comments on commit 608ad5b

Please sign in to comment.