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

Improve performance of hidden node parsing #1790

Merged
merged 3 commits into from
Jan 17, 2025
Merged

Conversation

msujew
Copy link
Member

@msujew msujew commented Jan 14, 2025

I've noticed in a project with a large file with a lot of comments (~20 000 LoC, ~10 000 comments), that parsing started taking a huge performance hit. It seems like our current method of iterating through the CST to add hidden nodes leads to super-linear behavior in the parsing phase.

You can reproduce this by using the test I've added to langium-parser.test.ts and run it on the current main branch. While the parse takes only ~150ms on my machine with the new changes, it takes roughly ~5000ms on main.

This change replaces the existing hidden node placement algorithm with a different one that should perform linearly. Note that the placement of the hidden nodes has not changed with this algorithm. Existing tests (and also a few new ones) verify this behavior.

Some ideas for this new algorithm were taken from #1733.

@msujew msujew added parser Parser related issue performance Issues related to the runtime performance of Langium labels Jan 14, 2025
@msujew msujew requested a review from Lotes January 15, 2025 11:20
@msujew msujew force-pushed the msujew/hidden-node-performance branch from 3266554 to 11d88a1 Compare January 15, 2025 13:05
Copy link
Contributor

@Lotes Lotes left a comment

Choose a reason for hiding this comment

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

I was hesitating with the approval, but actually there is no need for waiting, I got it. If you want an additional approval or more comments, you could reach out for Ben. Or ask me, what I should look at specifically.

@msujew msujew merged commit c006777 into main Jan 17, 2025
5 checks passed
@msujew msujew deleted the msujew/hidden-node-performance branch January 17, 2025 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parser Parser related issue performance Issues related to the runtime performance of Langium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants