perf(isolated-declarations): should clone transformed AST rather than original AST (#6078)

close: #6074
Performance regression introduced by https://github.com/oxc-project/oxc/pull/5909. After this PR we back to the fold pattern again
This commit is contained in:
Dunqing 2024-09-26 15:50:27 +00:00
parent cca433f84a
commit 6b7d3ed613
5 changed files with 200 additions and 154 deletions

View file

@ -1,3 +1,5 @@
use std::borrow::Cow;
use oxc_allocator::{Box, CloneIn};
#[allow(clippy::wildcard_imports)]
use oxc_ast::{ast::*, NONE};
@ -269,23 +271,26 @@ impl<'a> IsolatedDeclarations<'a> {
elements
}
/// Transform getter and setter methods
/// Collect return_type of getter and first parma type of setter
///
/// ### Getter
///
/// 1. If it has no return type, infer it from the function body
/// 2. If it cannot be inferred from the function body, try to infer it from setter method's first parameter
/// 1. If it has return type, use it
/// 2. If it has no return type, infer it from the function body
///
/// ### Setter
///
/// 1. If it has no parameter, create a parameter with the name `value`
/// 2. If it has no parameter type, infer it from the getter method's return type
fn transform_getter_or_setter_methods(&self, decl: &mut Class<'a>) {
let mut method_annotations: FxHashMap<_, (bool, _, _)> = FxHashMap::default();
for element in decl.body.body.iter_mut() {
/// 1. If it has no parameter type, infer it from the getter method's return type
fn collect_getter_or_setter_annotations(
&self,
decl: &Class<'a>,
) -> FxHashMap<Cow<str>, Box<'a, TSTypeAnnotation<'a>>> {
let mut method_annotations = FxHashMap::default();
for element in &decl.body.body {
if let ClassElement::MethodDefinition(method) = element {
if method.key.is_private_identifier()
&& (method.computed && !self.is_literal_key(&method.key))
if (method.key.is_private_identifier()
|| method.accessibility.is_some_and(TSAccessibility::is_private))
|| (method.computed && !self.is_literal_key(&method.key))
{
continue;
}
@ -296,52 +301,36 @@ impl<'a> IsolatedDeclarations<'a> {
match method.kind {
MethodDefinitionKind::Set => {
let params = &mut method.value.params;
if params.items.is_empty() {
*params = self.create_formal_parameters(
self.ast.binding_pattern_kind_binding_identifier(SPAN, "value"),
);
}
let Some(first_param) = method.value.params.items.first_mut() else {
let Some(first_param) = method.value.params.items.first() else {
continue;
};
let entry = method_annotations.entry(name).or_default();
entry.0 |= first_param.pattern.type_annotation.is_none();
entry.1 = Some(&mut first_param.pattern.type_annotation);
if let Some(annotation) =
first_param.pattern.type_annotation.clone_in(self.ast.allocator)
{
method_annotations.insert(name, annotation);
}
}
MethodDefinitionKind::Get => {
if method.accessibility.is_some_and(TSAccessibility::is_private) {
continue;
let function = &method.value;
if let Some(annotation) = function
.return_type
.clone_in(self.ast.allocator)
.or_else(|| self.infer_function_return_type(function))
{
method_annotations.insert(name, annotation);
}
let function = &mut method.value;
if function.return_type.is_none() {
function.return_type = self.infer_function_return_type(function);
};
let return_type = &mut function.return_type;
let entry = method_annotations.entry(name).or_default();
entry.0 |= return_type.is_none();
entry.2 = Some(&mut function.return_type);
}
_ => continue,
};
}
}
for (requires_inference, param, return_type) in method_annotations.into_values() {
if requires_inference {
if let (Some(Some(annotation)), Some(option))
| (Some(option), Some(Some(annotation))) = (param, return_type)
{
option.replace(annotation.clone_in(self.ast.allocator));
}
}
}
method_annotations
}
pub fn transform_class(
&self,
decl: &mut Class<'a>,
decl: &Class<'a>,
declare: Option<bool>,
) -> Option<Box<'a, Class<'a>>> {
if decl.declare {
@ -361,7 +350,7 @@ impl<'a> IsolatedDeclarations<'a> {
}
}
self.transform_getter_or_setter_methods(decl);
let setter_getter_annotations = self.collect_getter_or_setter_annotations(decl);
let mut has_private_key = false;
let mut elements = self.ast.vec();
let mut is_function_overloads = false;
@ -391,7 +380,32 @@ impl<'a> IsolatedDeclarations<'a> {
let function = &method.value;
let params = match method.kind {
MethodDefinitionKind::Set => function.params.clone_in(self.ast.allocator),
MethodDefinitionKind::Set => {
if method.accessibility.is_some_and(TSAccessibility::is_private) {
elements.push(self.transform_private_modifier_method(method));
continue;
}
let params = &method.value.params;
if params.items.is_empty() {
self.create_formal_parameters(
self.ast.binding_pattern_kind_binding_identifier(SPAN, "value"),
)
} else {
let mut params = params.clone_in(self.ast.allocator);
if let Some(param) = params.items.first_mut() {
if let Some(annotation) =
method.key.static_name().and_then(|name| {
setter_getter_annotations
.get(&name)
.map(|a| a.clone_in(self.ast.allocator))
})
{
param.pattern.type_annotation = Some(annotation);
}
}
params
}
}
MethodDefinitionKind::Constructor => {
let params = self.transform_formal_parameters(&function.params);
elements.splice(
@ -428,8 +442,16 @@ impl<'a> IsolatedDeclarations<'a> {
rt
}
MethodDefinitionKind::Get => {
let rt = method.value.return_type.clone_in(self.ast.allocator);
if method.value.return_type.is_none() {
let rt = method.value.return_type.clone_in(self.ast.allocator).or_else(
|| {
method
.key
.static_name()
.and_then(|name| setter_getter_annotations.get(&name))
.map(|a| a.clone_in(self.ast.allocator))
},
);
if rt.is_none() {
self.error(accessor_must_have_explicit_return_type(
method.key.span(),
));

View file

@ -195,7 +195,7 @@ impl<'a> IsolatedDeclarations<'a> {
pub fn transform_declaration(
&mut self,
decl: &mut Declaration<'a>,
decl: &Declaration<'a>,
check_binding: bool,
) -> Option<Declaration<'a>> {
match decl {

View file

@ -149,71 +149,60 @@ impl<'a> IsolatedDeclarations<'a> {
) -> oxc_allocator::Vec<'a, Statement<'a>> {
self.report_error_for_expando_function(stmts);
let filtered_stmts = stmts.iter().filter_map(|stmt| {
if stmt.is_declaration() {
Some(stmt.clone_in(self.ast.allocator))
let mut stmts =
self.ast.vec_from_iter(stmts.iter().filter(|stmt| {
stmt.is_declaration() && !self.has_internal_annotation(stmt.span())
}));
Self::remove_function_overloads_implementation(&mut stmts);
self.ast.vec_from_iter(stmts.iter().map(|stmt| {
if let Some(new_decl) = self.transform_declaration(stmt.to_declaration(), false) {
Statement::from(new_decl)
} else {
None
stmt.clone_in(self.ast.allocator)
}
});
let filtered_stmts = Self::remove_function_overloads_implementation(filtered_stmts);
let mut stmts = self.ast.vec_from_iter(filtered_stmts);
for stmt in stmts.iter_mut() {
if let Some(decl) = stmt.as_declaration_mut() {
if self.has_internal_annotation(decl.span()) {
continue;
}
if let Some(new_decl) = self.transform_declaration(decl, false) {
*decl = new_decl;
}
}
}
stmts
}))
}
#[allow(clippy::missing_panics_doc)]
pub fn transform_statements_on_demand(
&mut self,
stmts: &oxc_allocator::Vec<'a, Statement<'a>>,
) -> oxc_allocator::Vec<'a, Statement<'a>> {
self.report_error_for_expando_function(stmts);
let mut stmts = self.ast.vec_from_iter(stmts.iter().filter(|stmt| {
(stmt.is_declaration() || stmt.is_module_declaration())
&& !self.has_internal_annotation(stmt.span())
}));
Self::remove_function_overloads_implementation(&mut stmts);
// https://github.com/microsoft/TypeScript/pull/58912
let mut need_empty_export_marker = true;
let mut new_stmts = self.ast.vec();
// Use span to identify transformed nodes
let mut transformed_spans: FxHashSet<Span> = FxHashSet::default();
let filtered_stmts = stmts.iter().filter_map(|stmt| {
if stmt.is_declaration() || stmt.is_module_declaration() {
Some(stmt.clone_in(self.ast.allocator))
} else {
None
}
});
let filtered_stmts = Self::remove_function_overloads_implementation(filtered_stmts);
let mut transformed_stmts: FxHashMap<Span, Statement<'a>> = FxHashMap::default();
let mut transformed_variable_declarator: FxHashMap<Span, VariableDeclarator<'a>> =
FxHashMap::default();
let mut export_default_var = None;
// 1. Collect all declarations, module declarations
// 2. Transform export declarations
// 3. Collect all bindings / reference from module declarations
// 4. Collect transformed indexes
for mut stmt in filtered_stmts {
for stmt in &stmts {
match stmt {
match_declaration!(Statement) => {
if let Statement::TSModuleDeclaration(ref mut decl) = stmt {
if self.has_internal_annotation(decl.span) {
continue;
}
if let Statement::TSModuleDeclaration(decl) = stmt {
// `declare global { ... }` or `declare module "foo" { ... }`
// We need to emit it anyway
let is_global = decl.kind.is_global();
if is_global || decl.id.is_string_literal() {
transformed_spans.insert(decl.span);
let mut decl = decl.clone_in(self.ast.allocator);
// Remove export keyword from all statements in `declare module "xxx" { ... }`
if !is_global {
if let Some(body) =
@ -224,15 +213,17 @@ impl<'a> IsolatedDeclarations<'a> {
}
// We need to visit the module declaration to collect all references
self.scope.visit_ts_module_declaration(decl);
self.scope.visit_ts_module_declaration(decl.as_ref());
transformed_stmts.insert(
decl.span,
Statement::from(Declaration::TSModuleDeclaration(decl)),
);
}
}
if !self.has_internal_annotation(stmt.span()) {
new_stmts.push(stmt);
}
}
match_module_declaration!(Statement) => {
match stmt.to_module_declaration_mut() {
match stmt.to_module_declaration() {
ModuleDeclaration::ExportDefaultDeclaration(decl) => {
if self.has_internal_annotation(decl.span) {
continue;
@ -242,15 +233,19 @@ impl<'a> IsolatedDeclarations<'a> {
self.transform_export_default_declaration(decl)
{
if let Some(var_decl) = var_decl {
transformed_spans.insert(var_decl.span);
self.scope.visit_variable_declaration(&var_decl);
new_stmts.push(Statement::VariableDeclaration(
export_default_var = Some(Statement::VariableDeclaration(
self.ast.alloc(var_decl),
));
}
self.scope.visit_export_default_declaration(&new_decl);
*decl = self.ast.alloc(new_decl);
transformed_stmts.insert(
decl.span,
Statement::from(ModuleDeclaration::ExportDefaultDeclaration(
self.ast.alloc(new_decl),
)),
);
}
need_empty_export_marker = false;
@ -258,41 +253,43 @@ impl<'a> IsolatedDeclarations<'a> {
}
ModuleDeclaration::ExportNamedDeclaration(decl) => {
if self.has_internal_annotation(decl.span) {
continue;
}
transformed_spans.insert(decl.span);
if let Some(new_decl) = self.transform_export_named_declaration(decl) {
*decl = self.ast.alloc(new_decl);
self.scope.visit_export_named_declaration(&new_decl);
transformed_stmts.insert(
decl.span,
Statement::from(ModuleDeclaration::ExportNamedDeclaration(
self.ast.alloc(new_decl),
)),
);
} else if decl.declaration.is_none() {
need_empty_export_marker = false;
self.scope.visit_export_named_declaration(decl);
}
self.scope.visit_export_named_declaration(decl);
}
ModuleDeclaration::ImportDeclaration(_) => {
// We must transform this in the end, because we need to know all references
}
module_declaration => {
if self.has_internal_annotation(module_declaration.span()) {
continue;
}
transformed_spans.insert(module_declaration.span());
self.scope.visit_module_declaration(module_declaration);
}
}
new_stmts.push(stmt);
}
_ => {}
}
}
let last_transformed_len = transformed_spans.len();
let last_transformed_len = transformed_spans.len() + transformed_variable_declarator.len();
// 5. Transform statements until no more transformation can be done
loop {
let cur_transformed_len = transformed_spans.len();
for stmt in new_stmts.iter_mut() {
let Some(decl) = stmt.as_declaration_mut() else { continue };
let cur_transformed_len =
transformed_spans.len() + transformed_variable_declarator.len();
for stmt in &stmts {
if transformed_spans.contains(&stmt.span()) {
continue;
}
let Some(decl) = stmt.as_declaration() else { continue };
// Skip transformed declarations
if transformed_spans.contains(&decl.span()) {
@ -301,7 +298,7 @@ impl<'a> IsolatedDeclarations<'a> {
if let Declaration::VariableDeclaration(declaration) = decl {
let mut all_declarator_has_transformed = true;
for declarator in declaration.declarations.iter_mut() {
for declarator in &declaration.declarations {
if transformed_spans.contains(&declarator.span) {
continue;
}
@ -310,25 +307,40 @@ impl<'a> IsolatedDeclarations<'a> {
self.transform_variable_declarator(declarator, true)
{
self.scope.visit_variable_declarator(&new_declarator);
transformed_spans.insert(new_declarator.span());
*declarator = new_declarator;
transformed_variable_declarator.insert(declarator.span, new_declarator);
} else {
all_declarator_has_transformed = false;
}
}
if all_declarator_has_transformed {
declaration.declare = self.is_declare();
let declarations = self.ast.vec_from_iter(
declaration.declarations.iter().map(|declarator| {
transformed_variable_declarator.remove(&declarator.span).unwrap()
}),
);
let decl = self.ast.variable_declaration(
declaration.span,
declaration.kind,
declarations,
self.is_declare(),
);
transformed_stmts.insert(
declaration.span,
Statement::from(self.ast.declaration_from_variable(decl)),
);
transformed_spans.insert(declaration.span);
}
} else if let Some(new_decl) = self.transform_declaration(decl, true) {
self.scope.visit_declaration(&new_decl);
transformed_spans.insert(new_decl.span());
*decl = new_decl;
transformed_stmts.insert(new_decl.span(), Statement::from(new_decl));
}
}
// Use transformed_spans to weather we need to continue
if cur_transformed_len == transformed_spans.len() {
if cur_transformed_len
== transformed_spans.len() + transformed_variable_declarator.len()
{
break;
}
}
@ -339,35 +351,51 @@ impl<'a> IsolatedDeclarations<'a> {
}
// 6. Transform variable/using declarations, import statements, remove unused imports
new_stmts.retain_mut(|stmt| {
let mut new_stm =
self.ast.vec_with_capacity(stmts.len() + usize::from(export_default_var.is_some()));
stmts.iter().for_each(|stmt| {
if transformed_spans.contains(&stmt.span()) {
return true;
let new_stmt = transformed_stmts
.remove(&stmt.span())
.unwrap_or_else(|| stmt.clone_in(self.ast.allocator));
if matches!(new_stmt, Statement::ExportDefaultDeclaration(_)) {
if let Some(export_default_var) = export_default_var.take() {
new_stm.push(export_default_var);
}
}
new_stm.push(new_stmt);
return;
}
match stmt {
Statement::ImportDeclaration(decl) => {
// We must transform this in the end, because we need to know all references
if decl.specifiers.is_none() {
true
new_stm.push(stmt.clone_in(self.ast.allocator));
} else if let Some(new_decl) = self.transform_import_declaration(decl) {
*decl = new_decl;
true
} else {
false
new_stm.push(Statement::from(
self.ast.module_declaration_from_import_declaration(new_decl),
));
}
}
Statement::VariableDeclaration(ref mut decl) => {
// If here just one declaration, that means this variable declaration doesn't referenced by any other
if decl.declarations.len() == 1 {
false
} else {
Statement::VariableDeclaration(decl) => {
if decl.declarations.len() > 1 {
// Remove unreferenced declarations
decl.declarations
.retain(|declarator| transformed_spans.contains(&declarator.span));
decl.declare = self.is_declare();
true
let declarations = self.ast.vec_from_iter(
decl.declarations.iter().filter_map(|declarator| {
transformed_variable_declarator.remove(&declarator.span)
}),
);
new_stm.push(Statement::from(self.ast.declaration_from_variable(
self.ast.variable_declaration(
decl.span,
decl.kind,
declarations,
self.is_declare(),
),
)));
}
}
_ => false,
_ => {}
}
});
@ -376,27 +404,23 @@ impl<'a> IsolatedDeclarations<'a> {
let kind = ImportOrExportKind::Value;
let empty_export =
self.ast.alloc_export_named_declaration(SPAN, None, specifiers, None, kind, NONE);
new_stmts
.push(Statement::from(ModuleDeclaration::ExportNamedDeclaration(empty_export)));
new_stm.push(Statement::from(ModuleDeclaration::ExportNamedDeclaration(empty_export)));
} else if self.scope.is_ts_module_block() {
// If we are in a module block and we don't need to add `export {}`, in that case we need to remove `export` keyword from all ExportNamedDeclaration
// <https://github.com/microsoft/TypeScript/blob/a709f9899c2a544b6de65a0f2623ecbbe1394eab/src/compiler/transformers/declarations.ts#L1556-L1563>
self.strip_export_keyword(&mut new_stmts);
self.strip_export_keyword(&mut new_stm);
}
new_stmts
new_stm
}
pub fn remove_function_overloads_implementation<T>(
stmts: T,
) -> impl Iterator<Item = Statement<'a>>
where
T: Iterator<Item = Statement<'a>>,
{
pub fn remove_function_overloads_implementation(
stmts: &mut oxc_allocator::Vec<'a, &Statement<'a>>,
) {
let mut last_function_name: Option<Atom<'a>> = None;
let mut is_export_default_function_overloads = false;
stmts.filter_map(move |stmt| match stmt {
stmts.retain(move |stmt| match stmt {
Statement::FunctionDeclaration(ref func) => {
let name = &func
.id
@ -410,12 +434,12 @@ impl<'a> IsolatedDeclarations<'a> {
if func.body.is_some() {
if last_function_name.as_ref().is_some_and(|last_name| last_name == name) {
return None;
return false;
}
} else {
last_function_name = Some(name.clone());
}
Some(stmt)
true
}
Statement::ExportNamedDeclaration(ref decl) => {
if let Some(Declaration::FunctionDeclaration(ref func)) = decl.declaration {
@ -430,14 +454,14 @@ impl<'a> IsolatedDeclarations<'a> {
.name;
if func.body.is_some() {
if last_function_name.as_ref().is_some_and(|last_name| last_name == name) {
return None;
return false;
}
} else {
last_function_name = Some(name.clone());
}
Some(stmt)
true
} else {
Some(stmt)
true
}
}
Statement::ExportDefaultDeclaration(ref decl) => {
@ -446,17 +470,17 @@ impl<'a> IsolatedDeclarations<'a> {
{
if is_export_default_function_overloads && func.body.is_some() {
is_export_default_function_overloads = false;
return None;
return false;
}
is_export_default_function_overloads = true;
Some(stmt)
true
} else {
is_export_default_function_overloads = false;
Some(stmt)
true
}
}
_ => Some(stmt),
})
_ => true,
});
}
fn get_assignable_properties_for_namespaces(

View file

@ -9,9 +9,9 @@ use crate::{diagnostics::default_export_inferred, IsolatedDeclarations};
impl<'a> IsolatedDeclarations<'a> {
pub fn transform_export_named_declaration(
&mut self,
prev_decl: &mut ExportNamedDeclaration<'a>,
prev_decl: &ExportNamedDeclaration<'a>,
) -> Option<ExportNamedDeclaration<'a>> {
let decl = self.transform_declaration(prev_decl.declaration.as_mut()?, false)?;
let decl = self.transform_declaration(prev_decl.declaration.as_ref()?, false)?;
Some(self.ast.export_named_declaration(
prev_decl.span,
@ -35,9 +35,9 @@ impl<'a> IsolatedDeclarations<'a> {
pub fn transform_export_default_declaration(
&mut self,
decl: &mut ExportDefaultDeclaration<'a>,
decl: &ExportDefaultDeclaration<'a>,
) -> Option<(Option<VariableDeclaration<'a>>, ExportDefaultDeclaration<'a>)> {
let declaration = match &mut decl.declaration {
let declaration = match &decl.declaration {
ExportDefaultDeclarationKind::FunctionDeclaration(decl) => self
.transform_function(decl, Some(false))
.map(|d| (None, ExportDefaultDeclarationKind::FunctionDeclaration(d))),

View file

@ -7,7 +7,7 @@ input_file: crates/oxc_isolated_declarations/tests/fixtures/set-get-accessor.ts
declare class Cls {
get a(): number;
set a(value: number);
set a(value);
get b(): string;
set b(v: string);
private get c();