From 6c18e508f10aa5bf42dc80691654b9cde2a661cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Fri, 24 Oct 2014 11:08:42 +0200 Subject: [PATCH 1/5] Make MIN_ALIGN a const to allow better optimization With MIN_ALIGN as a static, other crates don't have access to its value at compile time, because it is an extern global. That means that the checks against it can't be optimized out, which is rather unfortunate. So let's make it a constant instead. --- src/liballoc/heap.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/liballoc/heap.rs b/src/liballoc/heap.rs index 5a0f860ff844e..0e5aca18ec794 100644 --- a/src/liballoc/heap.rs +++ b/src/liballoc/heap.rs @@ -112,10 +112,10 @@ unsafe fn exchange_free(ptr: *mut u8, size: uint, align: uint) { #[cfg(any(target_arch = "arm", target_arch = "mips", target_arch = "mipsel"))] -static MIN_ALIGN: uint = 8; +const MIN_ALIGN: uint = 8; #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] -static MIN_ALIGN: uint = 16; +const MIN_ALIGN: uint = 16; #[cfg(jemalloc)] mod imp { From 6f253bd49e80c809b7c22fd257bcef06a8ca7c30 Mon Sep 17 00:00:00 2001 From: Daniel Micay Date: Fri, 24 Oct 2014 17:34:37 -0400 Subject: [PATCH 2/5] rm unnecessary libc allocator usage --- src/librustrt/mutex.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustrt/mutex.rs b/src/librustrt/mutex.rs index b328df1949e0e..838dfd6b7abe6 100644 --- a/src/librustrt/mutex.rs +++ b/src/librustrt/mutex.rs @@ -516,7 +516,7 @@ mod imp { #[cfg(windows)] mod imp { - use alloc::libc_heap::malloc_raw; + use alloc::heap; use core::atomic; use core::ptr; use libc::{HANDLE, BOOL, LPSECURITY_ATTRIBUTES, c_void, DWORD, LPCSTR}; @@ -607,7 +607,7 @@ mod imp { } pub unsafe fn init_lock() -> uint { - let block = malloc_raw(CRIT_SECTION_SIZE as uint) as *mut c_void; + let block = heap::allocate(CRIT_SECTION_SIZE, 8) as *mut c_void; InitializeCriticalSectionAndSpinCount(block, SPIN_COUNT); return block as uint; } @@ -619,7 +619,7 @@ mod imp { pub unsafe fn free_lock(h: uint) { DeleteCriticalSection(h as LPCRITICAL_SECTION); - libc::free(h as *mut c_void); + heap::deallocate(h as *mut u8, CRIT_SECTION_SIZE, 8); } pub unsafe fn free_cond(h: uint) { From 2bc4d3ec23cc88155173729e60df54c8aa4949a6 Mon Sep 17 00:00:00 2001 From: Daniel Micay Date: Fri, 24 Oct 2014 17:34:57 -0400 Subject: [PATCH 3/5] get rid of libc_heap::{malloc_raw, realloc_raw} The C standard library functions should be used directly. The quirky NULL / zero-size allocation workaround is no longer necessary and was adding an extra branch to the allocator code path in a build without jemalloc. This is a small step towards liballoc being compatible with handling OOM errors instead of aborting (#18292). [breaking-change] --- src/liballoc/heap.rs | 27 +++++++++++++++++----- src/liballoc/lib.rs | 8 ++----- src/liballoc/libc_heap.rs | 48 --------------------------------------- src/librustrt/c_str.rs | 7 +++--- src/libstd/c_vec.rs | 4 ++-- src/libstd/rt/mod.rs | 2 +- 6 files changed, 30 insertions(+), 66 deletions(-) delete mode 100644 src/liballoc/libc_heap.rs diff --git a/src/liballoc/heap.rs b/src/liballoc/heap.rs index 0e5aca18ec794..20569d336a857 100644 --- a/src/liballoc/heap.rs +++ b/src/liballoc/heap.rs @@ -213,8 +213,8 @@ mod imp { mod imp { use core::cmp; use core::ptr; + use core::ptr::RawPtr; use libc; - use libc_heap; use super::MIN_ALIGN; extern { @@ -226,7 +226,11 @@ mod imp { #[inline] pub unsafe fn allocate(size: uint, align: uint) -> *mut u8 { if align <= MIN_ALIGN { - libc_heap::malloc_raw(size) + let ptr = libc::malloc(size as libc::size_t); + if ptr.is_null() { + ::oom(); + } + ptr as *mut u8 } else { let mut out = 0 as *mut libc::c_void; let ret = posix_memalign(&mut out, @@ -242,7 +246,11 @@ mod imp { #[inline] pub unsafe fn reallocate(ptr: *mut u8, old_size: uint, size: uint, align: uint) -> *mut u8 { if align <= MIN_ALIGN { - libc_heap::realloc_raw(ptr, size) + let ptr = libc::realloc(ptr as *mut libc::c_void, size as libc::size_t); + if ptr.is_null() { + ::oom(); + } + ptr as *mut u8 } else { let new_ptr = allocate(size, align); ptr::copy_memory(new_ptr, ptr as *const u8, cmp::min(size, old_size)); @@ -274,7 +282,6 @@ mod imp { mod imp { use libc::{c_void, size_t}; use libc; - use libc_heap; use core::ptr::RawPtr; use super::MIN_ALIGN; @@ -288,7 +295,11 @@ mod imp { #[inline] pub unsafe fn allocate(size: uint, align: uint) -> *mut u8 { if align <= MIN_ALIGN { - libc_heap::malloc_raw(size) + let ptr = libc::malloc(size as size_t); + if ptr.is_null() { + ::oom(); + } + ptr as *mut u8 } else { let ptr = _aligned_malloc(size as size_t, align as size_t); if ptr.is_null() { @@ -301,7 +312,11 @@ mod imp { #[inline] pub unsafe fn reallocate(ptr: *mut u8, _old_size: uint, size: uint, align: uint) -> *mut u8 { if align <= MIN_ALIGN { - libc_heap::realloc_raw(ptr, size) + let ptr = libc::realloc(ptr as *mut c_void, size as size_t); + if ptr.is_null() { + ::oom(); + } + ptr as *mut u8 } else { let ptr = _aligned_realloc(ptr as *mut c_void, size as size_t, align as size_t); diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 2df9a585fec99..c721649ca9d4f 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -54,11 +54,8 @@ //! //! ## Heap interfaces //! -//! The [`heap`](heap/index.html) and [`libc_heap`](libc_heap/index.html) -//! modules are the unsafe interfaces to the underlying allocation systems. The -//! `heap` module is considered the default heap, and is not necessarily backed -//! by libc malloc/free. The `libc_heap` module is defined to be wired up to -//! the system malloc/free. +//! The [`heap`](heap/index.html) module defines the low-level interface to the +//! default global allocator. It is not compatible with the libc allocator API. #![crate_name = "alloc"] #![experimental] @@ -90,7 +87,6 @@ pub use boxed as owned; // Heaps provided for low-level allocation strategies pub mod heap; -pub mod libc_heap; // Primitive types using the heaps above diff --git a/src/liballoc/libc_heap.rs b/src/liballoc/libc_heap.rs deleted file mode 100644 index 4f6400630fd9c..0000000000000 --- a/src/liballoc/libc_heap.rs +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - - -//! The global (exchange) heap. - -use libc::{c_void, size_t, free, malloc, realloc}; -use core::ptr::{RawPtr, null_mut}; - -/// A wrapper around libc::malloc, aborting on out-of-memory. -#[inline] -pub unsafe fn malloc_raw(size: uint) -> *mut u8 { - // `malloc(0)` may allocate, but it may also return a null pointer - // http://pubs.opengroup.org/onlinepubs/9699919799/functions/malloc.html - if size == 0 { - null_mut() - } else { - let p = malloc(size as size_t); - if p.is_null() { - ::oom(); - } - p as *mut u8 - } -} - -/// A wrapper around libc::realloc, aborting on out-of-memory. -#[inline] -pub unsafe fn realloc_raw(ptr: *mut u8, size: uint) -> *mut u8 { - // `realloc(ptr, 0)` may allocate, but it may also return a null pointer - // http://pubs.opengroup.org/onlinepubs/9699919799/functions/realloc.html - if size == 0 { - free(ptr as *mut c_void); - null_mut() - } else { - let p = realloc(ptr as *mut c_void, size as size_t); - if p.is_null() { - ::oom(); - } - p as *mut u8 - } -} diff --git a/src/librustrt/c_str.rs b/src/librustrt/c_str.rs index 0ef0601828d73..6a33777a413c5 100644 --- a/src/librustrt/c_str.rs +++ b/src/librustrt/c_str.rs @@ -71,7 +71,6 @@ fn main() { */ -use alloc::libc_heap::malloc_raw; use collections::string::String; use collections::hash; use core::fmt; @@ -101,7 +100,8 @@ impl Clone for CString { /// with C's allocator API, rather than the usual shallow clone. fn clone(&self) -> CString { let len = self.len() + 1; - let buf = unsafe { malloc_raw(len) } as *mut libc::c_char; + let buf = unsafe { libc::malloc(len as libc::size_t) } as *mut libc::c_char; + if buf.is_null() { fail!("out of memory") } unsafe { ptr::copy_nonoverlapping_memory(buf, self.buf, len); } CString { buf: buf as *const libc::c_char, owns_buffer_: true } } @@ -393,7 +393,8 @@ impl<'a> ToCStr for &'a [u8] { unsafe fn to_c_str_unchecked(&self) -> CString { let self_len = self.len(); - let buf = malloc_raw(self_len + 1); + let buf = libc::malloc(self_len as libc::size_t + 1) as *mut u8; + if buf.is_null() { fail!("out of memory") } ptr::copy_memory(buf, self.as_ptr(), self_len); *buf.offset(self_len as int) = 0; diff --git a/src/libstd/c_vec.rs b/src/libstd/c_vec.rs index 95f8ab7201693..7ec25acb17308 100644 --- a/src/libstd/c_vec.rs +++ b/src/libstd/c_vec.rs @@ -165,11 +165,11 @@ mod tests { use super::CVec; use libc; use ptr; - use rt::libc_heap::malloc_raw; fn malloc(n: uint) -> CVec { unsafe { - let mem = malloc_raw(n); + let mem = libc::malloc(n as libc::size_t); + if mem.is_null() { fail!("out of memory") } CVec::new_with_dtor(mem as *mut u8, n, proc() { libc::free(mem as *mut libc::c_void); }) diff --git a/src/libstd/rt/mod.rs b/src/libstd/rt/mod.rs index 689bcdad1311c..a91c6c572e686 100644 --- a/src/libstd/rt/mod.rs +++ b/src/libstd/rt/mod.rs @@ -62,7 +62,7 @@ pub use self::util::{default_sched_threads, min_stack, running_on_valgrind}; // Reexport functionality from librustrt and other crates underneath the // standard library which work together to create the entire runtime. -pub use alloc::{heap, libc_heap}; +pub use alloc::heap; pub use rustrt::{task, local, mutex, exclusive, stack, args, rtio, thread}; pub use rustrt::{Stdio, Stdout, Stderr, begin_unwind, begin_unwind_fmt}; pub use rustrt::{bookkeeping, at_exit, unwind, DEFAULT_ERROR_CODE, Runtime}; From a6426cb43dd24d0949755da57da85f3739fd9230 Mon Sep 17 00:00:00 2001 From: Daniel Micay Date: Fri, 24 Oct 2014 19:58:26 -0400 Subject: [PATCH 4/5] return the new usable size from reallocate_inplace The real size is also more useful than just a boolean, and the caller can easily determine if the operation failed from the real size. In most cases, the caller is only going to be growing the allocation so a branch can be avoided. [breaking-change] --- src/liballoc/heap.rs | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/liballoc/heap.rs b/src/liballoc/heap.rs index 20569d336a857..6827ea1479d8a 100644 --- a/src/liballoc/heap.rs +++ b/src/liballoc/heap.rs @@ -38,8 +38,8 @@ pub unsafe fn reallocate(ptr: *mut u8, old_size: uint, size: uint, align: uint) /// Extends or shrinks the allocation referenced by `ptr` to `size` bytes of /// memory in-place. /// -/// Returns true if successful, otherwise false if the allocation was not -/// altered. +/// If the operation succeeds, it returns `usable_size(size, align)` and if it +/// fails (or is a no-op) it returns `usable_size(old_size, align)`. /// /// Behavior is undefined if the requested size is 0 or the alignment is not a /// power of 2. The alignment must be no larger than the largest supported page @@ -49,7 +49,7 @@ pub unsafe fn reallocate(ptr: *mut u8, old_size: uint, size: uint, align: uint) /// create the allocation referenced by `ptr`. The `old_size` parameter may be /// any value in range_inclusive(requested_size, usable_size). #[inline] -pub unsafe fn reallocate_inplace(ptr: *mut u8, old_size: uint, size: uint, align: uint) -> bool { +pub unsafe fn reallocate_inplace(ptr: *mut u8, old_size: uint, size: uint, align: uint) -> uint { imp::reallocate_inplace(ptr, old_size, size, align) } @@ -178,16 +178,10 @@ mod imp { } #[inline] - pub unsafe fn reallocate_inplace(ptr: *mut u8, old_size: uint, size: uint, - align: uint) -> bool { + pub unsafe fn reallocate_inplace(ptr: *mut u8, _old_size: uint, size: uint, + align: uint) -> uint { let flags = align_to_flags(align); - let new_size = je_xallocx(ptr as *mut c_void, size as size_t, 0, flags) as uint; - // checking for failure to shrink is tricky - if size < old_size { - usable_size(size, align) == new_size as uint - } else { - new_size >= size - } + je_xallocx(ptr as *mut c_void, size as size_t, 0, flags) as uint } #[inline] @@ -260,9 +254,9 @@ mod imp { } #[inline] - pub unsafe fn reallocate_inplace(_ptr: *mut u8, old_size: uint, size: uint, - _align: uint) -> bool { - size == old_size + pub unsafe fn reallocate_inplace(_ptr: *mut u8, old_size: uint, _size: uint, + _align: uint) -> uint { + old_size } #[inline] @@ -328,9 +322,9 @@ mod imp { } #[inline] - pub unsafe fn reallocate_inplace(_ptr: *mut u8, old_size: uint, size: uint, - _align: uint) -> bool { - size == old_size + pub unsafe fn reallocate_inplace(_ptr: *mut u8, old_size: uint, _size: uint, + _align: uint) -> uint { + old_size } #[inline] @@ -363,7 +357,7 @@ mod test { let ptr = heap::allocate(size, 8); let ret = heap::reallocate_inplace(ptr, size, size, 8); heap::deallocate(ptr, size, 8); - assert!(ret); + assert_eq!(ret, heap::usable_size(size, 8)); } } From a9e85100cd598ddb9395b2ad31b31e0f9df84f22 Mon Sep 17 00:00:00 2001 From: Daniel Micay Date: Fri, 24 Oct 2014 20:11:28 -0400 Subject: [PATCH 5/5] fix sized deallocation documentation --- src/liballoc/heap.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/liballoc/heap.rs b/src/liballoc/heap.rs index 6827ea1479d8a..c780170d47aaf 100644 --- a/src/liballoc/heap.rs +++ b/src/liballoc/heap.rs @@ -28,8 +28,8 @@ pub unsafe fn allocate(size: uint, align: uint) -> *mut u8 { /// size on the platform. /// /// The `old_size` and `align` parameters are the parameters that were used to -/// create the allocation referenced by `ptr`. The `old_size` parameter may also -/// be the value returned by `usable_size` for the requested size. +/// create the allocation referenced by `ptr`. The `old_size` parameter may be +/// any value in range_inclusive(requested_size, usable_size). #[inline] pub unsafe fn reallocate(ptr: *mut u8, old_size: uint, size: uint, align: uint) -> *mut u8 { imp::reallocate(ptr, old_size, size, align) @@ -57,12 +57,12 @@ pub unsafe fn reallocate_inplace(ptr: *mut u8, old_size: uint, size: uint, align /// /// The `ptr` parameter must not be null. /// -/// The `size` and `align` parameters are the parameters that were used to -/// create the allocation referenced by `ptr`. The `size` parameter may also be -/// the value returned by `usable_size` for the requested size. +/// The `old_size` and `align` parameters are the parameters that were used to +/// create the allocation referenced by `ptr`. The `old_size` parameter may be +/// any value in range_inclusive(requested_size, usable_size). #[inline] -pub unsafe fn deallocate(ptr: *mut u8, size: uint, align: uint) { - imp::deallocate(ptr, size, align) +pub unsafe fn deallocate(ptr: *mut u8, old_size: uint, align: uint) { + imp::deallocate(ptr, old_size, align) } /// Returns the usable size of an allocation created with the specified the @@ -102,8 +102,8 @@ unsafe fn exchange_malloc(size: uint, align: uint) -> *mut u8 { #[cfg(not(test))] #[lang="exchange_free"] #[inline] -unsafe fn exchange_free(ptr: *mut u8, size: uint, align: uint) { - deallocate(ptr, size, align); +unsafe fn exchange_free(ptr: *mut u8, old_size: uint, align: uint) { + deallocate(ptr, old_size, align); } // The minimum alignment guaranteed by the architecture. This value is used to @@ -185,9 +185,9 @@ mod imp { } #[inline] - pub unsafe fn deallocate(ptr: *mut u8, size: uint, align: uint) { + pub unsafe fn deallocate(ptr: *mut u8, old_size: uint, align: uint) { let flags = align_to_flags(align); - je_sdallocx(ptr as *mut c_void, size as size_t, flags) + je_sdallocx(ptr as *mut c_void, old_size as size_t, flags) } #[inline] @@ -260,7 +260,7 @@ mod imp { } #[inline] - pub unsafe fn deallocate(ptr: *mut u8, _size: uint, _align: uint) { + pub unsafe fn deallocate(ptr: *mut u8, _old_size: uint, _align: uint) { libc::free(ptr as *mut libc::c_void) } @@ -328,7 +328,7 @@ mod imp { } #[inline] - pub unsafe fn deallocate(ptr: *mut u8, _size: uint, align: uint) { + pub unsafe fn deallocate(ptr: *mut u8, _old_size: uint, align: uint) { if align <= MIN_ALIGN { libc::free(ptr as *mut libc::c_void) } else {