Skip to content

Commit 2e84003

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 2191ae9 commit 2e84003

File tree

4 files changed

+197
-43
lines changed

4 files changed

+197
-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: 151 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: 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: FixedSizeAllocatorPool::new(0),
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: 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: FixedSizeAllocatorPool::new(thread_count),
84+
}
5485
}
5586

5687
#[cfg(not(all(target_pointer_width = "64", target_endian = "little")))]
@@ -64,21 +95,80 @@ 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+
if cfg!(all(feature = "fixed_size", target_pointer_width = "64", target_endian = "little"))
106+
{
107+
let allocator = self.fixed_size.get();
108+
return AllocatorGuard {
109+
allocator: ManuallyDrop::new(allocator),
110+
is_fixed_size: true,
111+
pool: self,
112+
};
113+
}
114+
115+
let allocator = self.standard.get();
116+
AllocatorGuard {
117+
allocator: ManuallyDrop::new(allocator),
73118
#[cfg(all(
74119
feature = "fixed_size",
75120
target_pointer_width = "64",
76121
target_endian = "little"
77122
))]
78-
AllocatorPoolInner::FixedSize(pool) => pool.get(),
79-
};
123+
is_fixed_size: false,
124+
pool: self,
125+
}
126+
}
80127

81-
AllocatorGuard { allocator: ManuallyDrop::new(allocator), pool: self }
128+
/// Retrieve an [`Allocator`] from the pool, choosing between standard and fixed-size allocators.
129+
///
130+
/// Returns an [`AllocatorGuard`] that gives access to the allocator.
131+
///
132+
/// # Parameters
133+
///
134+
/// * `fixed_size` - If `true`, returns a fixed-size allocator (suitable for raw transfer).
135+
/// If `false`, returns a standard allocator.
136+
///
137+
/// # Panics
138+
///
139+
/// Panics if:
140+
/// - The underlying mutex is poisoned.
141+
/// - `fixed_size` is `true` but the pool was not created with [`AllocatorPool::new_dual`] or [`AllocatorPool::new_fixed_size`].
142+
#[cfg(feature = "fixed_size")]
143+
pub fn get_maybe_fixed_size(&self, fixed_size: bool) -> AllocatorGuard<'_> {
144+
#[cfg(all(target_pointer_width = "64", target_endian = "little"))]
145+
{
146+
if fixed_size {
147+
let allocator = self.fixed_size.get();
148+
return AllocatorGuard {
149+
allocator: ManuallyDrop::new(allocator),
150+
is_fixed_size: true,
151+
pool: self,
152+
};
153+
}
154+
}
155+
156+
#[cfg(not(all(target_pointer_width = "64", target_endian = "little")))]
157+
if fixed_size {
158+
panic!("Fixed size allocators are only supported on 64-bit little-endian platforms");
159+
}
160+
161+
let allocator = self.standard.get();
162+
AllocatorGuard {
163+
allocator: ManuallyDrop::new(allocator),
164+
#[cfg(all(
165+
feature = "fixed_size",
166+
target_pointer_width = "64",
167+
target_endian = "little"
168+
))]
169+
is_fixed_size: false,
170+
pool: self,
171+
}
82172
}
83173

84174
/// Add an [`Allocator`] to the pool.
@@ -88,20 +178,32 @@ impl AllocatorPool {
88178
/// # Panics
89179
///
90180
/// Panics if the underlying mutex is poisoned.
181+
#[cfg(all(feature = "fixed_size", target_pointer_width = "64", target_endian = "little"))]
182+
fn add(&self, allocator: Allocator, is_fixed_size: bool) {
183+
// SAFETY: This method is only called from `AllocatorGuard::drop`.
184+
// `AllocatorGuard`s are only created by `AllocatorPool::get` or `AllocatorPool::get_maybe_fixed_size`,
185+
// so the `Allocator` must have been created by this pool. Therefore, it is the correct type for the pool.
186+
unsafe {
187+
if is_fixed_size {
188+
self.fixed_size.add(allocator);
189+
} else {
190+
self.standard.add(allocator);
191+
}
192+
}
193+
}
194+
195+
/// Add an [`Allocator`] to the pool (for platforms without fixed-size support).
196+
#[cfg(not(all(
197+
feature = "fixed_size",
198+
target_pointer_width = "64",
199+
target_endian = "little"
200+
)))]
91201
fn add(&self, allocator: Allocator) {
92202
// SAFETY: This method is only called from `AllocatorGuard::drop`.
93203
// `AllocatorGuard`s are only created by `AllocatorPool::get`, so the `Allocator` must have
94204
// been created by this pool. Therefore, it is the correct type for the pool.
95205
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-
}
206+
self.standard.add(allocator);
105207
}
106208
}
107209
}
@@ -111,9 +213,19 @@ impl AllocatorPool {
111213
/// On drop, the `Allocator` is reset and returned to the pool.
112214
pub struct AllocatorGuard<'alloc_pool> {
113215
allocator: ManuallyDrop<Allocator>,
216+
#[cfg(all(feature = "fixed_size", target_pointer_width = "64", target_endian = "little"))]
217+
is_fixed_size: bool,
114218
pool: &'alloc_pool AllocatorPool,
115219
}
116220

221+
impl AllocatorGuard<'_> {
222+
/// Returns `true` if this allocator is a fixed-size allocator (suitable for raw transfer).
223+
#[cfg(all(feature = "fixed_size", target_pointer_width = "64", target_endian = "little"))]
224+
pub fn is_fixed_size(&self) -> bool {
225+
self.is_fixed_size
226+
}
227+
}
228+
117229
impl Deref for AllocatorGuard<'_> {
118230
type Target = Allocator;
119231

@@ -127,6 +239,15 @@ impl Drop for AllocatorGuard<'_> {
127239
fn drop(&mut self) {
128240
// SAFETY: After taking ownership of the `Allocator`, we do not touch the `ManuallyDrop` again
129241
let allocator = unsafe { ManuallyDrop::take(&mut self.allocator) };
242+
243+
#[cfg(all(feature = "fixed_size", target_pointer_width = "64", target_endian = "little"))]
244+
self.pool.add(allocator, self.is_fixed_size);
245+
246+
#[cfg(not(all(
247+
feature = "fixed_size",
248+
target_pointer_width = "64",
249+
target_endian = "little"
250+
)))]
130251
self.pool.add(allocator);
131252
}
132253
}

crates/oxc_linter/src/service/runtime.rs

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -209,10 +209,11 @@ impl Runtime {
209209

210210
let thread_count = rayon::current_num_threads();
211211

212-
// If an external linter is used (JS plugins), we must use fixed-size allocators,
213-
// for compatibility with raw transfer
212+
// If an external linter is used (JS plugins), we use dual allocators:
213+
// - Fixed-size allocators for files that need linting (compatible with raw transfer to JS)
214+
// - Standard allocators for dependency files (only parsed for module resolution)
214215
let allocator_pool = if linter.has_external_linter() {
215-
AllocatorPool::new_fixed_size(thread_count)
216+
AllocatorPool::new_dual(thread_count)
216217
} else {
217218
AllocatorPool::new(thread_count)
218219
};
@@ -598,6 +599,13 @@ impl Runtime {
598599
return;
599600
}
600601

602+
if me.linter.has_external_linter() {
603+
debug_assert!(
604+
allocator_guard.is_fixed_size(),
605+
"External linters require fixed-size allocators for raw transfer"
606+
);
607+
}
608+
601609
let (mut messages, disable_directives) = me
602610
.linter
603611
.run_with_disable_directives(path, context_sub_hosts, allocator_guard);
@@ -712,6 +720,15 @@ impl Runtime {
712720
}
713721

714722
let path = Path::new(&module_to_lint.path);
723+
724+
// Validate that fixed-size allocator is used when external linter is present
725+
if me.linter.has_external_linter() {
726+
debug_assert!(
727+
allocator_guard.is_fixed_size(),
728+
"External linters require fixed-size allocators for raw transfer"
729+
);
730+
}
731+
715732
let (section_messages, disable_directives) = me
716733
.linter
717734
.run_with_disable_directives(path, context_sub_hosts, allocator_guard);
@@ -785,6 +802,13 @@ impl Runtime {
785802
return;
786803
}
787804

805+
if me.linter.has_external_linter() {
806+
debug_assert!(
807+
allocator_guard.is_fixed_size(),
808+
"External linters require fixed-size allocators for raw transfer"
809+
);
810+
}
811+
788812
messages.lock().unwrap().extend(
789813
me.linter.run(
790814
Path::new(&module.path),
@@ -831,9 +855,18 @@ impl Runtime {
831855
return None;
832856
}
833857

834-
let allocator_guard = self.allocator_pool.get();
858+
// Choose allocator type based on whether this file needs linting with JS plugins:
859+
// - Files in `paths` set are entry files that need linting
860+
// - Files not in `paths` set are dependency files (only parsed for module resolution)
861+
let file_needs_linting = paths.contains(path);
862+
863+
let allocator_guard = {
864+
let file_needs_linting_with_js_plugins =
865+
file_needs_linting && self.linter.has_external_linter();
866+
self.allocator_pool.get_maybe_fixed_size(file_needs_linting_with_js_plugins)
867+
};
835868

836-
if paths.contains(path) {
869+
if file_needs_linting {
837870
let mut records =
838871
SmallVec::<[Result<ResolvedModuleRecord, Vec<OxcDiagnostic>>; 1]>::new();
839872

0 commit comments

Comments
 (0)