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

Left-recursion improvements. #226

Merged
merged 5 commits into from
Apr 17, 2017

Conversation

veelo
Copy link
Collaborator

@veelo veelo commented Apr 15, 2017

This brings the parsing of a sample Extended Pascal file down from 13 minutes to 17 seconds. The mistake was not to assign the result of std.algorithm.mutation.remove, which caused an array to grow to a massive size and memoization to never be reenabled. (This is now on row 562.)

It also removes quite a bit of complexity and some requirements that I never really understood why were necessary. Now it appears they aren't.

This is worth a patch-level version bump, I think.

veelo added 5 commits March 13, 2017 12:17
…m.mutation.remove, which confusingly does not mutate the range, but returns a new range — which was never used.

When this was fixed, it appeared that blocking just the current cycle is not enough. Now all left-recursive rules are blocked, which may be a bit conservative, but it works. Brings parse time from almost 13 minutes down to 19 seconds.
@PhilippeSigaud PhilippeSigaud merged commit 1dc27c5 into dlang-community:master Apr 17, 2017
@PhilippeSigaud
Copy link
Collaborator

PhilippeSigaud commented Apr 17, 2017 via email

@veelo
Copy link
Collaborator Author

veelo commented Apr 17, 2017

Thanks.

I suppose the speed improvements will also be visible for other grammars than EP?

If the grammar is left-recursive then yes.

Although much better, 17 seconds is still really slow. This is a file that is compiled by the Prospero compiler in just 99 milliseconds... I am thinking that top-down parsing is not efficient for this language, but I'll see if I can do a bit more profiling.

@veelo veelo deleted the LeftRecursion branch September 7, 2023 14:47
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.

2 participants