Skip to content

Commit

Permalink
perf(transformer): React JSX reduce allocations (#3522)
Browse files Browse the repository at this point in the history
React JSX transform was creating `CompactStr`s (which can involve heap allocations) and then converting them back to `Atom`s to insert into AST.

This PR removes the intermediate `CompactStr`s and allocates strings directly into the arena without intermediate heap allocations.
  • Loading branch information
overlookmotel committed Jun 4, 2024
1 parent f3a755c commit 91519d9
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 45 deletions.
59 changes: 29 additions & 30 deletions crates/oxc_transformer/src/helpers/module_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ use std::cell::RefCell;

use oxc_allocator::{Allocator, Vec};
use oxc_ast::{ast::*, AstBuilder};
use oxc_span::{CompactStr, SPAN};
use oxc_span::{Atom, SPAN};

pub struct NamedImport {
imported: CompactStr,
local: Option<CompactStr>, // Not used in `require`
pub struct NamedImport<'a> {
imported: Atom<'a>,
local: Option<Atom<'a>>, // Not used in `require`
}

impl NamedImport {
pub fn new(imported: CompactStr, local: Option<CompactStr>) -> NamedImport {
impl<'a> NamedImport<'a> {
pub fn new(imported: Atom<'a>, local: Option<Atom<'a>>) -> Self {
Self { imported, local }
}
}
Expand All @@ -23,13 +23,13 @@ pub enum ImportKind {
}

#[derive(Hash, Eq, PartialEq)]
pub struct ImportType {
pub struct ImportType<'a> {
kind: ImportKind,
source: CompactStr,
source: Atom<'a>,
}

impl ImportType {
fn new(kind: ImportKind, source: CompactStr) -> Self {
impl<'a> ImportType<'a> {
fn new(kind: ImportKind, source: Atom<'a>) -> Self {
Self { kind, source }
}
}
Expand All @@ -39,7 +39,7 @@ impl ImportType {
pub struct ModuleImports<'a> {
ast: AstBuilder<'a>,

imports: RefCell<IndexMap<ImportType, std::vec::Vec<NamedImport>>>,
imports: RefCell<IndexMap<ImportType<'a>, std::vec::Vec<NamedImport<'a>>>>,
}

impl<'a> ModuleImports<'a> {
Expand All @@ -49,7 +49,7 @@ impl<'a> ModuleImports<'a> {
}

/// Add `import { named_import } from 'source'`
pub fn add_import(&self, source: CompactStr, import: NamedImport) {
pub fn add_import(&self, source: Atom<'a>, import: NamedImport<'a>) {
self.imports
.borrow_mut()
.entry(ImportType::new(ImportKind::Import, source))
Expand All @@ -58,7 +58,7 @@ impl<'a> ModuleImports<'a> {
}

/// Add `var named_import from 'source'`
pub fn add_require(&self, source: CompactStr, import: NamedImport, front: bool) {
pub fn add_require(&self, source: Atom<'a>, import: NamedImport<'a>, front: bool) {
let len = self.imports.borrow().len();
self.imports
.borrow_mut()
Expand All @@ -73,55 +73,54 @@ impl<'a> ModuleImports<'a> {
pub fn get_import_statements(&self) -> Vec<'a, Statement<'a>> {
self.ast.new_vec_from_iter(self.imports.borrow_mut().drain(..).map(
|(import_type, names)| match import_type.kind {
ImportKind::Import => self.get_named_import(&import_type.source, names),
ImportKind::Require => self.get_require(&import_type.source, names),
ImportKind::Import => self.get_named_import(import_type.source, names),
ImportKind::Require => self.get_require(import_type.source, names),
},
))
}

fn get_named_import(
&self,
source: &CompactStr,
names: std::vec::Vec<NamedImport>,
source: Atom<'a>,
names: std::vec::Vec<NamedImport<'a>>,
) -> Statement<'a> {
let specifiers = self.ast.new_vec_from_iter(names.into_iter().map(|name| {
let local = name.local.unwrap_or_else(|| name.imported.clone());
ImportDeclarationSpecifier::ImportSpecifier(self.ast.alloc(ImportSpecifier {
span: SPAN,
imported: ModuleExportName::Identifier(IdentifierName::new(
SPAN,
self.ast.new_atom(name.imported.as_str()),
)),
local: BindingIdentifier::new(
SPAN,
self.ast.new_atom(name.local.unwrap_or(name.imported).as_str()),
),
imported: ModuleExportName::Identifier(IdentifierName::new(SPAN, name.imported)),
local: BindingIdentifier::new(SPAN, local),
import_kind: ImportOrExportKind::Value,
}))
}));
let import_stmt = self.ast.import_declaration(
SPAN,
Some(specifiers),
self.ast.string_literal(SPAN, source.as_str()),
StringLiteral::new(SPAN, source),
None,
ImportOrExportKind::Value,
);
self.ast.module_declaration(ModuleDeclaration::ImportDeclaration(import_stmt))
}

fn get_require(&self, source: &CompactStr, names: std::vec::Vec<NamedImport>) -> Statement<'a> {
fn get_require(
&self,
source: Atom<'a>,
names: std::vec::Vec<NamedImport<'a>>,
) -> Statement<'a> {
let var_kind = VariableDeclarationKind::Var;
let callee = {
let ident = IdentifierReference::new(SPAN, "require".into());
let ident = IdentifierReference::new(SPAN, Atom::from("require"));
self.ast.identifier_reference_expression(ident)
};
let args = {
let string = self.ast.string_literal(SPAN, source.as_str());
let string = StringLiteral::new(SPAN, source);
let arg = Argument::from(self.ast.literal_string_expression(string));
self.ast.new_vec_single(arg)
};
let name = names.into_iter().next().unwrap();
let id = {
let ident = BindingIdentifier::new(SPAN, self.ast.new_atom(&name.imported));
let ident = BindingIdentifier::new(SPAN, name.imported);
self.ast.binding_pattern(self.ast.binding_pattern_identifier(ident), None, false)
};
let decl = {
Expand Down
30 changes: 15 additions & 15 deletions crates/oxc_transformer/src/react/jsx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::rc::Rc;

use oxc_allocator::Vec;
use oxc_ast::{ast::*, AstBuilder};
use oxc_span::{Atom, CompactStr, GetSpan, Span, SPAN};
use oxc_span::{Atom, GetSpan, Span, SPAN};
use oxc_syntax::{
identifier::{is_irregular_whitespace, is_line_terminator},
symbol::SymbolFlags,
Expand Down Expand Up @@ -40,7 +40,7 @@ pub struct ReactJsx<'a> {
pub(super) jsx_source: ReactJsxSource<'a>,

// States
jsx_runtime_importer: CompactStr,
jsx_runtime_importer: Atom<'a>,

// Doubles as var name for require react
import_create_element: Option<Atom<'a>>,
Expand All @@ -60,9 +60,9 @@ impl<'a> ReactJsx<'a> {
if options.import_source == "react" || default_runtime.is_classic() {
let source =
if options.development { "react/jsx-dev-runtime" } else { "react/jsx-runtime" };
CompactStr::from(source)
Atom::from(source)
} else {
CompactStr::from(format!(
ctx.ast.new_atom(&format!(
"{}/jsx-{}runtime",
options.import_source,
if options.development { "dev-" } else { "" }
Expand Down Expand Up @@ -234,46 +234,46 @@ impl<'a> ReactJsx<'a> {

fn add_import_create_element(&mut self, ctx: &mut TraverseCtx<'a>) {
if self.import_create_element.is_none() {
let source = self.options.import_source.as_ref();
let source = ctx.ast.new_atom(&self.options.import_source);
let var_name = if self.is_script() {
self.add_require_statement("react", source.into(), true, ctx)
self.add_require_statement("react", source, true, ctx)
} else {
self.add_import_statement("createElement", source.into(), ctx)
self.add_import_statement("createElement", source, ctx)
};
self.import_create_element = Some(var_name);
}
}

fn add_import_statement(
&mut self,
name: &str,
source: CompactStr,
name: &'static str,
source: Atom<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Atom<'a> {
let root_scope_id = ctx.scopes().root_scope_id();
let symbol_id = ctx.generate_uid(name, root_scope_id, SymbolFlags::FunctionScopedVariable);
let local = &ctx.symbols().names[symbol_id];
let local = ctx.ast.new_atom(&ctx.symbols().names[symbol_id]);

let import = NamedImport::new(name.into(), Some(local.clone()));
let import = NamedImport::new(Atom::from(name), Some(local.clone()));
self.ctx.module_imports.add_import(source, import);
ctx.ast.new_atom(local)
local
}

fn add_require_statement(
&mut self,
variable_name: &str,
source: CompactStr,
source: Atom<'a>,
front: bool,
ctx: &mut TraverseCtx<'a>,
) -> Atom<'a> {
let root_scope_id = ctx.scopes().root_scope_id();
let symbol_id =
ctx.generate_uid(variable_name, root_scope_id, SymbolFlags::FunctionScopedVariable);
let variable_name = &ctx.symbols().names[symbol_id];
let variable_name = ctx.ast.new_atom(&ctx.symbols().names[symbol_id]);

let import = NamedImport::new(variable_name.clone(), None);
self.ctx.module_imports.add_require(source, import, front);
ctx.ast.new_atom(variable_name)
variable_name
}
}

Expand Down

0 comments on commit 91519d9

Please sign in to comment.