8000 perf(allocator): remove `Arc` from `AllocatorPool` by overlookmotel · Pull Request #11760 · oxc-project/oxc · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

perf(allocator): remove Arc from AllocatorPool #11760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 24 additions & 15 deletions crates/oxc_allocator/src/pool.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{
mem::ManuallyDrop,
ops::{Deref, DerefMut},
sync::{Arc, Mutex},
sync::Mutex,
};

use crate::Allocator;
Expand All @@ -11,14 +11,14 @@ use crate::Allocator;
/// Internally uses a `Vec` protected by a `Mutex` to store available allocators.
#[derive(Default)]
pub struct AllocatorPool {
allocators: Arc<Mutex<Vec<Allocator>>>,
allocators: Mutex<Vec<Allocator>>,
}

impl AllocatorPool {
/// Creates a new `AllocatorPool` pre-filled with the given number of default `Allocator` instances.
pub fn new(size: usize) -> AllocatorPool {
let allocators = std::iter::repeat_with(Allocator::default).take(size).collect();
AllocatorPool { allocators: Arc::new(Mutex::new(allocators)) }
AllocatorPool { allocators: Mutex::new(allocators) }
}

/// Retrieves an `Allocator` from the pool, or creates a new one if the pool is empty.
Expand All @@ -34,41 +34,50 @@ impl AllocatorPool {
allocators.pop().unwrap_or_default()
};

AllocatorGuard {
allocator: ManuallyDrop::new(allocator),
pool: Arc::clone(&self.allocators),
}
AllocatorGuard { allocator: ManuallyDrop::new(allocator), pool: self }
}

/// Add an `Allocator` to the pool.
///
/// The `Allocator` should be empty, ready to be re-used.
///
/// # Panics
///
/// Panics if the underlying mutex is poisoned.
fn add(&self, allocator: Allocator) {
let mut allocators = self.allocators.lock().unwrap();
allocators.push(allocator);
}
}

/// A guard object representing exclusive access to an `Allocator` from the pool.
///
/// On drop, the `Allocator` is reset and returned to the pool.
pub struct AllocatorGuard {
pub struct AllocatorGuard<'alloc_pool> {
allocator: ManuallyDrop<Allocator>,
pool: Arc<Mutex<Vec<Allocator>>>,
pool: &'alloc_pool AllocatorPool,
}

impl Deref for AllocatorGuard {
impl Deref for AllocatorGuard<'_> {
type Target = Allocator;

fn deref(&self) -> &Self::Target {
&self.allocator
}
}

impl DerefMut for AllocatorGuard {
impl DerefMut for AllocatorGuard<'_> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.allocator
}
}

impl Drop for AllocatorGuard {
impl Drop for AllocatorGuard<'_> {
/// Return `Allocator` back to the pool.
fn drop(&mut self) {
// SAFETY: we're taking ownership and promise not to drop `allocator` again.
// SAFETY: After taking ownership of the `Allocator`, we do not touch the `ManuallyDrop` again
let mut allocator = unsafe { ManuallyDrop::take(&mut self.allocator) };
allocator.reset();
let mut allocators = self.pool.lock().unwrap();
allocators.push(allocator);
self.pool.add(allocator);
}
}
29 changes: 16 additions & 13 deletions crates/oxc_linter/src/service/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,16 @@ pub struct Runtime<'l> {
}

/// Output of `Runtime::process_path`
struct ModuleProcessOutput {
struct ModuleProcessOutput<'alloc_pool> {
/// All paths in `Runtime` are stored as `OsStr`, because `OsStr` hash is faster
/// than `Path` - go checkout their source code.
path: Arc<OsStr>,
processed_module: ProcessedModule,
processed_module: ProcessedModule<'alloc_pool>,
}

/// A module processed from a path
#[derive(Default)]
struct ProcessedModule {
struct ProcessedModule<'alloc_pool> {
/// Module records of source sections, or diagnostics if parsing failed on that section.
///
/// Modules with special extensions such as .vue could contain multiple source sections (see `PartialLoader::PartialLoader`).
Expand All @@ -71,7 +71,7 @@ struct ProcessedModule {
///
/// Note that `content` is `Some` even if parsing is unsuccessful as long as the source to lint is valid utf-8.
/// It is designed this way to cover the case where some but not all the sections fail to parse.
content: Option<ModuleContent>,
content: Option<ModuleContent<'alloc_pool>>,
}

struct ResolvedModuleRequest {
Expand All @@ -86,18 +86,18 @@ struct ResolvedModuleRecord {
}

self_cell! {
struct ModuleContent {
owner: ModuleContentOwner,
struct ModuleContent<'alloc_pool> {
owner: ModuleContentOwner<'alloc_pool>,
#[not_covariant]
dependent: SectionContents,
}
}
// Safety: dependent borrows from owner. They're safe to be sent together.
unsafe impl Send for ModuleContent {}
unsafe impl Send for ModuleContent<'_> {}

struct ModuleContentOwner {
struct ModuleContentOwner<'alloc_pool> {
source_text: String,
allocator: AllocatorGuard,
allocator: AllocatorGuard<'alloc_pool>,
}

/// source text and semantic for each source section. They are in the same order as `ProcessedModule.section_module_records`
Expand All @@ -113,13 +113,16 @@ struct SectionContent<'a> {
///
/// A `ModuleWithContent` is generated for each path in `runtime.paths`. It's basically the same
/// as `ProcessedModule`, except `content` is non-Option.
struct ModuleToLint {
struct ModuleToLint<'alloc_pool> {
path: Arc<OsStr>,
section_module_records: SmallVec<[Result<Arc<ModuleRecord>, Vec<OxcDiagnostic>>; 1]>,
content: ModuleContent,
content: ModuleContent<'alloc_pool>,
}
impl ModuleToLint {
fn from_processed_module(path: Arc<OsStr>, processed_module: ProcessedModule) -> Option<Self> {
impl<'alloc_pool> ModuleToLint<'alloc_pool> {
fn from_processed_module(
path: Arc<OsStr>,
processed_module: ProcessedModule<'alloc_pool>,
) -> Option<Self> {
processed_module.content.map(|content| Self {
path,
section_module_records: processed_module
Expand Down
Loading
0