Skip to content

Commit 99dd1a7

Browse files
committed
refactor(allocator/pool): share AllocatorGuard between pool impls (#13621)
Refactor. We have 2 versions of `AllocatorPool` (standard and fixed-size). Aim is to combine them into one. This is a step along that way. Previously both versions of `AllocatorPool` had very similar `AllocatorGuard` implementations. Replace them with a single `AllocatorGuard` implementation, which both versions share.
1 parent 0dde7f0 commit 99dd1a7

File tree

3 files changed

+55
-61
lines changed

3 files changed

+55
-61
lines changed

crates/oxc_allocator/src/pool/fixed_size.rs

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use std::{
22
alloc::{self, GlobalAlloc, Layout, System},
3-
mem::ManuallyDrop,
4-
ops::Deref,
3+
mem::{self, ManuallyDrop},
54
ptr::NonNull,
65
sync::{
76
Mutex,
@@ -16,6 +15,8 @@ use crate::{
1615
generated::fixed_size_constants::{BLOCK_ALIGN, BLOCK_SIZE, RAW_METADATA_SIZE},
1716
};
1817

18+
use super::AllocatorGuard;
19+
1920
const TWO_GIB: usize = 1 << 31;
2021
const FOUR_GIB: usize = 1 << 32;
2122

@@ -46,12 +47,12 @@ impl AllocatorPool {
4647
///
4748
/// Panics if the underlying mutex is poisoned.
4849
pub fn get(&self) -> AllocatorGuard<'_> {
49-
let allocator = {
50+
let fixed_size_allocator = {
5051
let mut allocators = self.allocators.lock().unwrap();
5152
allocators.pop()
5253
};
5354

54-
let allocator = allocator.unwrap_or_else(|| {
55+
let fixed_size_allocator = fixed_size_allocator.unwrap_or_else(|| {
5556
// Each allocator needs to have a unique ID, but the order those IDs are assigned in
5657
// doesn't matter, so `Ordering::Relaxed` is fine
5758
let id = self.next_id.fetch_add(1, Ordering::Relaxed);
@@ -61,45 +62,31 @@ impl AllocatorPool {
6162
FixedSizeAllocator::new(id)
6263
});
6364

64-
AllocatorGuard { allocator: ManuallyDrop::new(allocator), pool: self }
65+
// Unwrap `FixedSizeAllocator`.
66+
// `add` method will wrap it again, before returning it to pool, ensuring it gets dropped properly.
67+
// SAFETY: `FixedSizeAllocator` is just a wrapper around `ManuallyDrop<Allocator>`,
68+
// and is `#[repr(transparent)]`, so the 2 are equivalent.
69+
let allocator = unsafe {
70+
mem::transmute::<FixedSizeAllocator, ManuallyDrop<Allocator>>(fixed_size_allocator)
71+
};
72+
73+
AllocatorGuard { allocator, pool: self }
6574
}
6675

67-
/// Add a [`FixedSizeAllocator`] to the pool.
76+
/// Add an [`Allocator`] to the pool.
6877
///
6978
/// The `Allocator` is reset by this method, so it's ready to be re-used.
7079
///
7180
/// # Panics
7281
///
7382
/// Panics if the underlying mutex is poisoned.
74-
fn add(&self, mut allocator: FixedSizeAllocator) {
75-
allocator.reset();
76-
let mut allocators = self.allocators.lock().unwrap();
77-
allocators.push(allocator);
78-
}
79-
}
83+
pub(super) fn add(&self, allocator: Allocator) {
84+
let mut fixed_size_allocator =
85+
FixedSizeAllocator { allocator: ManuallyDrop::new(allocator) };
86+
fixed_size_allocator.reset();
8087

81-
/// A guard object representing exclusive access to an [`Allocator`] from the pool.
82-
///
83-
/// On drop, the `Allocator` is reset and returned to the pool.
84-
pub struct AllocatorGuard<'alloc_pool> {
85-
allocator: ManuallyDrop<FixedSizeAllocator>,
86-
pool: &'alloc_pool AllocatorPool,
87-
}
88-
89-
impl Deref for AllocatorGuard<'_> {
90-
type Target = Allocator;
91-
92-
fn deref(&self) -> &Self::Target {
93-
&self.allocator.allocator
94-
}
95-
}
96-
97-
impl Drop for AllocatorGuard<'_> {
98-
/// Return [`Allocator`] back to the pool.
99-
fn drop(&mut self) {
100-
// SAFETY: After taking ownership of the `FixedSizeAllocator`, we do not touch the `ManuallyDrop` again
101-
let allocator = unsafe { ManuallyDrop::take(&mut self.allocator) };
102-
self.pool.add(allocator);
88+
let mut allocators = self.allocators.lock().unwrap();
89+
allocators.push(fixed_size_allocator);
10390
}
10491
}
10592

@@ -198,6 +185,7 @@ const ALLOC_LAYOUT: Layout = match Layout::from_size_align(ALLOC_SIZE, ALLOC_ALI
198185
/// * `BLOCK_SIZE` is a multiple of 16.
199186
/// * `RawTransferMetadata` is 16 bytes.
200187
/// * Size of `FixedSizeAllocatorMetadata` is rounded up to a multiple of 16.
188+
#[repr(transparent)]
201189
struct FixedSizeAllocator {
202190
/// `Allocator` which utilizes part of the original allocation
203191
allocator: ManuallyDrop<Allocator>,

crates/oxc_allocator/src/pool/mod.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
use std::{mem::ManuallyDrop, ops::Deref};
2+
3+
use crate::Allocator;
4+
15
// Fixed size allocators are only supported on 64-bit little-endian platforms at present.
26
// They are only enabled if `fixed_size` feature enabled, and `disable_fixed_size` feature is not enabled.
37
//
@@ -31,3 +35,28 @@ mod standard;
3135
target_endian = "little"
3236
)))]
3337
pub use standard::*;
38+
39+
/// A guard object representing exclusive access to an [`Allocator`] from the pool.
40+
///
41+
/// On drop, the `Allocator` is reset and returned to the pool.
42+
pub struct AllocatorGuard<'alloc_pool> {
43+
allocator: ManuallyDrop<Allocator>,
44+
pool: &'alloc_pool AllocatorPool,
45+
}
46+
47+
impl Deref for AllocatorGuard<'_> {
48+
type Target = Allocator;
49+
50+
fn deref(&self) -> &Self::Target {
51+
&self.allocator
52+
}
53+
}
54+
55+
impl Drop for AllocatorGuard<'_> {
56+
/// Return [`Allocator`] back to the pool.
57+
fn drop(&mut self) {
58+
// SAFETY: After taking ownership of the `Allocator`, we do not touch the `ManuallyDrop` again
59+
let allocator = unsafe { ManuallyDrop::take(&mut self.allocator) };
60+
self.pool.add(allocator);
61+
}
62+
}

crates/oxc_allocator/src/pool/standard.rs

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
use std::{iter, mem::ManuallyDrop, ops::Deref, sync::Mutex};
1+
use std::{iter, mem::ManuallyDrop, sync::Mutex};
22

33
use crate::Allocator;
44

5+
use super::AllocatorGuard;
6+
57
/// A thread-safe pool for reusing [`Allocator`] instances to reduce allocation overhead.
68
///
79
/// Internally uses a `Vec` protected by a `Mutex` to store available allocators.
@@ -40,38 +42,13 @@ impl AllocatorPool {
4042
/// # Panics
4143
///
4244
/// Panics if the underlying mutex is poisoned.
43-
fn add(&self, mut allocator: Allocator) {
45+
pub(super) fn add(&self, mut allocator: Allocator) {
4446
allocator.reset();
4547
let mut allocators = self.allocators.lock().unwrap();
4648
allocators.push(allocator);
4749
}
4850
}
4951

50-
/// A guard object representing exclusive access to an [`Allocator`] from the pool.
51-
///
52-
/// On drop, the `Allocator` is reset and returned to the pool.
53-
pub struct AllocatorGuard<'alloc_pool> {
54-
allocator: ManuallyDrop<Allocator>,
55-
pool: &'alloc_pool AllocatorPool,
56-
}
57-
58-
impl Deref for AllocatorGuard<'_> {
59-
type Target = Allocator;
60-
61-
fn deref(&self) -> &Self::Target {
62-
&self.allocator
63-
}
64-
}
65-
66-
impl Drop for AllocatorGuard<'_> {
67-
/// Return [`Allocator`] back to the pool.
68-
fn drop(&mut self) {
69-
// SAFETY: After taking ownership of the `Allocator`, we do not touch the `ManuallyDrop` again
70-
let allocator = unsafe { ManuallyDrop::take(&mut self.allocator) };
71-
self.pool.add(allocator);
72-
}
73-
}
74-
7552
// Dummy implementations of interfaces from `fixed_size`, just to stop clippy complaining.
7653
// Seems to be necessary due to feature unification.
7754
#[allow(

0 commit comments

Comments
 (0)