Skip to content

Commit 66f5d6b

Browse files
committed
feat(allocator): optimize memory usage with dual allocator pools for import plugin
Add `AllocatorPool::new_dual()` to support both standard and fixed-size allocators in a single pool, allowing runtime selection based on file purpose. This reduces memory usage when using the import plugin by using standard allocators (~few MB) for dependency files that are only parsed for module resolution, while reserving fixed-size allocators (~2 GiB address space) only for entry files that need linting with JS plugins. Fixes #15799
1 parent 32f4fdd commit 66f5d6b

File tree

5 files changed

+314
-43
lines changed

5 files changed

+314
-43
lines changed

apps/oxlint/src/js_plugins/external_linter.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,11 @@ fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb {
7777
let (tx, rx) = channel();
7878

7979
// SAFETY: This function is only called when an `ExternalLinter` exists.
80-
// When that is the case, the `AllocatorPool` used to create `Allocator`s is created with
81-
// `AllocatorPool::new_fixed_size`, so all `Allocator`s are created via `FixedSizeAllocator`.
82-
// This is somewhat sketchy, as we don't have a type-level guarantee of this invariant,
83-
// but it does hold at present.
84-
// When we replace `bumpalo` with a custom allocator, we can close this soundness hole.
85-
// TODO: Do that.
80+
// When that is the case, the `AllocatorPool` is created with `AllocatorPool::new_dual`,
81+
// and `Allocator`s for files that need linting are created via `FixedSizeAllocator`
82+
// using `get_maybe_fixed_size(true)`.
83+
// This is validated with debug assertions in the linter runtime before calling this function.
84+
// The `AllocatorGuard` tracks whether an allocator is fixed-size via the `is_fixed_size` field.
8685
let (buffer_id, buffer) = unsafe { get_buffer(allocator) };
8786

8887
// Send data to JS

apps/oxlint/src/js_plugins/raw_fs.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ use oxc_linter::RuntimeFileSystem;
1414
/// Identical to `OsFileSystem`, except that `read_to_arena_str` reads the file's contents into
1515
/// start of the allocator, instead of the end. This conforms to what raw transfer needs.
1616
///
17-
/// Must only be used in conjunction with `AllocatorPool` created with `new_fixed_size`,
18-
/// which wraps `Allocator`s with a custom `Drop` impl, which makes `read_to_arena_str` safe.
17+
/// Must only be used with fixed-size `Allocator`s (created via `AllocatorPool::new_fixed_size`
18+
/// or `AllocatorPool::new_dual` with `get_maybe_fixed_size(true)`),
19+
/// which have a custom `Drop` impl that makes `read_to_arena_str` safe.
1920
///
2021
/// This is a temporary solution. When we replace `bumpalo` with our own allocator, all strings
2122
/// will be written at start of the arena, so then `OsFileSystem` will work fine, and we can

crates/oxc_allocator/src/pool/mod.rs

Lines changed: 250 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,39 +18,70 @@ pub use fixed_size::{FixedSizeAllocatorMetadata, free_fixed_size_allocator};
1818

1919
/// A thread-safe pool for reusing [`Allocator`] instances to reduce allocation overhead.
2020
///
21-
/// Uses either:
22-
/// 1. Standard allocators - suitable for general use.
23-
/// 2. Fixed-size allocators - compatible with raw transfer.
21+
/// Supports three configurations:
22+
/// 1. Standard allocators only - suitable for general use ([`AllocatorPool::new`]).
23+
/// 2. Fixed-size allocators only - compatible with raw transfer ([`AllocatorPool::new_fixed_size`]).
24+
/// 3. Dual mode with both types - allows choosing allocator type at runtime ([`AllocatorPool::new_dual`]).
2425
///
25-
/// Standard allocator pool is created by [`AllocatorPool::new`].
26-
/// Fixed-size allocator pool is created by [`AllocatorPool::new_fixed_size`].
26+
/// The dual mode is useful when some allocations need raw transfer (e.g., files to be linted with JS plugins)
27+
/// while others don't (e.g., dependency files only parsed for module resolution).
2728
///
2829
/// Fixed-size allocators are only supported on 64-bit little-endian platforms at present,
2930
/// and require the `fixed_size` Cargo feature to be enabled.
30-
#[repr(transparent)]
31-
pub struct AllocatorPool(AllocatorPoolInner);
32-
33-
/// Inner type of [`AllocatorPool`], holding either a standard or fixed-size allocator pool.
34-
enum AllocatorPoolInner {
35-
Standard(StandardAllocatorPool),
31+
pub struct AllocatorPool {
32+
standard: StandardAllocatorPool,
3633
#[cfg(all(feature = "fixed_size", target_pointer_width = "64", target_endian = "little"))]
37-
FixedSize(FixedSizeAllocatorPool),
34+
fixed_size: Option<FixedSizeAllocatorPool>,
3835
}
3936

4037
impl AllocatorPool {
4138
/// Create a new [`AllocatorPool`] for use across the specified number of threads,
42-
/// which uses standard allocators.
39+
/// which uses standard allocators only.
4340
pub fn new(thread_count: usize) -> AllocatorPool {
44-
Self(AllocatorPoolInner::Standard(StandardAllocatorPool::new(thread_count)))
41+
Self {
42+
standard: StandardAllocatorPool::new(thread_count),
43+
#[cfg(all(
44+
feature = "fixed_size",
45+
target_pointer_width = "64",
46+
target_endian = "little"
47+
))]
48+
fixed_size: None,
49+
}
4550
}
4651

4752
/// Create a new [`AllocatorPool`] for use across the specified number of threads,
48-
/// which uses fixed-size allocators (suitable for raw transfer).
53+
/// which uses fixed-size allocators only (suitable for raw transfer).
4954
#[cfg(feature = "fixed_size")]
5055
pub fn new_fixed_size(thread_count: usize) -> AllocatorPool {
5156
#[cfg(all(target_pointer_width = "64", target_endian = "little"))]
5257
{
53-
Self(AllocatorPoolInner::FixedSize(FixedSizeAllocatorPool::new(thread_count)))
58+
Self {
59+
standard: StandardAllocatorPool::new(0), // Not used, but need to initialize
60+
fixed_size: Some(FixedSizeAllocatorPool::new(thread_count)),
61+
}
62+
}
63+
64+
#[cfg(not(all(target_pointer_width = "64", target_endian = "little")))]
65+
{
66+
let _thread_count = thread_count; // Avoid unused vars lint warning
67+
panic!("Fixed size allocators are only supported on 64-bit little-endian platforms");
68+
}
69+
}
70+
71+
/// Create a new [`AllocatorPool`] for use across the specified number of threads,
72+
/// which contains both standard and fixed-size allocators.
73+
///
74+
/// This allows choosing the allocator type at runtime via [`AllocatorPool::get_maybe_fixed_size`].
75+
/// Useful when some allocations need raw transfer (e.g., files to be linted with JS plugins)
76+
/// while others don't (e.g., dependency files only parsed for module resolution).
77+
#[cfg(feature = "fixed_size")]
78+
pub fn new_dual(thread_count: usize) -> AllocatorPool {
79+
#[cfg(all(target_pointer_width = "64", target_endian = "little"))]
80+
{
81+
Self {
82+
standard: StandardAllocatorPool::new(thread_count),
83+
fixed_size: Some(FixedSizeAllocatorPool::new(thread_count)),
84+
}
5485
}
5586

5687
#[cfg(not(all(target_pointer_width = "64", target_endian = "little")))]
@@ -64,21 +95,87 @@ impl AllocatorPool {
6495
///
6596
/// Returns an [`AllocatorGuard`] that gives access to the allocator.
6697
///
98+
/// If the pool was created with [`AllocatorPool::new_fixed_size`], returns a fixed-size allocator.
99+
/// Otherwise, returns a standard allocator.
100+
///
67101
/// # Panics
68102
///
69103
/// Panics if the underlying mutex is poisoned.
70104
pub fn get(&self) -> AllocatorGuard<'_> {
71-
let allocator = match &self.0 {
72-
AllocatorPoolInner::Standard(pool) => pool.get(),
105+
#[cfg(all(feature = "fixed_size", target_pointer_width = "64", target_endian = "little"))]
106+
{
107+
if let Some(ref fixed_size_pool) = self.fixed_size {
108+
let allocator = fixed_size_pool.get();
109+
return AllocatorGuard {
110+
allocator: ManuallyDrop::new(allocator),
111+
is_fixed_size: true,
112+
pool: self,
113+
};
114+
}
115+
}
116+
117+
let allocator = self.standard.get();
118+
AllocatorGuard {
119+
allocator: ManuallyDrop::new(allocator),
73120
#[cfg(all(
74121
feature = "fixed_size",
75122
target_pointer_width = "64",
76123
target_endian = "little"
77124
))]
78-
AllocatorPoolInner::FixedSize(pool) => pool.get(),
79-
};
125+
is_fixed_size: false,
126+
pool: self,
127+
}
128+
}
129+
130+
/// Retrieve an [`Allocator`] from the pool, choosing between standard and fixed-size allocators.
131+
///
132+
/// Returns an [`AllocatorGuard`] that gives access to the allocator.
133+
///
134+
/// # Parameters
135+
///
136+
/// * `fixed_size` - If `true`, returns a fixed-size allocator (suitable for raw transfer).
137+
/// If `false`, returns a standard allocator.
138+
///
139+
/// # Panics
140+
///
141+
/// Panics if:
142+
/// - The underlying mutex is poisoned.
143+
/// - `fixed_size` is `true` but the pool was not created with [`AllocatorPool::new_dual`] or [`AllocatorPool::new_fixed_size`].
144+
#[cfg(feature = "fixed_size")]
145+
pub fn get_maybe_fixed_size(&self, fixed_size: bool) -> AllocatorGuard<'_> {
146+
#[cfg(all(target_pointer_width = "64", target_endian = "little"))]
147+
{
148+
if fixed_size {
149+
let Some(ref fixed_size_pool) = self.fixed_size else {
150+
panic!(
151+
"Requested fixed-size allocator but pool was not created with new_dual() or new_fixed_size()"
152+
);
153+
};
154+
let allocator = fixed_size_pool.get();
155+
return AllocatorGuard {
156+
allocator: ManuallyDrop::new(allocator),
157+
is_fixed_size: true,
158+
pool: self,
159+
};
160+
}
161+
}
80162

81-
AllocatorGuard { allocator: ManuallyDrop::new(allocator), pool: self }
163+
#[cfg(not(all(target_pointer_width = "64", target_endian = "little")))]
164+
if fixed_size {
165+
panic!("Fixed size allocators are only supported on 64-bit little-endian platforms");
166+
}
167+
168+
let allocator = self.standard.get();
169+
AllocatorGuard {
170+
allocator: ManuallyDrop::new(allocator),
171+
#[cfg(all(
172+
feature = "fixed_size",
173+
target_pointer_width = "64",
174+
target_endian = "little"
175+
))]
176+
is_fixed_size: false,
177+
pool: self,
178+
}
82179
}
83180

84181
/// Add an [`Allocator`] to the pool.
@@ -88,20 +185,37 @@ impl AllocatorPool {
88185
/// # Panics
89186
///
90187
/// Panics if the underlying mutex is poisoned.
188+
#[cfg(all(feature = "fixed_size", target_pointer_width = "64", target_endian = "little"))]
189+
fn add(&self, allocator: Allocator, is_fixed_size: bool) {
190+
// SAFETY: This method is only called from `AllocatorGuard::drop`.
191+
// `AllocatorGuard`s are only created by `AllocatorPool::get` or `AllocatorPool::get_maybe_fixed_size`,
192+
// so the `Allocator` must have been created by this pool. Therefore, it is the correct type for the pool.
193+
unsafe {
194+
if is_fixed_size {
195+
if let Some(ref fixed_size_pool) = self.fixed_size {
196+
fixed_size_pool.add(allocator);
197+
} else {
198+
// This should never happen if `AllocatorGuard` is used correctly
199+
unreachable!("Fixed-size allocator returned to pool without fixed-size pool");
200+
}
201+
} else {
202+
self.standard.add(allocator);
203+
}
204+
}
205+
}
206+
207+
/// Add an [`Allocator`] to the pool (for platforms without fixed-size support).
208+
#[cfg(not(all(
209+
feature = "fixed_size",
210+
target_pointer_width = "64",
211+
target_endian = "little"
212+
)))]
91213
fn add(&self, allocator: Allocator) {
92214
// SAFETY: This method is only called from `AllocatorGuard::drop`.
93215
// `AllocatorGuard`s are only created by `AllocatorPool::get`, so the `Allocator` must have
94216
// been created by this pool. Therefore, it is the correct type for the pool.
95217
unsafe {
96-
match &self.0 {
97-
AllocatorPoolInner::Standard(pool) => pool.add(allocator),
98-
#[cfg(all(
99-
feature = "fixed_size",
100-
target_pointer_width = "64",
101-
target_endian = "little"
102-
))]
103-
AllocatorPoolInner::FixedSize(pool) => pool.add(allocator),
104-
}
218+
self.standard.add(allocator);
105219
}
106220
}
107221
}
@@ -111,9 +225,19 @@ impl AllocatorPool {
111225
/// On drop, the `Allocator` is reset and returned to the pool.
112226
pub struct AllocatorGuard<'alloc_pool> {
113227
allocator: ManuallyDrop<Allocator>,
228+
#[cfg(all(feature = "fixed_size", target_pointer_width = "64", target_endian = "little"))]
229+
is_fixed_size: bool,
114230
pool: &'alloc_pool AllocatorPool,
115231
}
116232

233+
impl AllocatorGuard<'_> {
234+
/// Returns `true` if this allocator is a fixed-size allocator (suitable for raw transfer).
235+
#[cfg(all(feature = "fixed_size", target_pointer_width = "64", target_endian = "little"))]
236+
pub fn is_fixed_size(&self) -> bool {
237+
self.is_fixed_size
238+
}
239+
}
240+
117241
impl Deref for AllocatorGuard<'_> {
118242
type Target = Allocator;
119243

@@ -127,6 +251,102 @@ impl Drop for AllocatorGuard<'_> {
127251
fn drop(&mut self) {
128252
// SAFETY: After taking ownership of the `Allocator`, we do not touch the `ManuallyDrop` again
129253
let allocator = unsafe { ManuallyDrop::take(&mut self.allocator) };
254+
255+
#[cfg(all(feature = "fixed_size", target_pointer_width = "64", target_endian = "little"))]
256+
self.pool.add(allocator, self.is_fixed_size);
257+
258+
#[cfg(not(all(
259+
feature = "fixed_size",
260+
target_pointer_width = "64",
261+
target_endian = "little"
262+
)))]
130263
self.pool.add(allocator);
131264
}
132265
}
266+
267+
#[cfg(test)]
268+
mod tests {
269+
use super::*;
270+
271+
#[test]
272+
fn test_standard_pool() {
273+
let pool = AllocatorPool::new(2);
274+
let guard1 = pool.get();
275+
let guard2 = pool.get();
276+
277+
// Allocate something to ensure allocators work
278+
let _ = guard1.alloc(42);
279+
let _ = guard2.alloc(24);
280+
281+
drop(guard1);
282+
drop(guard2);
283+
284+
// Get allocators again to ensure they were returned to pool
285+
let guard3 = pool.get();
286+
let _ = guard3.alloc(100);
287+
}
288+
289+
#[test]
290+
#[cfg(feature = "fixed_size")]
291+
fn test_fixed_size_pool() {
292+
let pool = AllocatorPool::new_fixed_size(2);
293+
let guard = pool.get();
294+
295+
#[cfg(all(target_pointer_width = "64", target_endian = "little"))]
296+
assert!(guard.is_fixed_size());
297+
298+
let _ = guard.alloc(42);
299+
}
300+
301+
#[test]
302+
#[cfg(feature = "fixed_size")]
303+
fn test_dual_pool() {
304+
#[cfg(all(target_pointer_width = "64", target_endian = "little"))]
305+
{
306+
let pool = AllocatorPool::new_dual(2);
307+
308+
// Get a standard allocator
309+
let standard_guard = pool.get_maybe_fixed_size(false);
310+
assert!(!standard_guard.is_fixed_size());
311+
let _ = standard_guard.alloc(42);
312+
drop(standard_guard);
313+
314+
// Get a fixed-size allocator
315+
let fixed_guard = pool.get_maybe_fixed_size(true);
316+
assert!(fixed_guard.is_fixed_size());
317+
let _ = fixed_guard.alloc(24);
318+
drop(fixed_guard);
319+
320+
// Get them again to ensure they were returned to their respective pools
321+
let standard_guard2 = pool.get_maybe_fixed_size(false);
322+
assert!(!standard_guard2.is_fixed_size());
323+
324+
let fixed_guard2 = pool.get_maybe_fixed_size(true);
325+
assert!(fixed_guard2.is_fixed_size());
326+
}
327+
328+
#[cfg(not(all(target_pointer_width = "64", target_endian = "little")))]
329+
{
330+
// On non-64-bit platforms, fixed-size allocators are not supported
331+
// This test should still compile but won't run the dual pool test
332+
}
333+
}
334+
335+
#[test]
336+
#[cfg(feature = "fixed_size")]
337+
#[should_panic(
338+
expected = "Requested fixed-size allocator but pool was not created with new_dual() or new_fixed_size()"
339+
)]
340+
fn test_get_fixed_size_from_standard_pool_panics() {
341+
#[cfg(all(target_pointer_width = "64", target_endian = "little"))]
342+
{
343+
let pool = AllocatorPool::new(2);
344+
let _ = pool.get_maybe_fixed_size(true); // Should panic
345+
}
346+
347+
#[cfg(not(all(target_pointer_width = "64", target_endian = "little")))]
348+
panic!(
349+
"Requested fixed-size allocator but pool was not created with new_dual() or new_fixed_size()"
350+
);
351+
}
352+
}

crates/oxc_linter/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ rust-version.workspace = true
1414
description.workspace = true
1515

1616
[features]
17-
default = []
17+
default = ["fixed_size"]
18+
fixed_size = ["oxc_allocator/fixed_size"]
1819
ruledocs = ["oxc_macros/ruledocs"] # Enables the `ruledocs` feature for conditional compilation
1920
force_test_reporter = []
2021

0 commit comments

Comments
 (0)