Skip to content

Commit

Permalink
Auto merge of rust-lang#90919 - nnethercote:rm-DropArena, r=Mark-Simu…
Browse files Browse the repository at this point in the history 
…lacrum

Remove `DropArena`.

Most arena-allocate types that impl `Drop` get their own `TypedArena`, but a
few infrequently used ones share a `DropArena`. This sharing adds complexity
but doesn't help performance or memory usage. Perhaps it was more effective in
the past prior to some other improvements to arenas.

This commit removes `DropArena` and the sharing of arenas via the `few`
attribute of the `arena_types` macro. This change removes over 100 lines of
code and nine uses of `unsafe` (one of which affects the parallel compiler) and
makes the remaining code easier to read.
  • Loading branch information
@bors
bors committed Nov 16, 2021
2 parents 934624f + fb80c73 commit d914f17
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 154 deletions.
141 changes: 7 additions & 134 deletions compiler/rustc_arena/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#![feature(rustc_attrs)]
#![cfg_attr(test, feature(test))]

use rustc_data_structures::sync;
use smallvec::SmallVec;

use std::alloc::Layout;
Expand Down Expand Up @@ -517,130 +516,12 @@ impl DroplessArena {
}
}

/// Calls the destructor for an object when dropped.
struct DropType {
drop_fn: unsafe fn(*mut u8),
obj: *mut u8,
}

// SAFETY: we require `T: Send` before type-erasing into `DropType`.
#[cfg(parallel_compiler)]
unsafe impl sync::Send for DropType {}

impl DropType {
#[inline]
unsafe fn new<T: sync::Send>(obj: *mut T) -> Self {
unsafe fn drop_for_type<T>(to_drop: *mut u8) {
std::ptr::drop_in_place(to_drop as *mut T)
}

DropType { drop_fn: drop_for_type::<T>, obj: obj as *mut u8 }
}
}

impl Drop for DropType {
fn drop(&mut self) {
unsafe { (self.drop_fn)(self.obj) }
}
}

/// An arena which can be used to allocate any type.
///
/// # Safety
///
/// Allocating in this arena is unsafe since the type system
/// doesn't know which types it contains. In order to
/// allocate safely, you must store a `PhantomData<T>`
/// alongside this arena for each type `T` you allocate.
#[derive(Default)]
pub struct DropArena {
/// A list of destructors to run when the arena drops.
/// Ordered so `destructors` gets dropped before the arena
/// since its destructor can reference memory in the arena.
destructors: RefCell<Vec<DropType>>,
arena: DroplessArena,
}

impl DropArena {
#[inline]
pub unsafe fn alloc<T>(&self, object: T) -> &mut T
where
T: sync::Send,
{
let mem = self.arena.alloc_raw(Layout::new::<T>()) as *mut T;
// Write into uninitialized memory.
ptr::write(mem, object);
let result = &mut *mem;
// Record the destructor after doing the allocation as that may panic
// and would cause `object`'s destructor to run twice if it was recorded before.
self.destructors.borrow_mut().push(DropType::new(result));
result
}

#[inline]
pub unsafe fn alloc_from_iter<T, I>(&self, iter: I) -> &mut [T]
where
T: sync::Send,
I: IntoIterator<Item = T>,
{
let mut vec: SmallVec<[_; 8]> = iter.into_iter().collect();
if vec.is_empty() {
return &mut [];
}
let len = vec.len();

let start_ptr = self.arena.alloc_raw(Layout::array::<T>(len).unwrap()) as *mut T;

let mut destructors = self.destructors.borrow_mut();
// Reserve space for the destructors so we can't panic while adding them.
destructors.reserve(len);

// Move the content to the arena by copying it and then forgetting
// the content of the SmallVec.
vec.as_ptr().copy_to_nonoverlapping(start_ptr, len);
mem::forget(vec.drain(..));

// Record the destructors after doing the allocation as that may panic
// and would cause `object`'s destructor to run twice if it was recorded before.
for i in 0..len {
destructors.push(DropType::new(start_ptr.add(i)));
}

slice::from_raw_parts_mut(start_ptr, len)
}
}

pub macro arena_for_type {
([][$ty:ty]) => {
$crate::TypedArena<$ty>
},
([few $(, $attrs:ident)*][$ty:ty]) => {
::std::marker::PhantomData<$ty>
},
([$ignore:ident $(, $attrs:ident)*]$args:tt) => {
$crate::arena_for_type!([$($attrs),*]$args)
},
}

pub macro which_arena_for_type {
([][$arena:expr]) => {
::std::option::Option::Some($arena)
},
([few$(, $attrs:ident)*][$arena:expr]) => {
::std::option::Option::None
},
([$ignore:ident$(, $attrs:ident)*]$args:tt) => {
$crate::which_arena_for_type!([$($attrs),*]$args)
},
}

#[rustc_macro_transparency = "semitransparent"]
pub macro declare_arena([$($a:tt $name:ident: $ty:ty,)*], $tcx:lifetime) {
#[derive(Default)]
pub struct Arena<$tcx> {
pub dropless: $crate::DroplessArena,
drop: $crate::DropArena,
$($name: $crate::arena_for_type!($a[$ty]),)*
$($name: $crate::TypedArena<$ty>,)*
}

pub trait ArenaAllocatable<'tcx, T = Self>: Sized {
Expand Down Expand Up @@ -670,13 +551,9 @@ pub macro declare_arena([$($a:tt $name:ident: $ty:ty,)*], $tcx:lifetime) {
#[inline]
fn allocate_on<'a>(self, arena: &'a Arena<$tcx>) -> &'a mut Self {
if !::std::mem::needs_drop::<Self>() {
return arena.dropless.alloc(self);
}
match $crate::which_arena_for_type!($a[&arena.$name]) {
::std::option::Option::<&$crate::TypedArena<Self>>::Some(ty_arena) => {
ty_arena.alloc(self)
}
::std::option::Option::None => unsafe { arena.drop.alloc(self) },
arena.dropless.alloc(self)
} else {
arena.$name.alloc(self)
}
}

Expand All @@ -686,13 +563,9 @@ pub macro declare_arena([$($a:tt $name:ident: $ty:ty,)*], $tcx:lifetime) {
iter: impl ::std::iter::IntoIterator<Item = Self>,
) -> &'a mut [Self] {
if !::std::mem::needs_drop::<Self>() {
return arena.dropless.alloc_from_iter(iter);
}
match $crate::which_arena_for_type!($a[&arena.$name]) {
::std::option::Option::<&$crate::TypedArena<Self>>::Some(ty_arena) => {
ty_arena.alloc_from_iter(iter)
}
::std::option::Option::None => unsafe { arena.drop.alloc_from_iter(iter) },
arena.dropless.alloc_from_iter(iter)
} else {
arena.$name.alloc_from_iter(iter)
}
}
}
Expand Down
15 changes: 5 additions & 10 deletions compiler/rustc_hir/src/arena.rs
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
/// This declares a list of types which can be allocated by `Arena`.
///
/// The `few` modifier will cause allocation to use the shared arena and recording the destructor.
/// This is faster and more memory efficient if there's only a few allocations of the type.
/// Leaving `few` out will cause the type to get its own dedicated `TypedArena` which is
/// faster and more memory efficient if there is lots of allocations.
///
/// Specifying the `decode` modifier will add decode impls for `&T` and `&[T]`,
/// where `T` is the type listed. These impls will appear in the implement_ty_decoder! macro.
#[macro_export]
macro_rules! arena_types {
($macro:path, $tcx:lifetime) => (
$macro!([
// HIR types
[few] hir_krate: rustc_hir::Crate<$tcx>,
[] hir_krate: rustc_hir::Crate<$tcx>,
[] arm: rustc_hir::Arm<$tcx>,
[] asm_operand: (rustc_hir::InlineAsmOperand<$tcx>, Span),
[] asm_template: rustc_ast::InlineAsmTemplatePiece,
Expand All @@ -29,14 +24,14 @@ macro_rules! arena_types {
[] pat_field: rustc_hir::PatField<$tcx>,
[] fn_decl: rustc_hir::FnDecl<$tcx>,
[] foreign_item: rustc_hir::ForeignItem<$tcx>,
[few] foreign_item_ref: rustc_hir::ForeignItemRef,
[] foreign_item_ref: rustc_hir::ForeignItemRef,
[] impl_item: rustc_hir::ImplItem<$tcx>,
[] impl_item_ref: rustc_hir::ImplItemRef,
[] item: rustc_hir::Item<$tcx>,
[few] inline_asm: rustc_hir::InlineAsm<$tcx>,
[few] llvm_inline_asm: rustc_hir::LlvmInlineAsm<$tcx>,
[] inline_asm: rustc_hir::InlineAsm<$tcx>,
[] llvm_inline_asm: rustc_hir::LlvmInlineAsm<$tcx>,
[] local: rustc_hir::Local<$tcx>,
[few] mod_: rustc_hir::Mod<$tcx>,
[] mod_: rustc_hir::Mod<$tcx>,
[] owner_info: rustc_hir::OwnerInfo<$tcx>,
[] param: rustc_hir::Param<$tcx>,
[] pat: rustc_hir::Pat<$tcx>,
Expand Down
15 changes: 5 additions & 10 deletions compiler/rustc_middle/src/arena.rs
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
/// This declares a list of types which can be allocated by `Arena`.
///
/// The `few` modifier will cause allocation to use the shared arena and recording the destructor.
/// This is faster and more memory efficient if there's only a few allocations of the type.
/// Leaving `few` out will cause the type to get its own dedicated `TypedArena` which is
/// faster and more memory efficient if there is lots of allocations.
///
/// Specifying the `decode` modifier will add decode impls for `&T` and `&[T]` where `T` is the type
/// listed. These impls will appear in the implement_ty_decoder! macro.
#[macro_export]
Expand Down Expand Up @@ -37,7 +32,7 @@ macro_rules! arena_types {
[decode] code_region: rustc_middle::mir::coverage::CodeRegion,
[] const_allocs: rustc_middle::mir::interpret::Allocation,
// Required for the incremental on-disk cache
[few] mir_keys: rustc_hir::def_id::DefIdSet,
[] mir_keys: rustc_hir::def_id::DefIdSet,
[] region_scope_tree: rustc_middle::middle::region::ScopeTree,
[] dropck_outlives:
rustc_middle::infer::canonical::Canonical<'tcx,
Expand Down Expand Up @@ -77,10 +72,10 @@ macro_rules! arena_types {
rustc_middle::infer::canonical::Canonical<'tcx,
rustc_middle::infer::canonical::QueryResponse<'tcx, rustc_middle::ty::Ty<'tcx>>
>,
[few] all_traits: Vec<rustc_hir::def_id::DefId>,
[few] privacy_access_levels: rustc_middle::middle::privacy::AccessLevels,
[few] foreign_module: rustc_session::cstore::ForeignModule,
[few] foreign_modules: Vec<rustc_session::cstore::ForeignModule>,
[] all_traits: Vec<rustc_hir::def_id::DefId>,
[] privacy_access_levels: rustc_middle::middle::privacy::AccessLevels,
[] foreign_module: rustc_session::cstore::ForeignModule,
[] foreign_modules: Vec<rustc_session::cstore::ForeignModule>,
[] upvars_mentioned: rustc_data_structures::fx::FxIndexMap<rustc_hir::HirId, rustc_hir::Upvar>,
[] object_safety_violations: rustc_middle::traits::ObjectSafetyViolation,
[] codegen_unit: rustc_middle::mir::mono::CodegenUnit<$tcx>,
Expand Down

0 comments on commit d914f17

Please sign in to comment.