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

Constant phases #567

Merged
merged 4 commits into from
Feb 21, 2024
Merged

Constant phases #567

merged 4 commits into from
Feb 21, 2024

Conversation

ambv
Copy link
Member

@ambv ambv commented Mar 3, 2023

This is #272 but rebased on top of current master.

I made cursory benchmarking of this and I see no difference between master and this branch.

Python Tests executed Time with PR Time on master
2.7.18 28238 47.3s 47.2s
3.9.16 19062 13.4s 13.6s
3.11.2 23563 12.4s 12.4s
pypy3.8 23563 44.3s 44.4s

Each timing is geomean from 30 runs. This is executed on Homebrew builds of all Pythons above on an M1 Max Macbook Pro. The slow result from pypy3 (v7.3.9) is because it's running in Rosetta (x86-64 emulation). The important thing is to compare the time on the PR with the time on master.

gsnedders and others added 3 commits September 27, 2020 21:43
This added a fair bit of complexity, and notable made the Phase classes
dynamically generated.

However, by doing this, we no longer include "process the
token using the rules for" phases in the debug log.
@ambv
Copy link
Member Author

ambv commented Mar 3, 2023

When looking at the PR without whitespace changes, it's not that complicated. All tests pass, no performance regression.

I think this should go in as is. This will help us optimize, whether with Cython or with mypyc, as the result is a plain old Phase class, which I like 👍🏻

@@ -204,6 +191,9 @@ def mainLoop(self):
DoctypeToken = tokenTypes["Doctype"]
ParseErrorToken = tokenTypes["ParseError"]

type_names = {value: key for key, value in tokenTypes.items()}
Copy link
Member

Choose a reason for hiding this comment

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

I suppose one could skip computing this unless debug is set, but probably not worthwhile.

@ambv ambv merged commit fd4f032 into master Feb 21, 2024
26 checks passed
@ambv ambv deleted the constant_phases branch February 21, 2024 15:31
@ambv ambv mentioned this pull request Feb 21, 2024
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