diff options
| author | Daniel Micay <danielmicay@gmail.com> | 2013-06-29 22:35:04 -0400 |
|---|---|---|
| committer | Daniel Micay <danielmicay@gmail.com> | 2013-06-30 03:45:36 -0400 |
| commit | b883d6a54c460f8357b1107b3599108eb1f8580b (patch) | |
| tree | 64589d59861624f36771f0f1d1a300fe4d49ffec | |
| parent | 439b13f071a4a884ea8645670df83162ffcf129f (diff) | |
| download | rust-b883d6a54c460f8357b1107b3599108eb1f8580b.tar.gz rust-b883d6a54c460f8357b1107b3599108eb1f8580b.zip | |
simplify the exchange allocator
* stop using an atomic counter, this has a significant cost and valgrind will already catch these leaks * remove the extra layer of function calls * remove the assert of non-null in free, freeing null is well defined but throwing a failure from free will not be * stop initializing the `prev`/`next` pointers * abort on out-of-memory, failing won't necessarily work
| -rw-r--r-- | src/libstd/os.rs | 6 | ||||
| -rw-r--r-- | src/libstd/rt/global_heap.rs | 94 | ||||
| -rw-r--r-- | src/libstd/unstable/exchange_alloc.rs | 83 | ||||
| -rw-r--r-- | src/libstd/unstable/lang.rs | 17 | ||||
| -rw-r--r-- | src/rt/rust_exchange_alloc.cpp | 16 | ||||
| -rw-r--r-- | src/rt/rust_exchange_alloc.h | 6 | ||||
| -rw-r--r-- | src/rt/rust_kernel.cpp | 1 | ||||
| -rw-r--r-- | src/rt/rustrt.def.in | 1 |
8 files changed, 44 insertions, 180 deletions
diff --git a/src/libstd/os.rs b/src/libstd/os.rs index 1fbcda12dce..9b74754d711 100644 --- a/src/libstd/os.rs +++ b/src/libstd/os.rs @@ -730,7 +730,7 @@ pub fn list_dir(p: &Path) -> ~[~str] { #[cfg(windows)] unsafe fn get_list(p: &Path) -> ~[~str] { use libc::consts::os::extra::INVALID_HANDLE_VALUE; - use libc::wcslen; + use libc::{wcslen, free}; use libc::funcs::extra::kernel32::{ FindFirstFileW, FindNextFileW, @@ -739,7 +739,7 @@ pub fn list_dir(p: &Path) -> ~[~str] { use os::win32::{ as_utf16_p }; - use rt::global_heap::{malloc_raw, free_raw}; + use rt::global_heap::malloc_raw; #[nolink] extern { unsafe fn rust_list_dir_wfd_size() -> libc::size_t; @@ -772,7 +772,7 @@ pub fn list_dir(p: &Path) -> ~[~str] { ::cast::transmute(wfd_ptr)); } FindClose(find_handle); - free_raw(wfd_ptr); + free(wfd_ptr) } strings } diff --git a/src/libstd/rt/global_heap.rs b/src/libstd/rt/global_heap.rs index 1e9f9aab834..3994b722f59 100644 --- a/src/libstd/rt/global_heap.rs +++ b/src/libstd/rt/global_heap.rs @@ -8,62 +8,21 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use sys::{size_of}; -use libc::{c_void, size_t, uintptr_t}; -use c_malloc = libc::malloc; -use c_free = libc::free; +use libc::{c_char, c_void, size_t, uintptr_t, free, malloc}; use managed::raw::{BoxHeaderRepr, BoxRepr}; -use cast::transmute; -use unstable::intrinsics::{atomic_xadd,atomic_xsub,TyDesc}; -use ptr::null; +use unstable::intrinsics::TyDesc; +use sys::size_of; -pub unsafe fn malloc(td: *TyDesc, size: uint) -> *c_void { - assert!(td.is_not_null()); - - let total_size = get_box_size(size, (*td).align); - let p = c_malloc(total_size as size_t); - assert!(p.is_not_null()); - - let box: &mut BoxRepr = transmute(p); - box.header.ref_count = -1; // Exchange values not ref counted - box.header.type_desc = td; - box.header.prev = null(); - box.header.next = null(); - - let exchange_count = &mut *exchange_count_ptr(); - atomic_xadd(exchange_count, 1); - - return transmute(box); -} -/** -Thin wrapper around libc::malloc, none of the box header -stuff in exchange_alloc::malloc -*/ -pub unsafe fn malloc_raw(size: uint) -> *c_void { - let p = c_malloc(size as size_t); - if p.is_null() { - fail!("Failure in malloc_raw: result ptr is null"); - } - p -} - -pub unsafe fn free(ptr: *c_void) { - let exchange_count = &mut *exchange_count_ptr(); - atomic_xsub(exchange_count, 1); - - assert!(ptr.is_not_null()); - c_free(ptr); -} -///Thin wrapper around libc::free, as with exchange_alloc::malloc_raw -pub unsafe fn free_raw(ptr: *c_void) { - c_free(ptr); +extern { + #[rust_stack] + fn abort(); } fn get_box_size(body_size: uint, body_align: uint) -> uint { let header_size = size_of::<BoxHeaderRepr>(); // FIXME (#2699): This alignment calculation is suspicious. Is it right? let total_size = align_to(header_size, body_align) + body_size; - return total_size; + total_size } // Rounds |size| to the nearest |alignment|. Invariant: |alignment| is a power @@ -73,11 +32,40 @@ fn align_to(size: uint, align: uint) -> uint { (size + align - 1) & !(align - 1) } -fn exchange_count_ptr() -> *mut int { - // XXX: Need mutable globals - unsafe { transmute(&rust_exchange_count) } +/// A wrapper around libc::malloc, aborting on out-of-memory +pub unsafe fn malloc_raw(size: uint) -> *c_void { + let p = malloc(size as size_t); + if p.is_null() { + // we need a non-allocating way to print an error here + abort(); + } + p } -extern { - static rust_exchange_count: uintptr_t; +// FIXME #4942: Make these signatures agree with exchange_alloc's signatures +#[lang="exchange_malloc"] +#[inline] +pub unsafe fn exchange_malloc(td: *c_char, size: uintptr_t) -> *c_char { + let td = td as *TyDesc; + let size = size as uint; + + assert!(td.is_not_null()); + + let total_size = get_box_size(size, (*td).align); + let p = malloc_raw(total_size as uint); + + let box: *mut BoxRepr = p as *mut BoxRepr; + (*box).header.ref_count = -1; // Exchange values not ref counted + (*box).header.type_desc = td; + + box as *c_char +} + +// NB: Calls to free CANNOT be allowed to fail, as throwing an exception from +// inside a landing pad may corrupt the state of the exception handler. +#[cfg(not(test))] +#[lang="exchange_free"] +#[inline] +pub unsafe fn exchange_free(ptr: *c_char) { + free(ptr as *c_void); } diff --git a/src/libstd/unstable/exchange_alloc.rs b/src/libstd/unstable/exchange_alloc.rs deleted file mode 100644 index 5c47901df48..00000000000 --- a/src/libstd/unstable/exchange_alloc.rs +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright 2012 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 <LICENSE-APACHE or -// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license -// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -use sys::size_of; -use libc::{c_void, size_t}; -use c_malloc = libc::malloc; -use c_free = libc::free; -use managed::raw::{BoxHeaderRepr, BoxRepr}; -use cast::transmute; -use unstable::intrinsics::{atomic_xadd,atomic_xsub}; -use ptr::null; -#[cfg(stage0)] -use intrinsic::TyDesc; -#[cfg(not(stage0))] -use unstable::intrinsics::TyDesc; - -pub unsafe fn malloc(td: *TyDesc, size: uint) -> *c_void { - assert!(td.is_not_null()); - - let total_size = get_box_size(size, (*td).align); - let p = c_malloc(total_size as size_t); - assert!(p.is_not_null()); - - let box: &mut BoxRepr = transmute(p); - box.header.ref_count = -1; // Exchange values not ref counted - box.header.type_desc = td; - box.header.prev = null(); - box.header.next = null(); - - let exchange_count = &mut *rust_get_exchange_count_ptr(); - atomic_xadd(exchange_count, 1); - - return transmute(box); -} -/** -Thin wrapper around libc::malloc, none of the box header -stuff in exchange_alloc::malloc -*/ -pub unsafe fn malloc_raw(size: uint) -> *c_void { - let p = c_malloc(size as size_t); - if p.is_null() { - fail!("Failure in malloc_raw: result ptr is null"); - } - p -} - -pub unsafe fn free(ptr: *c_void) { - let exchange_count = &mut *rust_get_exchange_count_ptr(); - atomic_xsub(exchange_count, 1); - - assert!(ptr.is_not_null()); - c_free(ptr); -} -///Thin wrapper around libc::free, as with exchange_alloc::malloc_raw -pub unsafe fn free_raw(ptr: *c_void) { - c_free(ptr); -} - -fn get_box_size(body_size: uint, body_align: uint) -> uint { - let header_size = size_of::<BoxHeaderRepr>(); - // FIXME (#2699): This alignment calculation is suspicious. Is it right? - let total_size = align_to(header_size, body_align) + body_size; - return total_size; -} - -// Rounds |size| to the nearest |alignment|. Invariant: |alignment| is a power -// of two. -fn align_to(size: uint, align: uint) -> uint { - assert!(align != 0); - (size + align - 1) & !(align - 1) -} - -extern { - #[rust_stack] - fn rust_get_exchange_count_ptr() -> *mut int; -} diff --git a/src/libstd/unstable/lang.rs b/src/libstd/unstable/lang.rs index d37579b0c47..fddd847af34 100644 --- a/src/libstd/unstable/lang.rs +++ b/src/libstd/unstable/lang.rs @@ -22,7 +22,6 @@ use rt::task::Task; use rt::local::Local; use option::{Option, Some, None}; use io; -use rt::global_heap; #[allow(non_camel_case_types)] pub type rust_task = c_void; @@ -150,13 +149,6 @@ unsafe fn fail_borrowed(box: *mut BoxRepr, file: *c_char, line: size_t) { } } -// FIXME #4942: Make these signatures agree with exchange_alloc's signatures -#[lang="exchange_malloc"] -#[inline] -pub unsafe fn exchange_malloc(td: *c_char, size: uintptr_t) -> *c_char { - transmute(global_heap::malloc(transmute(td), transmute(size))) -} - /// Because this code is so perf. sensitive, use a static constant so that /// debug printouts are compiled out most of the time. static ENABLE_DEBUG: bool = false; @@ -228,15 +220,6 @@ impl DebugPrints for io::fd_t { } } -// NB: Calls to free CANNOT be allowed to fail, as throwing an exception from -// inside a landing pad may corrupt the state of the exception handler. If a -// problem occurs, call exit instead. -#[lang="exchange_free"] -#[inline] -pub unsafe fn exchange_free(ptr: *c_char) { - global_heap::free(transmute(ptr)) -} - #[lang="malloc"] pub unsafe fn local_malloc(td: *c_char, size: uintptr_t) -> *c_char { match context() { diff --git a/src/rt/rust_exchange_alloc.cpp b/src/rt/rust_exchange_alloc.cpp index 89257dc9f6e..658d97031ce 100644 --- a/src/rt/rust_exchange_alloc.cpp +++ b/src/rt/rust_exchange_alloc.cpp @@ -15,16 +15,10 @@ #include <string.h> #include <stdio.h> -extern uintptr_t rust_exchange_count; -uintptr_t rust_exchange_count = 0; - void * rust_exchange_alloc::malloc(size_t size) { void *value = ::malloc(size); assert(value); - - sync::increment(rust_exchange_count); - return value; } @@ -37,15 +31,5 @@ rust_exchange_alloc::realloc(void *ptr, size_t size) { void rust_exchange_alloc::free(void *ptr) { - sync::decrement(rust_exchange_count); ::free(ptr); } - -void -rust_check_exchange_count_on_exit() { - if (rust_exchange_count != 0) { - printf("exchange heap not empty on exit\n"); - printf("%d dangling allocations\n", (int)rust_exchange_count); - abort(); - } -} diff --git a/src/rt/rust_exchange_alloc.h b/src/rt/rust_exchange_alloc.h index 767caf01323..9699ef6b5e9 100644 --- a/src/rt/rust_exchange_alloc.h +++ b/src/rt/rust_exchange_alloc.h @@ -21,10 +21,4 @@ class rust_exchange_alloc { void free(void *mem); }; -extern "C" uintptr_t * -rust_get_exchange_count_ptr(); - -void -rust_check_exchange_count_on_exit(); - #endif diff --git a/src/rt/rust_kernel.cpp b/src/rt/rust_kernel.cpp index c1c40222f1a..583f836c0d6 100644 --- a/src/rt/rust_kernel.cpp +++ b/src/rt/rust_kernel.cpp @@ -211,7 +211,6 @@ rust_kernel::run() { assert(osmain_driver != NULL); osmain_driver->start_main_loop(); sched_reaper.join(); - rust_check_exchange_count_on_exit(); return rval; } diff --git a/src/rt/rustrt.def.in b/src/rt/rustrt.def.in index a4876618c97..b572f1aba6a 100644 --- a/src/rt/rustrt.def.in +++ b/src/rt/rustrt.def.in @@ -196,7 +196,6 @@ rust_register_exit_function rust_get_global_data_ptr rust_inc_kernel_live_count rust_dec_kernel_live_count -rust_exchange_count rust_get_rt_tls_key swap_registers rust_readdir |
