Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Tarjan SCC algorithm implementation #332

Merged
merged 2 commits into from
Feb 7, 2025

Conversation

yamadapc
Copy link
Contributor

@yamadapc yamadapc commented Feb 6, 2025

Adds an implementation of Tarjan's algorithm to find strongly connected
components.

This will be used on the experimental bundler to simplify cycle handling and
can be used on other parts of the codebase to simplify the symbol propagation
implementation.

On the bundler, we will use this to convert the graph to an acyclic graph
(replace the strongly connected components with a single 'group' node) before
doing other work.

Test Plan: yarn test

Created using spr 1.3.5
@yamadapc yamadapc requested a review from a team February 6, 2025 02:43
yamadapc added a commit that referenced this pull request Feb 6, 2025
Adds an implementation of Tarjan's algorithm to find strongly connected
components.

This will be used on the experimental bundler to simplify cycle handling and
can be used on other parts of the codebase to simplify the symbol propagation
implementation.

On the bundler, we will use this to convert the graph to an acyclic graph
(replace the strongly connected components with a single 'group' node) before
doing other work.

Test Plan: yarn test

Pull Request: #332
Created using spr 1.3.5
yamadapc added a commit that referenced this pull request Feb 6, 2025
Adds an implementation of Tarjan's algorithm to find strongly connected
components.

This will be used on the experimental bundler to simplify cycle handling and
can be used on other parts of the codebase to simplify the symbol propagation
implementation.

On the bundler, we will use this to convert the graph to an acyclic graph
(replace the strongly connected components with a single 'group' node) before
doing other work.

Test Plan: yarn test

Pull Request: #332
* Time complexity: O(V + E)
* Space complexity (worst case): O(V)
*
* * https://web.archive.org/web/20170829214726id_/http://www.cs.ucsb.edu/~gilbert/cs240a/old/cs240aSpr2011/slides/TarjanDFS.pdf
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a great precedent to set 💯. I'm often getting confused what the underlying algorithms in this repo are

const result = [];
let index = 0;
const stack = [];
const state: State[] = new Array(graph.nodes.length).fill(null).map(() => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

fill isn't necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's necessary because the array returned from new Array(length) will be sparse https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Indexed_collections#sparse_arrays

Copy link
Contributor

@MonicaOlejniczak MonicaOlejniczak Feb 7, 2025

Choose a reason for hiding this comment

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

Ah right, I was mixing it up with this syntax which is a bit more succinct

Array.from({ length: 10 }, () => ({
  index: null,
  lowlink: null,
  onStack: false,
}))

@yamadapc yamadapc merged commit 719a782 into main Feb 7, 2025
18 checks passed
@yamadapc yamadapc deleted the spr/yamadapc/add-tarjan-scc-algorithm-implementation branch February 7, 2025 01:14
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