Add mechanism to disable optimizations

[nedbat]

BPO	2506
Nosy	@warsaw, @brettcannon, @rhettinger, @terryjreedy, @amauryfa, @abalkin, @vstinner, @devdanzin, @nedbat, @merwok, @alex, @florentx, @ethanfurman, @markshannon, @ericsnowcurrently, @serhiy-storchaka, @1st1, @dianaclarke, @skirpichev, @artgoldberg, @pablogsal
PRs	bpo-2506: Experiment with adding a "-X noopt" flag #9693 bpo-2506: Add -X noopt command line option #13600 bpo-2506: Add mechanism to disable optimizations #22027
Files	continue.py: Test file to trace to see problem. traceme.py

^{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 2021-01-13.15:54:30.125>
created_at = <Date 2008-03-29.13:38:40.439>
labels = ['interpreter-core', 'type-feature', '3.8']
title = 'Add mechanism to disable optimizations'
updated_at = <Date 2021-01-13.15:54:30.124>
user = 'https://github.com/nedbat'

bugs.python.org fields:

activity = <Date 2021-01-13.15:54:30.124>
actor = 'Mark.Shannon'
assignee = 'none'
closed = True
closed_date = <Date 2021-01-13.15:54:30.125>
closer = 'Mark.Shannon'
components = ['Interpreter Core']
creation = <Date 2008-03-29.13:38:40.439>
creator = 'nedbat'
dependencies = []
files = ['9888', '48371']
hgrepos = []
issue_num = 2506
keywords = ['patch']
message_count = 54.0
messages = ['64692', '64699', '64713', '64716', '64720', '64725', '64726', '64729', '64754', '64764', '64765', '64768', '64774', '64775', '64809', '64957', '140281', '140290', '140303', '218811', '218814', '218816', '218829', '218869', '218891', '218892', '218893', '218894', '218923', '221250', '253345', '279493', '279494', '304195', '316928', '316930', '316935', '318510', '324296', '327021', '327022', '327036', '333185', '333245', '335013', '343717', '343752', '350448', '350560', '350576', '363926', '376147', '383615', '385044']
nosy_count = 25.0
nosy_names = ['barry', 'brett.cannon', 'rhettinger', 'terry.reedy', 'amaury.forgeotdarc', 'belopolsky', 'vstinner', 'ajaksu2', 'nedbat', 'eric.araujo', 'alex', 'flox', 'THRlWiTi', 'ethan.furman', 'Mark.Shannon', 'tshepang', 'eric.snow', 'serhiy.storchaka', 'yselivanov', 'diana', 'Trip.Volpe', 'pgimeno', 'Sergey.Kirpichev', 'ArthurGoldberg', 'pablogsal']
pr_nums = ['9693', '13600', '22027']
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue2506'
versions = ['Python 3.8']

[nedbat]

When tracing line execution with sys.settrace, a particular code
structure fails to report an executed line. The line is a continue
statement after an if condition in which the if condition is true every
time it is executed.

Attached is a file with two copies of the same code, except in the first
the if condition is always true, and in the second it is sometimes true.
In the first, trace.py reports that the continue is never executed,
even though it is (as evidenced from the values of a, b, and c after
execution).

In the second code, the continue is properly reported.

This bug has been present since version 2.3. 2.2 does not exhibit it
(trace.py didn't exist in 2.2, but coverage.py shows the problem also).

To see the problem, execute "trace.py -c -m continue.py". Then
continue.py.cover will show:

    1: a = b = c = 0
  101: for n in range(100):
  100:     if n % 2:
   50:         if n % 4:
   50:             a += 1
>>>>>>         continue
           else:
   50:         b += 1
   50:     c += 1
    1: assert a == 50 and b == 50 and c == 50

1: a = b = c = 0

101: for n in range(100):
100: if n % 2:
50: if n % 3:
33: a += 1
17: continue
else:
50: b += 1
50: c += 1
1: assert a == 33 and b == 50 and c == 50

[amauryfa]

This is because of a "peephole" optimization of the generated bytecode:
a jump instruction which target is another jump instruction can be
modified modified to target the final location.

You gain a few opcodes, but tracing is confusing...
Not sure how to fix this, though.

[abalkin]

I think this is not a bug. Here is a simpler way to illustrate the
issue:

def f(x): 
    for i in range(10): 
        if x: 
            pass 
        continue 
f(True)
f(False)

If you run the code above under trace, you get the following coverage:

1: def f(x):

22: for i in range(10):
20: if x:
10: pass
10: continue
1: f(True)
1: f(False)

Note that the 'continue' line is executed 10 instead of expected 20
times. This happens exactly as Amaury explained. If you disassemble f,
you'll see

2 0 SETUP_LOOP 34 (to 37)
3 LOAD_GLOBAL 0 (range)
6 LOAD_CONST 1 (10)
9 CALL_FUNCTION 1
12 GET_ITER
>> 13 FOR_ITER 20 (to 36)
16 STORE_FAST 1 (i)

3 19 LOAD_FAST 0 (x)
22 JUMP_IF_FALSE 4 (to 29)
25 POP_TOP

4 26 JUMP_ABSOLUTE 13
>> 29 POP_TOP

5 30 JUMP_ABSOLUTE 13
33 JUMP_ABSOLUTE 13
>> 36 POP_BLOCK
>> 37 LOAD_CONST 0 (None)
40 RETURN_VALUE

Note how peephole optimizer replaced jump to the 'continue' line (5)
from the 'pass' line (4) with a jump to the 'for' line by replacing

4 26 JUMP_FORWARD 1 (to 30)

with

4 26 JUMP_ABSOLUTE 13

I say this is not a bug because trace is correct in showing that the
continue statement is never reached when executing f(True).

[nedbat]

I see that the cause of the problem is the peephole optimizer. That
doesn't mean this isn't a problem.

I am measuring the code coverage of a set of tests, and one of my lines
is being marked as not executed. This is not the fault of the tests,
because in fact, without the optimization, the line would be executed.
Conceptually, the line has been executed (the loop is restarted, rather
than execution continuing).

I don't know what the solution to this is. Some options include fixing
the line tracing code to somehow indicate that the continue was
executed; or providing a way to disable peephole optimization for times
when accurate execution tracing is more important than speed.

[abalkin]

On Sat, Mar 29, 2008 at 2:51 PM, Ned Batchelder <report@bugs.python.org> wrote:

Ned Batchelder <nedbat@users.sourceforge.net> added the comment:

I am measuring the code coverage of a set of tests, and one of my lines
is being marked as not executed. This is not the fault of the tests,
because in fact, without the optimization, the line would be executed.
Conceptually, the line has been executed (the loop is restarted, rather
than execution continuing).

.. but the continue statement on line 5 is NOT executed in x == True
case. Note that without optimization, the if statement + the
continue line translate to

3 19 LOAD_FAST 0 (x)
22 JUMP_IF_FALSE 4 (to 29)
25 POP_TOP

4 26 JUMP_FORWARD 1 (to 30)
>> 29 POP_TOP

5 >> 30 JUMP_ABSOLUTE 13

where the second jump is to the continue statement. Peephole
optimizer recognizes that the jump target is an unconditional jump and
changes the code to jump directly to the final target bypassing the
continue line. The optimized code is

3 19 LOAD_FAST 0 (x)
22 JUMP_IF_FALSE 4 (to 29)
25 POP_TOP

4 26 JUMP_ABSOLUTE 13
>> 29 POP_TOP

5 30 JUMP_ABSOLUTE 13

If x is true, line five is NOT executed.

I don't know what the solution to this is. Some options include fixing
the line tracing code to somehow indicate that the continue was
executed; or providing a way to disable peephole optimization for times
when accurate execution tracing is more important than speed.

I think it is a good idea to provide a way to disable peephole
optimizer. In fact, I recently proposed exactly that in msg64638. My
only problem is that I would like to follow gcc tradition and make -O
option take an optional numeric argument with 0 meaning no
optimization and increasingly aggressive optimization as the argument
increases. Unfortunately -O0 will be confusingly similar to -OO.
Since -OO is not really optimization, but rather "strip" option, it
should probably migrate to -s or something. In any case, such drastic
changes to command line options are not acceptable for 2.x, but maybe
possible for 3.0.

I can easily implement -N (No optimization) or -g (debug) option that
will disable the peephole optimizer if there is support for such
feature.

[amauryfa]

Unfortunately -O0 will be confusingly similar to -OO.
On my browser, both are shown identically at the pixel level.

Microsoft compilers use -Od to disable optimization...

[rhettinger]

This has basically almost never been a problem in the real world. No
need to complicate the world further by adding yet another option and
the accompanying implementation-specific knowledge of why you would
ever want to use it.

Also, when the peepholer is moved (after the AST is created, but before
the opcodes), then little oddities like this will go away.

Recommend closing as "won't fix".

[nedbat]

I recognize that this is an unusual case, but it did come up in the real
world. I found this while measuring test coverage, and the continue
line was marked as not executed, when it was.

I don't understand "when the peepholer is moved", so maybe you are right
that this will no longer be an issue. But it seems to me to be endemic
to code optimization to lose the one-to-one correspondence between
source lines and ranges of bytecodes. And as the compiler becomes more
complex and performs more optmizations, problems like this will likely
increase, no?

In any case, I'd like to know more about the changes planned for the AST
and compiler...

[abalkin]

On Sat, Mar 29, 2008 at 4:58 PM, Raymond Hettinger
<report@bugs.python.org> wrote:

This has basically almost never been a problem in the real world.

I believe Ned gave an important use case. In coverage testing,
optimized runs can show false gaps in coverage. In addition, a no
optimize option would provide a valuable learning tool. Python has an
excellent simple VM very suitable for a case study in introductory CS
courses. Unfortunately, inability to disable peephole optimizer makes
understanding the resulting bytecode more difficult, particularly
given some arbitrary choices made by the optimizer (such as 2*3+1 =>
7, but 1+2*3 => 1+6). Furthermore, as Raymond suggested in another
thread, peephole optimizer was deliberately kept to bare minimum out
of concerns about compilation time. Given that most python code is
pre-compiled, I think it is a rare case when code size/speed
improvements would not be worth increased compilation time. In a rare
case when compilation time is an issue, users can consider disabling
optimization. Finally, an easy way to disable the optimizer would
help in developing the optimizer itself by providing an easy way to
measure improvements and debugging.

No need to complicate the world further by adding yet another option and
the accompanying implementation-specific knowledge of why you would
ever want to use it.

This would not really be a new option. Most users expect varying
levels of optimization with -O option and python already has 3 levels:
plain, -O, and -OO or Py_OptimizeFlag = 0,1, and 2. Moreover, in fact,
Py_OptimizeFlag can be set to an arbitrary positive integer using
undocumented -OOO.. option. I don't see how anyone would consider
adding say -G with Py_OptimizeFlag = -1 that would disable all
optimization as "complicating the world."

Also, when the peepholer is moved (after the AST is created, but before
the opcodes), then little oddities like this will go away.

I don't see how moving optimization up the chain will help with this
particular issue. Note that the problem is not with peepholer emiting
erroneous line number information, but the fact that the continue
statement is optimized away by replacing the if statement's jump to
continue with a direct jump to the start of the loop. As I stated in
my first comment, trace output is correct and as long as the compiler
avoids redundant double jumps, the continue statement will not show up
in trace regardless where in compilation chain it is optimized. The
only way to get correct coverage information is to disable double jump
optimization.

[rhettinger]

Weigh the cost/benefit carefully before pushing further. I don't doubt
the legitimacy of the use case, but do think it affects far fewer than
one percent of Python programmers. In contrast, introducing new
command line options is a big deal and will cause its own issues
(possibly needing its own buildbot runs to exercise the non-optimized
version, having optimized code possibly have subtle differences from
the code being traced/debugged/profiled, and more importantly the
mental overhead of having to learn what it is, why it's there, and when
to use it).

My feeling is that adding a new compiler option using a cannon to kill
a mosquito. If you decide to press the case for this one, it should go
to python-dev since command line options affect everyone.

This little buglet has been around since Py2.3. That we're only
hearing about it now is a pretty good indicator that this is a very
minor in the Python world and doesn't warrant a heavy-weight solution.

It would be *much* more useful to direct effort improving the mis-
reporting of the number of arguments given versus those required for 
instance methods:
   >>> a.f(1, 2)
   TypeError: f() takes exactly 1 argument (3 given)

[nedbat]

Raymond, do you have a cannon-less recommendation of how to kill this
particular mosquito?

[abalkin]

On Sun, Mar 30, 2008 at 5:01 PM, Raymond Hettinger
<report@bugs.python.org> wrote:
..
>  It would be *much* more useful to direct effort improving the mis-
>  reporting of the number of arguments given versus those required for
>  instance methods:
>    >>> a.f(1, 2)
>    TypeError: f() takes exactly 1 argument (3 given)

Please see bpo-2516.

[abalkin]

On Sun, Mar 30, 2008 at 5:01 PM, Raymond Hettinger
<report@bugs.python.org> wrote:
..

Weigh the cost/benefit carefully before pushing further. I don't doubt
the legitimacy of the use case, but do think it affects far fewer than
one percent of Python programmers.

I agree with you, but only because fewer than 1% of Python programmers
have complete test coverage for their code. :-) On the other hand, I
wanted a no-optimize option regardless of the trace issue. Once it is
there, I am sure everyone interested in how python compiler works will
use it. (I am not sure what % of Python programmers would fall into
that category.)

I don't know how big of a deal an extra buildbot is, but I don't think
it will be necessary. It is hard to imagine optimization that would
fix (mask) errors in non-optimized code. Therefore, a non-optimized
buildbot is unlikely to flag errors that ar not present in optimized
runs. On the other hand errors introduced by optimizer will be easier
to diagnose if they disappear when the code runs without optimization.

Mental overhead is important, but I think it will be easier to explain
the effect of no optimize option than to explain what -O does in the
current version. As far as I can tell, -O has nothing to do with
peephole optimization and only removes assert statements and replaces
__debug__ with 0. I am sure most python users are not aware of the
fact that peephole optimization is performed without -O option.

My feeling is that adding a new compiler option using a cannon to kill
a mosquito. If you decide to press the case for this one, it should go
to python-dev since command line options affect everyone.

As an alternative to the command line option, what would you say to
making sys.flags.optimize writeable and disable peepholer if
Py_OprimizeFlag < 0? This will allow python tracing tools to disable
optimization from within python code. The fact that setting
sys.flags.optimize flag will not affect modules that are already
loaded is probably a good thing because tracing code itself will run
optimized. Such tracing tools may also need to use a custom importer
that would ignore precompiled code and effectively set
dont_write_bytecode flag.

This little buglet has been around since Py2.3. That we're only
hearing about it now is a pretty good indicator that this is a very
minor in the Python world and doesn't warrant a heavy-weight solution.

I still maintain that this is not a bug. Not hearing about it before
is probably an indication that users sophisticated enough to try to
achieve full test coverage for their code were able to recognize false
coverage gaps as such.