classification
Title: Merge generator.gi_running and frame executing flag into single frame state
Type: performance Stage: resolved
Components: Interpreter Core Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Mark.Shannon Nosy List: Mark.Shannon, chris.jerdonek, vstinner
Priority: normal Keywords: patch

Created on 2020-06-10 13:33 by Mark.Shannon, last changed 2020-09-23 15:53 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 20803 merged Mark.Shannon, 2020-06-11 13:27
PR 22377 merged vstinner, 2020-09-23 10:59
PR 22378 merged vstinner, 2020-09-23 11:00
Messages (11)
msg371194 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2020-06-10 13:33
Generators have a "gi_running" attribute (coroutines have an equivalent cr_running flag). Internally frame also has an executing flag.

We use these, plus the test `f_stacktop pointer == NULL`, to determine what state a generator or frame is in.

It would be much cleaner to maintain a single f_state field in the frame to track the state of a frame, reducing the change of ambiguity and error.

The possible states of a frame are:
CREATED -- Frame exists but has not been executed at all
SUSPENDED -- Frame has been executed, and has been suspended by a yield
EXECUTING -- Frame is being executed
RETURNED -- Frame has completed by a RETURN_VALUE instruction
RAISED -- Frame has completed as a result of an exception being raised
CLEARED -- Frame has been cleared, either by explicit call to clear or by the GC.
msg371277 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2020-06-11 13:27
This change combines the explicit state in `PyFrameObject.f_excuting` and `PyGenObject.gi_running`, and the implicit state in `PyFrameObject.f_stacktop == NULL` and `PyFrameObject.f_last == -1` into a single enum field in `PyFrameObject`.

Since we no longer need to test `PyFrameObject.f_stacktop == NULL` , The `f_stacktop` pointer can be replaced with a `f_stackdepth` integer, which make for simpler code when iterating over the stack and avoids the potential hazard of NULL pointers.

There are  three benefits to these changes:

1. The code is more robust and, IMO, easier to understand, as all state is now explicit.
2. It carries additional information about the state of the frame. Information about whether a frame exiting by `return` or by `raise` is available.
3. A modest reduction in size of frame and generator objects.
msg373388 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-07-09 09:59
Mark, I want to flag issue29590 for you ("Incorrect stack traces when re-entering a generator/coroutine stack via .throw()") in case this issue relates to or would help at all with that.
msg373401 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2020-07-09 13:20
issue29590 is unrelated
msg373813 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2020-07-17 10:44
New changeset cb9879b948a19c9434316f8ab6aba9c4601a8173 by Mark Shannon in branch 'master':
bpo-40941: Unify implicit and explicit state in the frame and generator objects into a single value. (GH-20803)
https://github.com/python/cpython/commit/cb9879b948a19c9434316f8ab6aba9c4601a8173
msg376292 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-03 10:03
The change introduced compiler warnings on Windows:

c:\vstinner\python\master\python\ceval.c(1448): warning C4244: '=': conversion  
from '__int64' to 'int', possible loss of data [C:\vstinner\python\master\PCbui 
ld\pythoncore.vcxproj]
c:\vstinner\python\master\python\ceval.c(2254): warning C4244: '=': conversion  
from '__int64' to 'int', possible loss of data [C:\vstinner\python\master\PCbui 
ld\pythoncore.vcxproj]
c:\vstinner\python\master\python\ceval.c(2271): warning C4244: '=': conversion  
from '__int64' to 'int', possible loss of data [C:\vstinner\python\master\PCbui 
ld\pythoncore.vcxproj]
msg377369 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-23 12:06
New changeset 71f2ff4ccf4ff8bdb56cc30d115ca2ddc602b12f by Victor Stinner in branch 'master':
bpo-40941: Fix fold_tuple_on_constants() compiler warnings (GH-22378)
https://github.com/python/cpython/commit/71f2ff4ccf4ff8bdb56cc30d115ca2ddc602b12f
msg377370 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-23 12:07
New changeset b7d8d8dbfe83040087a63662e0b908f4b5ac24b0 by Victor Stinner in branch 'master':
bpo-40941: Fix stackdepth compiler warnings (GH-22377)
https://github.com/python/cpython/commit/b7d8d8dbfe83040087a63662e0b908f4b5ac24b0
msg377371 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-23 12:08
The main change is merged, and I fixed compiler warnings. It seems like there is no remaining thing to do, so I close the issue.
msg377399 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2020-09-23 15:45
Thanks Victor.
Sorry I didn't get round to this sooner
msg377402 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-23 15:53
No problem. These warnings are only visible on 64-bit Windows. I spotted them waiting looking at some Windows errors.
History
Date User Action Args
2020-09-23 15:53:08vstinnersetmessages: + msg377402
2020-09-23 15:45:28Mark.Shannonsetmessages: + msg377399
2020-09-23 12:08:01vstinnersetstatus: open -> closed
versions: + Python 3.10
messages: + msg377371

resolution: fixed
stage: patch review -> resolved
2020-09-23 12:07:20vstinnersetmessages: + msg377370
2020-09-23 12:06:59vstinnersetmessages: + msg377369
2020-09-23 11:00:36vstinnersetpull_requests: + pull_request21417
2020-09-23 10:59:03vstinnersetpull_requests: + pull_request21416
2020-09-03 10:03:02vstinnersetnosy: + vstinner
messages: + msg376292
2020-07-17 10:44:27Mark.Shannonsetmessages: + msg373813
2020-07-09 13:20:11Mark.Shannonsetmessages: + msg373401
2020-07-09 10:00:00chris.jerdoneksetnosy: + chris.jerdonek
messages: + msg373388
2020-06-11 13:27:42Mark.Shannonsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request19999
2020-06-11 13:27:24Mark.Shannonsetmessages: + msg371277
2020-06-10 13:33:12Mark.Shannoncreate