Skip to content

Commit

Permalink
perf(semantic): remove branch from Nodes::add_node (#4361)
Browse files Browse the repository at this point in the history
`AstNodes::add_node` had a branch specifically to handle `Program`. Remove that by inlining the special logic for `Program` into `visit_program`.

Probably won't have much effect on benchmarks as the branch is easy to predict, but still removes a few instructions and makes `add_node` easier for compiler to inline.
  • Loading branch information
overlookmotel committed Jul 19, 2024
1 parent 729b288 commit 7469e01
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 9 deletions.
14 changes: 13 additions & 1 deletion crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,19 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
});
/* cfg - must be above directives as directives are in cfg */

self.enter_node(kind);
// Don't call `enter_node` here as `Program` is a special case - node has no `parent_id`.
// Inline the specific logic for `Program` here instead.
// This avoids `Nodes::add_node` having to handle the special case.
// We can also skip calling `self.enter_kind`, and `self.jsdoc.retrieve_attached_jsdoc`
// as they are no-ops for `Program`.
self.current_node_id = self.nodes.add_program_node(
kind,
self.current_scope_id,
control_flow!(self, |cfg| cfg.current_node_ix),
self.current_node_flags,
);
self.record_ast_node();

self.enter_scope(
{
let mut flags = ScopeFlags::Top;
Expand Down
25 changes: 17 additions & 8 deletions crates/oxc_semantic/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ impl<'a> AstNodes<'a> {
}

/// Create and add an `AstNode` to the `AstNodes` tree and returns its `AstNodeId`.
/// Node must not be `Program`. Use `add_program_node` instead.
pub fn add_node(
&mut self,
kind: AstKind<'a>,
Expand All @@ -150,14 +151,22 @@ impl<'a> AstNodes<'a> {
cfg_id: BasicBlockId,
flags: NodeFlags,
) -> AstNodeId {
let ast_node_id = match kind {
AstKind::Program(_) => {
let id = self.parent_ids.push(None);
self.root = Some(id);
id
}
_ => self.parent_ids.push(Some(parent_node_id)),
};
let ast_node_id = self.parent_ids.push(Some(parent_node_id));
let node = AstNode::new(kind, scope_id, cfg_id, flags, ast_node_id);
self.nodes.push(node);
ast_node_id
}

/// Create and add an `AstNode` to the `AstNodes` tree and returns its `AstNodeId`.
pub fn add_program_node(
&mut self,
kind: AstKind<'a>,
scope_id: ScopeId,
cfg_id: BasicBlockId,
flags: NodeFlags,
) -> AstNodeId {
let ast_node_id = self.parent_ids.push(None);
self.root = Some(ast_node_id);
let node = AstNode::new(kind, scope_id, cfg_id, flags, ast_node_id);
self.nodes.push(node);
ast_node_id
Expand Down

0 comments on commit 7469e01

Please sign in to comment.