Skip to content

Conversation

@justinbay
Copy link
Contributor

update compute_scc_recursive to refresh lowlink in-place via get_mut, remove the per-iteration clone of the wrapper node and node id

@reviewable-StarkWare
Copy link

This change is Reviewable

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 1 files reviewed, 1 unresolved discussion


crates/cairo-lang-utils/src/graph_algos/strongly_connected_components.rs line 102 at r1 (raw file):

        // Update current_node in ctx.known_nodes without re-cloning the wrapper node.
        let wrapper_node = ctx.known_nodes.get_mut(&current_node_id).unwrap();
        wrapper_node.lowlink = current_wrapper_node.lowlink;

Suggestion:

    let mut current_lowlink = ctx.next_index;
    let current_node_id = current_node.get_id();
    ctx.known_nodes.insert(current_node_id.clone(), SccAlgoNode {
        node: current_node.clone(),
        index: ctx.next_index,
        lowlink: current_lowlink,
        on_stack: true,
    });
    ctx.next_index += 1;
    ctx.stack.push(current_node_id.clone());

    for neighbor in current_node.get_neighbors() {
        let neighbor_id = neighbor.get_id();
        match ctx.known_nodes.get(&neighbor_id) {
            None => {
                // neighbor was not visited yet. Visit it and maybe apply its lowlink to root.
                compute_scc_recursive(ctx, &neighbor);
                // Now neighbor should be in known_nodes.
                current_lowlink = core::cmp::min(
                    current_lowlink,
                    ctx.known_nodes[&neighbor_id].lowlink,
                );
            }
            Some(neighbor_node) => {
                if ctx.known_nodes[&neighbor_id].on_stack {
                    // This is a back edge, meaning neighbor is in current_node's SCC.
                    current_wrapper_node.lowlink =
                        core::cmp::min(current_wrapper_node.lowlink, neighbor_node.index);
                } else {
                    // If neighbor is known but not on stack, it's in a concluded dropped SCC.
                    // Ignore it.
                    continue;
                }
            }
        };

        // Update current_node's lowlink in ctx.known_nodes without re-cloning the wrapper node.
        ctx.known_nodes[&current_node_id].lowlink = current_lowlink;

@orizi
Copy link
Collaborator

orizi commented Nov 30, 2025

No response. Closing.

@orizi orizi closed this Nov 30, 2025
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.

3 participants