Skip to content

bpo-40902: Speed up PEG parser by using Dijkstra's shunting yard algorithm#20696

Closed
pablogsal wants to merge 19 commits into
python:masterfrom
we-like-parsers:operator_precedence
Closed

bpo-40902: Speed up PEG parser by using Dijkstra's shunting yard algorithm#20696
pablogsal wants to merge 19 commits into
python:masterfrom
we-like-parsers:operator_precedence

Conversation

@pablogsal

@pablogsal pablogsal commented Jun 7, 2020

Copy link
Copy Markdown
Member

@pablogsal pablogsal changed the title bpo-40902: Speed up PEG parser by using operator precedence for binary operators bpo-40902: Speed up PEG parser by using Dijkstra's shunting yard algorithm Jun 8, 2020
@pablogsal

Copy link
Copy Markdown
Member Author

It seems that we need unary operators to participate in order to also add ** to the set of operators...

@gvanrossum

Copy link
Copy Markdown
Member

@pablogsal How do you feel about this? Should we finish it up (fix the merge conflict) and land it, or abandon?

@pablogsal

pablogsal commented Jun 10, 2020

Copy link
Copy Markdown
Member Author

@pablogsal How do you feel about this?

I am torn: I really like the speedup but I dislike that some operators have their precedence defined via the new method and some others (unary and **) use the "regular" method. It makes a bit complex to document the grammar as it uses two different methods for the same purpose.

Regarding the code complexity, I think is fine as is very localized and although the algorithm can be a bit more complex because the common stack, I think is readable and small enough to not be a major problem IMHO.

I think that unless we found a straightforward way to include unary operators (so we can also include the right-associative **), I propose to abandon this to not increase the complexity of documenting the grammar (and the grammar itself).

What do you think?

@gvanrossum

Copy link
Copy Markdown
Member

Agreed. We could shelve it now and consider it if we get demands for a faster parser.

@pablogsal pablogsal closed this Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants