classification
Title: If without else generates redundant jump
Type: performance Stage: resolved
Components: Interpreter Core Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, eltoder, georg.brandl, jcea, pitrou, python-dev, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2011-03-11 21:49 by eltoder, last changed 2014-09-18 01:07 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
if_no_else.patch eltoder, 2011-03-11 21:49
if_no_else_test.patch eltoder, 2011-03-11 21:49
if_else_nojump.patch georg.brandl, 2013-10-14 13:48 review
if_else_nojump_2.patch georg.brandl, 2013-10-14 15:45 review
Messages (12)
msg130623 - (view) Author: Eugene Toder (eltoder) Date: 2011-03-11 21:49
If statement without else part generates unnecessary JUMP_FORWARD insn with jumps right to the next insn:

>>> def foo(x):
	if x: x = 1

>>> dis(foo)
  2           0 LOAD_FAST                0 (x) 
              3 POP_JUMP_IF_FALSE       15 
              6 LOAD_CONST               1 (1) 
              9 STORE_FAST               0 (x) 
             12 JUMP_FORWARD             0 (to 15) 
        >>   15 LOAD_CONST               0 (None) 
             18 RETURN_VALUE         

This patch suppresses generation of this jump.

Testing revealed another issue: when AST is produced from string empty 'orelse' sequences are represented with NULLs. However when AST is converted from Python AST objects empty 'orelse' is a pointer to 0-length sequence. I've changed this to produce NULL pointers, like in the string case. This uses less memory and doesn't introduce different code path in compiler. Without this change test_compile failed with my first change.

make test passes.
msg130624 - (view) Author: Eugene Toder (eltoder) Date: 2011-03-11 21:49
Test case (needed some refactoring to avoid duplication).
msg199886 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-10-14 13:48
Python-ast.c can't be changed; it is auto-generated.  But the whole thing can be handled in compile.c I think -- see attached patch.  Test suite passes (except for test_dis, which checks compilation result against a given list of bytecodes).
msg199887 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-10-14 13:49
I'll make a complete patch with test suite additions (and fixing test_dis) if this is deemed to be a correct approach.
msg199891 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2013-10-14 14:10
I think it's fine with the simplification I suggested.
msg199910 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-10-14 15:45
Updated patch with test suite update.
msg199911 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2013-10-14 15:47
lgtm
msg199913 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-10-14 15:49
Just for the record, have you passed the whole test suite after cleaning up the pyc files,
msg199917 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2013-10-14 16:00
Yep :)
msg226561 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-09-08 07:32
LGTM.

Attila Fazekas just has provided almost the same patch in issue22358.
msg227020 - (view) Author: Roundup Robot (python-dev) Date: 2014-09-18 01:06
New changeset c0ca9d32aed4 by Antoine Pitrou in branch 'default':
Closes #11471: avoid generating a JUMP_FORWARD instruction at the end of an if-block if there is no else-clause.
https://hg.python.org/cpython/rev/c0ca9d32aed4
msg227021 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-09-18 01:07
Pushed after applying Serhiy's suggestion. Thank you!
History
Date User Action Args
2014-09-18 01:07:40pitrousetmessages: + msg227021
2014-09-18 01:06:58python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg227020

resolution: fixed
stage: commit review -> resolved
2014-09-08 14:35:03jceasetnosy: + jcea
2014-09-08 07:32:49serhiy.storchakasetstage: patch review -> commit review
messages: + msg226561
versions: + Python 3.5, - Python 3.4
2014-09-08 07:28:36serhiy.storchakalinkissue22358 superseder
2013-12-22 21:04:07pitrousetnosy: + serhiy.storchaka
stage: patch review

versions: + Python 3.4, - Python 3.3
2013-10-14 16:00:11georg.brandlsetmessages: + msg199917
2013-10-14 15:49:39pitrousetmessages: + msg199913
2013-10-14 15:47:54benjamin.petersonsetmessages: + msg199911
2013-10-14 15:45:29georg.brandlsetfiles: + if_else_nojump_2.patch

messages: + msg199910
2013-10-14 14:10:52benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg199891
2013-10-14 13:49:23georg.brandlsetmessages: + msg199887
2013-10-14 13:48:02georg.brandlsetfiles: + if_else_nojump.patch
nosy: + rhettinger, georg.brandl, pitrou
messages: + msg199886

2011-03-11 21:49:58eltodersetfiles: + if_no_else_test.patch

messages: + msg130624
2011-03-11 21:49:15eltodercreate