bpo-45256: Don't track the exact depth of each InterpreterFrame
#30372
Conversation
Include/internal/pycore_frame.h
Outdated
| @@ -44,7 +46,7 @@ typedef struct _interpreter_frame { | |||
| int f_lasti; /* Last instruction if called */ | |||
| int stacktop; /* Offset of TOS from localsplus */ | |||
| PyFrameState f_state; /* What state the frame is in */ | |||
| int depth; /* Depth of the frame in a ceval loop */ | |||
| bool own_cframe; // Whether this is the "root" frame for the current CFrame | |||
There was a problem hiding this comment.
I'm not sure about this name.
I don't think the frame really owns the C frame. Also, be wary of using CFrame, as CFrame is a struct name.
If I were to suggest a name, it would be is_entry as this is the entry InterpreterFrame for _PyEval_EvalFrameDefault.
If you think ownership is a better mental model, then owns_cframe is fine.
There was a problem hiding this comment.
@markshannon Can it be named backed_by_cframe or cframe_backed instead?
There was a problem hiding this comment.
This has nothing to do with how the data is stored.
There was a problem hiding this comment.
I agree with Mark, I think it should be called entry_frame or something similar. Reasoning about ownership is a bit more obscure in my opinion
There was a problem hiding this comment.
I was hoping somebody would suggest a better name! is_entry it is...
Include/internal/pycore_frame.h
Outdated
| @@ -101,7 +103,7 @@ _PyFrame_InitializeSpecials( | |||
| frame->generator = NULL; | |||
| frame->f_lasti = -1; | |||
| frame->f_state = FRAME_CREATED; | |||
| frame->depth = 0; | |||
| frame->own_cframe = true; | |||
There was a problem hiding this comment.
This should default to false. Then it only need be set at the start of _PyEval_EvalFrameDefault, not for every internal call.
|
When you're done making the requested changes, leave the comment: |
We only really care whether or not an exiting frame is the "root" of the current
CFrame, which a boolean value can track more efficiently.(In fact, the current code is buggy, and doesn't even maintain
frame->depthcorrectly... it's only ever set to0or1anyways!)https://bugs.python.org/issue45256