Description

markshannon

opened on Jan 16, 2025

Issue body actions

We should remove the conditional stack effects in instruction definitions in bytecodes.c

Conditional stack effects already complicate code generation and that is only going to get worse with top-of-stack caching and other interpreter/JIT optimizations.

There were two reasons for having conditional stack effects:

1. Ease of porting the old ceval code to bytecodes.c
2. Performance

Reason 1 no longer applies. Instructions are much more regular now and it isn't that much work to remove the remaining conditional stack effects.

That leaves performance. I experimentally removed the conditional stack effects for LOAD_GLOBAL and LOAD_ATTR which is the worse possible case for performance as it makes no attempt to mitigate the extra dispatch costs and possibly worse specialization.
The results are here
Overall we see a 0.8% slowdown. It seems that specialization is not significantly worse, but there is a large increase in PUSH_NULL following LOAD_GLOBAL that appears to responsible for the slowdown. An extra specialization should fix that.

Prior discussion

Linked PRs

• GH-128914: Remove conditional stack effects from bytecodes.c and the code generators #128918
• Revert "GH-128914: Remove conditional stack effects from bytecodes.c and the code generators (GH-128918)" #129202
• GH-128914: Remove all but one conditional stack effects #129226

Activity

1. picnixz

   added

   type-featureA feature request or enhancementA feature request or enhancement

   interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)

   performancePerformance or resource usagePerformance or resource usage

   on Jan 17, 2025
2. picnixz commented on Jan 17, 2025

   

   picnixz

   on Jan 17, 2025

   Member

   More actions

   (Tagging it as a performance change even though we would end up with a slowdown. However, this is still something that is performance-related)
3. markshannon

   added a commit that references this issue on Jan 20, 2025

   GH-128914: Remove conditional stack effects from `bytecodes.c` and th…

   ab61d3f
4. srinivasreddy

   added a commit that references this issue on Jan 21, 2025

   pythonGH-128914: Remove conditional stack effects from `bytecodes.c` …

   b0a2da6
5. markshannon

   closed this as completedon Jan 22, 2025
6. colesbury

   added 2 commits that reference this issue on Jan 22, 2025

   Revert "pythonGH-128914: Remove conditional stack effects from `bytec…

   68ce740

   Revert "pythonGH-128914: Remove conditional stack effects from `bytec…

   f60435e
7. markshannon

   added a commit that references this issue on Jan 23, 2025

   Revert "GH-128914: Remove conditional stack effects from `bytecodes.c…

   a10f993
8. markshannon commented on Jan 23, 2025

   

   markshannon

   on Jan 23, 2025

   MemberAuthor

   More actions

   It seems that #128918 caused a 2-3% slowdown on the free-threading build.
9. markshannon

   reopened this on Jan 23, 2025
10. mdboom commented on Jan 23, 2025

    

    mdboom

    on Jan 23, 2025

    Contributor

    More actions

    It seems that #128918 caused a 2-3% slowdown on the free-threading build.

    FWIW, I couldn't reproduce this on our infrastructure. I'm not claiming one is better or more reliable, only that the effect being noticed here might be more complicated...
11. markshannon

    added a commit that references this issue on Jan 27, 2025

    GH-128914: Remove all but one conditional stack effects (GH-129226)

    75b4962
12. markshannon commented on Jan 28, 2025

    

    markshannon

    on Jan 28, 2025

    MemberAuthor

    More actions

    Latest implementation has no adverse impact on performance.
13. markshannon

    closed this as completedon Jan 28, 2025

Issue actions