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.

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

bpo-39151: Simplify dfs in the assembler #17733

Merged
merged 5 commits into from
Jun 28, 2020
Merged

bpo-39151: Simplify dfs in the assembler #17733

merged 5 commits into from
Jun 28, 2020

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Dec 29, 2019 •
edited
Loading

This simplification has the advantage that we don't stack overflow if the amount of jumps in the blocks is insane (maybe some autogenerated code).

The current code already relies on the topological order of the blocks, this PR just continues with that assumption to further simplify.

https://bugs.python.org/issue39151

@pablogsal pablogsal changed the title Simplify dfs in the assembler bpo-39151: Simplify dfs in the assembler Dec 29, 2019
@pablogsal pablogsal marked this pull request as ready for review December 29, 2019 06:01
@pablogsal pablogsal self-assigned this Dec 29, 2019
@pablogsal pablogsal requested a review from Yhg1s January 3, 2020 12:32
@pablogsal
Copy link
Member Author

As a bonus, this does hilariously jumpy code compilation faster:

master

./python.exe -m pyperf timeit 'code = ("\n".join("  "*i + "if x:\n" + "  "*i + "  ...\n" + "  "*i + "else:" for i in range(30)) + "\n" + "  "*30 + "...")' 'compile(code, "<dummy>", "exec")'
.....................
Mean +- std dev: 785 us +- 16 us

patched

./python.exe -m pyperf timeit 'code = ("\n".join("  "*i + "if x:\n" + "  "*i + "  ...\n" + "  "*i + "else:" for i in range(30)) + "\n" + "  "*30 + "...")' 'compile(code, "<dummy>", "exec")'
.....................
Mean +- std dev: 734 us +- 9 us

brettcannon
brettcannon approved these changes Jan 6, 2020
View reviewed changes
pablogsal and others added 3 commits January 6, 2020 20:09
@pablogsal @brettcannon
Update Python/compile.c
a86401d 
Co-Authored-By: Brett Cannon <54418+brettcannon@users.noreply.github.com>
@pablogsal @brettcannon
Update Python/compile.c
2f20527 
Co-Authored-By: Brett Cannon <54418+brettcannon@users.noreply.github.com>
@pablogsal @brettcannon
Update Python/compile.c
24da62a 
Co-Authored-By: Brett Cannon <54418+brettcannon@users.noreply.github.com>
@csabella
Copy link
Contributor

@pablogsal, should this be merged since it was approved?

@pablogsal
Copy link
Member Author

Thanks, @csabella for the ping! :)

pablogsal
pablogsal commented Jun 6, 2020
View reviewed changes
@pablogsal pablogsal merged commit 60eb9f1 into python:master Jun 28, 2020
@pablogsal pablogsal deleted the simplify_dfs branch June 28, 2020 00:55
fasih pushed a commit to fasih/cpython that referenced this pull request Jun 29, 2020
@pablogsal @fasihahmad
bpo-39151: Simplify DFS in the assembler (pythonGH-17733)
a2e45a1
arun-mani-j pushed a commit to arun-mani-j/cpython that referenced this pull request Jul 21, 2020
@pablogsal @arun-mani-j
bpo-39151: Simplify DFS in the assembler (pythonGH-17733)
4b6916a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brettcannon brettcannon brettcannon approved these changes

@Yhg1s Yhg1s Awaiting requested review from Yhg1s

Assignees

@pablogsal pablogsal

Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pablogsal @csabella @brettcannon @the-knights-who-say-ni @bedevere-bot