8000 Remove redundant artificial rules and functions for them in parser.c file · Issue #124889 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

Remove redundant artificial rules and functions for them in parser.c file #124889

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.

Sign up for GitHub

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

Jump to bottom
Closed
efimov-mikhail opened this issue Oct 2, 2024 · 3 comments
Closed

Remove redundant artificial rules and functions for them in parser.c file #124889

efimov-mikhail opened this issue Oct 2, 2024 · 3 comments
Labels
topic-parser type-feature A feature request or enhancement

Comments

@efimov-mikhail
Copy link
Contributor
efimov-mikhail commented Oct 2, 2024 •
edited by bedevere-app bot
Loading

Feature or enhancement

Proposal:

It seems like PEG parser produces more than one artificial rule for the same grammar rule.
For example, on each gather rule (','.NAME+) in Grammar/python.gram a function '_gather_N_rule' will be generated.
But only one implementation of such function makes sense, others are just redundant.

Some simple changes in Tools/peg_generator/pegen/c_generator.py file should be made.
We can save artificial rules in cache by Node string representation as a key instead of Node object itself.
Total count of artificial rules can be lowered from 283 to 170 with this change, size of parser.c file can be lowered from 1.5 megabytes to 1.3 megabytes without any cost.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs
@efimov-mikhail efimov-mikhail added the type-feature A feature request or enhancement label Oct 2, 2024
efimov-mikhail added a commit to efimov-mikhail/cpython that referenced this issue Oct 2, 2024
@efimov-mikhail
pythongh-124889: Remove redundant artificial rules in PEG parser
731ea7f 
Cache in C PEG-generator reworked:
we save artificial rules in cache by Node string representation as a key instead of Node object itself.
As a result total count of artificial rules in parsers.c is lowered from 283 to 170.
More natural number ordering is used for the names of artificial rules.
@bedevere-app bedevere-app bot mentioned this issue Oct 2, 2024
Merged
efimov-mikhail added a commit to efimov-mikhail/cpython 8000 that referenced this issue Oct 3, 2024
@efimov-mikhail
pythongh-124889: Remove redundant artificial rules in PEG parser
b9f82de 
Cache in C PEG-generator reworked:
we save artificial rules in cache by Node string representation as a key instead of Node object itself.
As a result total count of artificial rules in parsers.c is lowered from 283 to 170.
More natural number ordering is used for the names of artificial rules.
efimov-mikhail added a commit to efimov-mikhail/cpython that referenced this issue Oct 3, 2024
@efimov-mikhail
pythongh-124889: Remove redundant artificial rules, some refactoring (p…
0a8f197 
…ython#124893)

Auxiliary method CCallMakerVisitor._generate_artificial_rule_call is added.
Its purpose is abstracting work with artificial rules cache.
efimov-mikhail added a commit to efimov-mikhail/cpython that referenced this issue Oct 3, 2024
@efimov-mikhail
pythongh-124889: Remove redundant artificial rules, some refactoring 2 (
e1b62e5 
python#124893)

Explicit using of "is_repeat1" kwarg is added to visit_Repeat0 and visit_Repeat1 methods.
Its slightly improve code readabitily.
pablogsal pushed a commit that referenced this issue Oct 3, 2024
@efimov-mikhail
gh-124889: Remove redundant artificial rules in PEG parser (#124893)
1f9025a 
Cache in C PEG-generator reworked:
we save artificial rules in cache by Node string representation as a key instead of Node object itself.
As a result total count of artificial rules in parsers.c is lowered from 283 to 170.
More natural number ordering is used for the names of artificial rules.

Auxiliary method CCallMakerVisitor._generate_artificial_rule_call is added.
Its purpose is abstracting work with artificial rules cache.

Explicit using of "is_repeat1" kwarg is added to visit_Repeat0 and visit_Repeat1 methods.
Its slightly improve code readabitily.
@nineteendo
Copy link
Contributor

Is this finished?

@efimov-mikhail
Copy link
Contributor Author

The original issue has been implemented.
But it seems like similar changes may be provided for Tools/peg_generator/pegen/python_generator.py.
IMHO, this issue should be closed after such investigations and creating new PR, if possible.

@nineteendo
Copy link
Contributor

I agree that this should be closed.

efimov-mikhail added a commit to efimov-mikhail/cpython that referenced this issue Oct 21, 2024
@efimov-mikhail
pythongh-124889: Rework Python generator cache
413cf8a
@bedevere-app bedevere-app bot mentioned this issue Oct 21, 2024
Merged
efimov-mikhail added a commit to efimov-mikhail/cpython that referenced this issue Oct 21, 2024
@efimov-mikhail
pythongh-124889: Typo fix
c9b3693
pablogsal pushed a commit that referenced this issue Oct 22, 2024
@efimov-mikhail
gh-124889: Rework Python generator cache (#125816)
c1bdbe8
ebonnal pushed a commit to ebonnal/cpython that referenced this issue Jan 12, 2025
@efimov-mikhail @ebonnal
pythongh-124889: Rework Python generator cache (python#125816)
be91149
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic-parser type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@efimov-mikhail @pablogsal @nineteendo @Eclips4
0