classification
Title: co_stacksize is calculated from unoptimized code
Type: performance Stage: resolved
Components: Interpreter Core Versions: Python 3.6
process
Status: closed Resolution: out of date
Dependencies: Superseder: AST-level Constant folding
View: 29469
Assigned To: Nosy List: Jelle Zijlstra, benjamin.peterson, brett.cannon, georg.brandl, meador.inge, ncoghlan, serhiy.storchaka, vstinner, yselivanov, ztane
Priority: normal Keywords:

Created on 2016-03-12 22:45 by ztane, last changed 2017-12-14 13:30 by vstinner. This issue is now closed.

Messages (7)
msg261668 - (view) Author: Antti Haapala (ztane) * Date: 2016-03-12 22:45
When answering a question on StackOverflow, I noticed that a function that only loads a constant tuple to a local variable still has a large `co_stacksize` as if it was built with BUILD_TUPLE.

e.g.

    >>> def foo():
    ...     a = (1,2,3,4,5,6,7,8,9,10)
    ...
    >>> foo.__code__.co_stacksize
    10
    >>> dis.dis(foo)
      2           0 LOAD_CONST              11 ((1, 2, 3, 4, 5, 6, 7, 8, 9, 10))
                  3 STORE_FAST               0 (a)
                  6 LOAD_CONST               0 (None)
                  9 RETURN_VALUE

I suspect it is because in the `makecode` the stack usage is calculated from the unoptimized assembler output instead of the actual optimized bytecode. I do not know if there is any optimization that would increase the stack usage, but perhaps it should be calculated from the resulting output.
msg261700 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-03-13 17:36
I also suspect you're right, Antti, that the stack size is calculated prior to the bytecode being passed to through the peepholer which would have made the built tuple a value in the const array.

Off the top of my head I don't remember where the stack size calculation is made, but my suspicion is it's in the AT -> bytecode step, which would mean making it work from bytecode would mean re-implementing that calculation to work from the bytecode itself (assuming I'm right).
msg264088 - (view) Author: Jelle Zijlstra (Jelle Zijlstra) * (Python triager) Date: 2016-04-24 01:34
This also affects co_consts, which includes constants that are no longer used by the optimized code:

In [8]: def f():
    return (1, 2, 3, 4, 5)
   ...: 

In [9]: f.func_code.co_consts
Out[9]: (None, 1, 2, 3, 4, 5, (1, 2, 3, 4, 5))

In [12]: dis.dis(f)
  2           0 LOAD_CONST               6 ((1, 2, 3, 4, 5))
              3 RETURN_VALUE
msg265217 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2016-05-10 02:02
Strictly speaking, the stack size is calculated *after* the peephole optimizer is run, but the code that computes the stack depth operates on the basic block graph instead of the assembled and optimized byte code.

Anyway, the conclusion is the same as Brett noted -- the stack depth analysis would need to be re-written to operate on the optimized bytecode array.
msg265702 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2016-05-16 15:54
See also issue24340
msg308281 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-12-14 08:27
Seems moving constant folding to the AST level (issue29469) have solved this issue.

>>> def foo():
...     a = (1,2,3,4,5,6,7,8,9,10)
... 
>>> foo.__code__.co_stacksize
1
msg308298 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-12-14 13:30
> Seems moving constant folding to the AST level (issue29469) have solved this issue.

Wow, it's cool to see this bug finally fixed!
History
Date User Action Args
2017-12-14 13:30:36vstinnersetmessages: + msg308298
2017-12-14 08:27:59serhiy.storchakasetstatus: open -> closed
superseder: AST-level Constant folding
messages: + msg308281

resolution: out of date
stage: resolved
2017-07-22 09:33:07pitrousetnosy: + vstinner, serhiy.storchaka
2016-05-18 19:40:37vstinnersettype: performance
2016-05-16 15:54:03meador.ingesetmessages: + msg265702
2016-05-10 02:02:02meador.ingesetnosy: + meador.inge
messages: + msg265217
2016-04-24 01:34:40Jelle Zijlstrasetnosy: + Jelle Zijlstra
messages: + msg264088
2016-03-13 17:36:44brett.cannonsetmessages: + msg261700
2016-03-13 16:02:52SilentGhostsetnosy: + brett.cannon, georg.brandl, ncoghlan, benjamin.peterson, yselivanov
2016-03-12 22:45:45ztanecreate