
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2019-06-10 08:24 byserhiy.storchaka, last changed2022-04-11 14:59 byadmin. This issue is nowclosed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| test_peepholer.diff | serhiy.storchaka,2019-06-12 06:00 | |||
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 13969 | merged | pablogsal,2019-06-11 10:14 | |
| PR 14063 | merged | miss-islington,2019-06-13 18:17 | |
| Messages (13) | |||
|---|---|---|---|
| msg345108 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2019-06-10 08:24 | |
The optimization is skipped if lnotab contains 255. It was very uncommon in older versions (only when the function contains very large expressions, larger than hundreds of lines or bytecode instructions), but in 3.8 this situation is common.For example:[x for x in a if x] 1 0 BUILD_LIST 0 2 LOAD_FAST 0 (.0) >> 4 FOR_ITER 12 (to 18) 2 6 STORE_FAST 1 (x) 8 LOAD_FAST 1 (x) 10 POP_JUMP_IF_FALSE 16 1 12 LOAD_FAST 1 (x) 14 LIST_APPEND 2 >> 16 JUMP_ABSOLUTE 4 >> 18 RETURN_VALUEif x: if (y and z): foo()else: bar() 1 0 LOAD_NAME 0 (x) 2 POP_JUMP_IF_FALSE 20 2 4 LOAD_NAME 1 (y) 6 POP_JUMP_IF_FALSE 18 3 8 LOAD_NAME 2 (z) 2 10 POP_JUMP_IF_FALSE 18 4 12 LOAD_NAME 3 (foo) 14 CALL_FUNCTION 0 16 POP_TOP >> 18 JUMP_FORWARD 6 (to 26) 6 >> 20 LOAD_NAME 4 (bar) 22 CALL_FUNCTION 0 24 POP_TOP >> 26 LOAD_CONST 0 (None) 28 RETURN_VALUEYou can see non-optimized jumps to jumps (from 10 to 16 and from 6 and 10 to 16 correspondingly).This is a consequence of two features: ability to encode negative line differences in lnotab and setting lines for both outer and inner expressions.Two ways to solve this issue:1. Move optimizations fromPython/peephole.c toPython/compile.c (seeissue32477 andissue33318). This is a new feature and it is too late for 3.8.2. Make the peepholer to work with lnotab containing 255.Pablo, are you interesting? | |||
| msg345117 -(view) | Author: Pablo Galindo Salgado (pablogsal)*![]() | Date: 2019-06-10 10:12 | |
Thank you very much, Serhiy!I am interested, I will try to look at the problem and try to get a PR soon.What of the two possible solutions that you mention you think is better? I assume if we make the peephole optimizer work with lnotab containing 255 we could backport to 3.8 as a bugfix, right? | |||
| msg345120 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2019-06-10 10:39 | |
Yes, we should backport the fix to 3.8. There is a bug in 3.8. | |||
| msg345268 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2019-06-11 20:53 | |
I removed the memchr(255) to see which tests fail:test_extended_opargs (test.test_modulefinder.ModuleFinderTest) ... python:Python/peephole.c:469: PyCode_Optimize: Assertion `cum_orig_offset % sizeof(_Py_CODEUNIT) == 0' failed.Fatal Python error: Abortedtest_extended_arg (test.test_compile.TestSpecifics) ... python:Python/peephole.c:469: PyCode_Optimize: Assertion `cum_orig_offset % sizeof(_Py_CODEUNIT) == 0' failed.Fatal Python error: Abortedtest_field_named_like_builtin (test.test_dataclasses.TestCase) ... python:Python/peephole.c:469: PyCode_Optimize: Assertion `cum_orig_offset % sizeof(_Py_CODEUNIT) == 0' failed.Fatal Python error: Abortedtest_field_named_like_builtin_frozen (test.test_dataclasses.TestCase) ... python:Python/peephole.c:469: PyCode_Optimize: Assertion `cum_orig_offset % sizeof(_Py_CODEUNIT) == 0' failed.Fatal Python error: AbortedDoes test_compile have unit tests test_modulefinder and test_dataclasses cases? | |||
| msg345309 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2019-06-12 06:00 | |
test_peepholer.diff adds tests which cover various peepholer optimizations not tested before. All of them are failed now and should be passed after mergingPR 13969. Pablo, you can include these tests inPR 13969. | |||
| msg345529 -(view) | Author: Pablo Galindo Salgado (pablogsal)*![]() | Date: 2019-06-13 18:16 | |
New changeset3498c642f4e83f3d8e2214654c0fa8e0d51cebe5 by Pablo Galindo in branch 'master':bpo-37213: Handle negative line deltas correctly in the peephole optimizer (GH-13969)https://github.com/python/cpython/commit/3498c642f4e83f3d8e2214654c0fa8e0d51cebe5 | |||
| msg345530 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2019-06-13 18:30 | |
> The optimization is skipped if lnotab contains 255. It was very uncommon in older versions (only when the function contains very large expressions, larger than hundreds of lines or bytecode instructions), but in 3.8 this situation is common.Do you know why 255 became more common? Is it the side effect if an AST optimization? | |||
| msg345531 -(view) | Author: miss-islington (miss-islington) | Date: 2019-06-13 18:36 | |
New changeset5282b3b1d2e0bdf13899b1616aea20a6e3c4e13e by Miss Islington (bot) in branch '3.8':bpo-37213: Handle negative line deltas correctly in the peephole optimizer (GH-13969)https://github.com/python/cpython/commit/5282b3b1d2e0bdf13899b1616aea20a6e3c4e13e | |||
| msg345532 -(view) | Author: Pablo Galindo Salgado (pablogsal)*![]() | Date: 2019-06-13 18:51 | |
Should we backport this to 3.7 as well? | |||
| msg345534 -(view) | Author: Ned Deily (ned.deily)*![]() | Date: 2019-06-13 19:00 | |
> Should we backport this to 3.7 as well?Not unless someone can show how this is a major problem in 3.7 and then only if the changes will not introduce any 3.7.x compatibility problems. | |||
| msg345535 -(view) | Author: Pablo Galindo Salgado (pablogsal)*![]() | Date: 2019-06-13 19:01 | |
> Not unless someone can show how this is a major problem in 3.7 I would say is not a major problem in 3.7I will close the issue then. Thanks everyone who participated! | |||
| msg345562 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2019-06-14 07:10 | |
> Do you know why 255 became more common?Because the line number is now correctly set for every bytecode instruction.Compare the output inmsg345108 for 3.8 with the corresponding output in 3.7: 1 0 BUILD_LIST 0 2 LOAD_FAST 0 (.0) >> 4 FOR_ITER 12 (to 18) 2 6 STORE_FAST 1 (x) 8 LOAD_FAST 1 (x) 10 POP_JUMP_IF_FALSE 4 12 LOAD_FAST 1 (x) 14 LIST_APPEND 2 16 JUMP_ABSOLUTE 4 >> 18 RETURN_VALUE | |||
| msg345566 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2019-06-14 09:49 | |
> Because the line number is now correctly set for every bytecode instruction.That's a great enhancement! Should it be documented inhttps://docs.python.org/3.8/whatsnew/3.8.html ? | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:59:16 | admin | set | github: 81394 |
| 2019-06-14 09:49:54 | vstinner | set | messages: +msg345566 |
| 2019-06-14 07:10:34 | serhiy.storchaka | set | messages: +msg345562 |
| 2019-06-13 19:01:58 | pablogsal | set | status: open -> closed resolution: fixed messages: +msg345535 stage: patch review -> resolved |
| 2019-06-13 19:00:03 | ned.deily | set | nosy: +ned.deily messages: +msg345534 |
| 2019-06-13 18:51:17 | pablogsal | set | messages: +msg345532 |
| 2019-06-13 18:36:03 | miss-islington | set | nosy: +miss-islington messages: +msg345531 |
| 2019-06-13 18:30:07 | vstinner | set | messages: +msg345530 |
| 2019-06-13 18:17:04 | miss-islington | set | pull_requests: +pull_request13923 |
| 2019-06-13 18:16:26 | pablogsal | set | messages: +msg345529 |
| 2019-06-12 06:00:27 | serhiy.storchaka | set | files: +test_peepholer.diff messages: +msg345309 |
| 2019-06-11 20:54:52 | yselivanov | set | nosy: -yselivanov |
| 2019-06-11 20:53:44 | vstinner | set | messages: +msg345268 |
| 2019-06-11 10:14:31 | pablogsal | set | keywords: +patch stage: patch review pull_requests: +pull_request13836 |
| 2019-06-10 10:39:04 | serhiy.storchaka | set | messages: +msg345120 |
| 2019-06-10 10:12:40 | pablogsal | set | assignee:pablogsal messages: +msg345117 |
| 2019-06-10 08:32:54 | serhiy.storchaka | set | keywords: +3.8regression |
| 2019-06-10 08:24:23 | serhiy.storchaka | create | |