Skip to content

Conversation

@eytan-starkware
Copy link
Contributor

@eytan-starkware eytan-starkware commented Oct 16, 2025

TL;DR

Optimize syntax node traversal by introducing a SyntaxNodeBuilder that delays node construction until needed.

What changed?

This PR introduces a new SyntaxNodeBuilder type that provides access to a child node's metadata without immediately constructing a full SyntaxNode. The key changes include:

  • Modified ChildrenIter to return SyntaxNodeBuilder instead of SyntaxNode
  • Added a .nodes(db) method to eagerly build syntax nodes when needed
  • Updated all code that uses get_children(db) to either:
    • Use the new builder directly when only metadata is needed
    • Call .nodes(db) or .build(db) when a full syntax node is required
  • Optimized collect_green_nodes to work directly with green nodes instead of constructing syntax nodes

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

eytan-starkware commented Oct 16, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@eytan-starkware eytan-starkware marked this pull request as ready for review October 16, 2025 08:25
@eytan-starkware eytan-starkware force-pushed the eytan_graphite/_optimization_using_syntaxnodebuilder_pattern_for_get_children branch from babde02 to b6e377e Compare October 16, 2025 08:28
@eytan-starkware eytan-starkware force-pushed the eytan_graphite/_optimization_using_syntaxnodebuilder_pattern_for_get_children branch from b6e377e to 983fa39 Compare October 16, 2025 09:10
@eytan-starkware eytan-starkware force-pushed the eytan_graphite/make_get_children_an_iterator branch from afa2a8e to d60a80f Compare October 16, 2025 09:10
Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orizi reviewed all commit messages.
Reviewable status: 0 of 14 files reviewed, 2 unresolved discussions (waiting on @TomerStarkware)


crates/cairo-lang-syntax/src/node/mod.rs line 145 at r1 (raw file):

    /// Eagerly builds syntax nodes for the remaining children.
    pub fn nodes(

Suggestion:

pub fn as_syntax_nodes(

crates/cairo-lang-syntax/src/node/mod.rs line 193 at r1 (raw file):

    /// Constructs a [SyntaxNode] for this child.
    pub fn build(&self, db: &'a dyn Database) -> SyntaxNode<'a> {

Suggestion:

    pub fn as_syntax_node(&self, db: &'a dyn Database) -> SyntaxNode<'a> {

Copy link
Collaborator

@TomerStarkware TomerStarkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@TomerStarkware reviewed 14 of 14 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @eytan-starkware)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants