Auto merge of #90919 - nnethercote:rm-DropArena, r=Mark-Simulacrum

bors · bors · commit d914f17ca71a · 2021-11-16T11:48:37.000Z
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.
diff --git a/compiler/rustc_arena/src/lib.rs b/compiler/rustc_arena/src/lib.rs
@@ -19,7 +19,6 @@
 #![feature(rustc_attrs)]
 #![cfg_attr(test, feature(test))]
 
-use rustc_data_structures::sync;
 use smallvec::SmallVec;
 
 use std::alloc::Layout;
@@ -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 {
@@ -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)
                 }
             }
 
@@ -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)
                 }
             }
         }
diff --git a/compiler/rustc_hir/src/arena.rs b/compiler/rustc_hir/src/arena.rs
@@ -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,
@@ -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>,
diff --git a/compiler/rustc_middle/src/arena.rs b/compiler/rustc_middle/src/arena.rs
@@ -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]
@@ -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,
@@ -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>,
