Skip to content

Commit

Permalink
perf(semantic): reduce overhead of cfg recording ast nodes (#4262)
Browse files Browse the repository at this point in the history
Control flow graph builder records AST node IDs in a temp structure `ast_nodes_records`. Only the first AST node ID inserted into the `Vec` is ever used, so turn it into just a single value.

Using `AstNodeId::dummy()` as sentinel for "not set" instead of using `Option<AstNodeId>` to reduce size of the records stack. `Option<AstNodeId>` is 16 bytes.
  • Loading branch information
overlookmotel committed Jul 15, 2024
1 parent a8dc4f3 commit da69076
Showing 1 changed file with 18 additions and 15 deletions.
33 changes: 18 additions & 15 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub struct SemanticBuilder<'a> {

pub class_table_builder: ClassTableBuilder,

ast_nodes_records: Vec<Vec<AstNodeId>>,
ast_node_records: Vec<AstNodeId>,
}

pub struct SemanticBuilderReturn<'a> {
Expand Down Expand Up @@ -137,7 +137,7 @@ impl<'a> SemanticBuilder<'a> {
check_syntax_error: false,
cfg: None,
class_table_builder: ClassTableBuilder::new(),
ast_nodes_records: Vec::new(),
ast_node_records: Vec::new(),
}
}

Expand Down Expand Up @@ -250,16 +250,19 @@ impl<'a> SemanticBuilder<'a> {
}

fn record_ast_nodes(&mut self) {
self.ast_nodes_records.push(Vec::new());
self.ast_node_records.push(AstNodeId::dummy());
}

fn retrieve_recorded_ast_nodes(&mut self) -> Vec<AstNodeId> {
self.ast_nodes_records.pop().expect("there is no ast nodes record to stop.")
#[allow(clippy::unnecessary_wraps)]
fn retrieve_recorded_ast_node(&mut self) -> Option<AstNodeId> {
Some(self.ast_node_records.pop().expect("there is no ast node record to stop."))
}

fn record_ast_node(&mut self) {
if let Some(records) = self.ast_nodes_records.last_mut() {
records.push(self.current_node_id);
if let Some(record) = self.ast_node_records.last_mut() {
if *record == AstNodeId::dummy() {
*record = self.current_node_id;
}
}
}

Expand Down Expand Up @@ -599,7 +602,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {

self.record_ast_nodes();
self.visit_expression(&stmt.test);
let test_node = self.retrieve_recorded_ast_nodes().into_iter().next();
let test_node = self.retrieve_recorded_ast_node();

/* cfg */
control_flow!(|self, cfg| {
Expand Down Expand Up @@ -718,7 +721,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {

self.record_ast_nodes();
self.visit_expression(&expr.test);
let test_node = self.retrieve_recorded_ast_nodes().into_iter().next();
let test_node = self.retrieve_recorded_ast_node();

/* cfg */
let (after_condition_graph_ix, before_consequent_expr_graph_ix) =
Expand Down Expand Up @@ -793,7 +796,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
if let Some(test) = &stmt.test {
self.record_ast_nodes();
self.visit_expression(test);
let test_node = self.retrieve_recorded_ast_nodes().into_iter().next();
let test_node = self.retrieve_recorded_ast_node();

/* cfg */
control_flow!(|self, cfg| cfg.append_condition_to(test_graph_ix, test_node));
Expand Down Expand Up @@ -859,7 +862,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {

self.record_ast_nodes();
self.visit_expression(&stmt.right);
let right_node = self.retrieve_recorded_ast_nodes().into_iter().next();
let right_node = self.retrieve_recorded_ast_node();

/* cfg */
let (end_of_prepare_cond_graph_ix, iteration_graph_ix, body_graph_ix) =
Expand Down Expand Up @@ -923,7 +926,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {

self.record_ast_nodes();
self.visit_expression(&stmt.right);
let right_node = self.retrieve_recorded_ast_nodes().into_iter().next();
let right_node = self.retrieve_recorded_ast_node();

/* cfg */
let (end_of_prepare_cond_graph_ix, iteration_graph_ix, body_graph_ix) =
Expand Down Expand Up @@ -980,7 +983,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {

self.record_ast_nodes();
self.visit_expression(&stmt.test);
let test_node = self.retrieve_recorded_ast_nodes().into_iter().next();
let test_node = self.retrieve_recorded_ast_node();

/* cfg */
let (after_test_graph_ix, before_consequent_stmt_graph_ix) = control_flow!(|self, cfg| {
Expand Down Expand Up @@ -1172,7 +1175,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
if let Some(expr) = &case.test {
self.record_ast_nodes();
self.visit_expression(expr);
let test_node = self.retrieve_recorded_ast_nodes().into_iter().next();
let test_node = self.retrieve_recorded_ast_node();
control_flow!(|self, cfg| cfg.append_condition_to(cfg.current_node_ix, test_node));
}

Expand Down Expand Up @@ -1350,7 +1353,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {

self.record_ast_nodes();
self.visit_expression(&stmt.test);
let test_node = self.retrieve_recorded_ast_nodes().into_iter().next();
let test_node = self.retrieve_recorded_ast_node();

/* cfg - body basic block */
let body_graph_ix = control_flow!(|self, cfg| {
Expand Down

0 comments on commit da69076

Please sign in to comment.