If without else generates redundant jump

[eltoder]

BPO	11471
Nosy	@birkenfeld, @rhettinger, @jcea, @pitrou, @benjaminp, @eltoder, @serhiy-storchaka
Files	if_no_else.patch if_no_else_test.patch if_else_nojump.patch if_else_nojump_2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2014-09-18.01:06:58.282>
created_at = <Date 2011-03-11.21:49:15.854>
labels = ['interpreter-core', 'performance']
title = 'If without else generates redundant jump'
updated_at = <Date 2014-09-18.01:07:40.930>
user = 'https://github.com/eltoder'

bugs.python.org fields:

activity = <Date 2014-09-18.01:07:40.930>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2014-09-18.01:06:58.282>
closer = 'python-dev'
components = ['Interpreter Core']
creation = <Date 2011-03-11.21:49:15.854>
creator = 'eltoder'
dependencies = []
files = ['21091', '21092', '32117', '32120']
hgrepos = []
issue_num = 11471
keywords = ['patch']
message_count = 12.0
messages = ['130623', '130624', '199886', '199887', '199891', '199910', '199911', '199913', '199917', '226561', '227020', '227021']
nosy_count = 8.0
nosy_names = ['georg.brandl', 'rhettinger', 'jcea', 'pitrou', 'benjamin.peterson', 'python-dev', 'eltoder', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue11471'
versions = ['Python 3.5']

[eltoder]

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.

[eltoder]

Test case (needed some refactoring to avoid duplication).

[birkenfeld]

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).

[birkenfeld]

I'll make a complete patch with test suite additions (and fixing test_dis) if this is deemed to be a correct approach.

[benjaminp]

I think it's fine with the simplification I suggested.

[birkenfeld]

Updated patch with test suite update.

[benjaminp]

lgtm

[pitrou]

Just for the record, have you passed the whole test suite after cleaning up the pyc files,

[birkenfeld]

Yep :)

[serhiy-storchaka]

LGTM.

Attila Fazekas just has provided almost the same patch in bpo-22358.

[python-dev]

New changeset c0ca9d32aed4 by Antoine Pitrou in branch 'default':
Closes bpo-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

[pitrou]

Pushed after applying Serhiy's suggestion. Thank you!