Skip to content
This repository was archived by the owner on Feb 13, 2025. It is now read-only.

Use an unmodified PyFrameObject structure and simplify ceval.c #270

Closed
akruis opened this issue Jun 13, 2021 · 2 comments
Closed

Use an unmodified PyFrameObject structure and simplify ceval.c #270

akruis opened this issue Jun 13, 2021 · 2 comments

Comments

@akruis
Copy link

akruis commented Jun 13, 2021

Currently PyFrameObject and PyCFrameObject share the fields f_back and f_execute. The files f_execute is an Stackless specific addition to PyFrameObject.
I propose to remove the field f_execute from PyFrameObject and store the information previously held in f_execute in the field f_executing.

Rationale

The only possible values of PyFrameObject.f_execute are PyEval_EvalFrameEx_slp, slp_eval_frame_value,
slp_eval_frame_noval, slp_eval_frame_setup_with, slp_eval_frame_with_cleanup, slp_eval_frame_yield_from and - in case of unpickling invalid frames - corresponding cannot_* functions.
Of these slp_eval_frame_noval, slp_eval_frame_setup_with, slp_eval_frame_with_cleanup and slp_eval_frame_yield_from are trivial wrappers around slp_eval_frame_value with an completely identical body.

stackless/Python/ceval.c

Lines 3840 to 3913 in 74f33db

PyObject * _Py_HOT_FUNCTION
slp_eval_frame_noval(PyFrameObject *f, int throwflag, PyObject *retval)
{
PyObject *r;
/*
* this function is identical to PyEval_EvalFrame_value.
* it serves as a marker whether we expect a value or
* not, and it makes debugging a little easier.
*/
SLP_DO_NOT_OPTIMIZE_AWAY((char *)1);
r = slp_eval_frame_value(f, throwflag, retval);
return r;
}
PyObject *
slp_eval_frame_iter(PyFrameObject *f, int throwflag, PyObject *retval)
{
PyObject *r;
/*
* this function is identical to PyEval_EvalFrame_value.
* it serves as a marker whether we are inside of a
* for_iter operation. In this case we need to handle
* null without error as valid result.
*/
SLP_DO_NOT_OPTIMIZE_AWAY((char *)2);
r = slp_eval_frame_value(f, throwflag, retval);
return r;
}
PyObject *
slp_eval_frame_setup_with(PyFrameObject *f, int throwflag, PyObject *retval)
{
PyObject *r;
/*
* this function is identical to PyEval_EvalFrame_value.
* it serves as a marker whether we are inside of a
* SETUP_WITH operation.
* NOTE / XXX: see above.
*/
SLP_DO_NOT_OPTIMIZE_AWAY((char *)3);
r = slp_eval_frame_value(f, throwflag, retval);
return r;
}
PyObject *
slp_eval_frame_with_cleanup(PyFrameObject *f, int throwflag, PyObject *retval)
{
PyObject *r;
/*
* this function is identical to PyEval_EvalFrame_value.
* it serves as a marker whether we are inside of a
* WITH_CLEANUP operation.
* NOTE / XXX: see above.
*/
SLP_DO_NOT_OPTIMIZE_AWAY((char *)4);
r = slp_eval_frame_value(f, throwflag, retval);
return r;
}
PyObject *
slp_eval_frame_yield_from(PyFrameObject *f, int throwflag, PyObject *retval)
{
PyObject *r;
/*
* this function is identical to PyEval_EvalFrame_value.
* it serves as a marker whether we are inside of a
* WITH_CLEANUP operation.
* NOTE / XXX: see above.
*/
SLP_DO_NOT_OPTIMIZE_AWAY((char *)5);
r = slp_eval_frame_value(f, throwflag, retval);
return r;
}

PyEval_EvalFrameEx_slp is also a fairly simple wrapper around slp_eval_frame_value. Differences in behavior between the five execution functions slp_eval_frame* is not caused by their body, but only by an if-block in slp_eval_frame_value:

stackless/Python/ceval.c

Lines 1090 to 1116 in 74f33db

if (f->f_execute == slp_eval_frame_value) {
/* this is a return */
PUSH(retval); /* we are back from a function call */
}
else if (f->f_execute == slp_eval_frame_noval) {
f->f_execute = slp_eval_frame_value;
/* don't push it, frame ignores value */
Py_XDECREF(retval);
}
else if (f->f_execute == slp_eval_frame_iter) {
f->f_execute = slp_eval_frame_value;
goto slp_continue_slp_eval_frame_iter;
}
else if (f->f_execute == slp_eval_frame_setup_with) {
f->f_execute = slp_eval_frame_value;
goto slp_continue_slp_eval_frame_setup_with;
}
else if (f->f_execute == slp_eval_frame_with_cleanup) {
f->f_execute = slp_eval_frame_value;
goto slp_continue_slp_eval_frame_with_cleanup;
}
else if (f->f_execute == slp_eval_frame_yield_from) {
f->f_execute = slp_eval_frame_value;
goto slp_continue_slp_eval_frame_yield_from;
}
else
Py_FatalError("invalid frame function");

That is, PyFrameObject.f_execute is already used as a flag and not as a dispatch pointer. We can replace it with the already existing field f_executing. This field is a boolean flag field:

  • 0: frame is not executing
  • !0: frame is executing

C-Python stores only 0 and 1 in f_executing. Because C-Python makes no difference between various non-zero values of f_executing, it is possible to use values other than 1 to store the information currently encoded by different values of f_execute.

Now, if we drop the field f_execute from PyFrameObject we get the following benefits:

  1. Unmodified PyFrameObject: better compatibility with C-Python. Eventually we can even support the frame evaluation API "eval_frame" defined in PEP 523 (see PyCharm pydevd debug crash #240).
  2. Unmodified PyFrameObject: better maintainability.
  3. We can remove the functions slp_eval_frame_noval, slp_eval_frame_setup_with, slp_eval_frame_with_cleanup, slp_eval_frame_yield_from
  4. Probably it is possible to merge PyEval_EvalFrameEx_slp and slp_eval_frame_value. This would remove duplicated code in ceval.c and greatly improve maintainability.

Today I made a quick test implementation to prove, that the idea works. I'm going to create pull requests for the implementation.

@kristjanvalur
Copy link
Collaborator

Sounds good!

akruis added a commit that referenced this issue Jun 18, 2021
Stackless now uses an unmodified PyFrameObject structure. The field
PyFrameObject.f_executing now stores the information how to evaluate a
frame.

Additional consequences:
- all frame execution functions now take a "PyCFrameObject *" as first
  argument.
- Pickled frames no contain f_executing instead of the name of the
  execution function.
akruis pushed a commit that referenced this issue Jun 18, 2021
akruis pushed a commit that referenced this issue Jun 20, 2021
Fix test_sys.SizeofTest.test_objecttypes for frames. Stackless has now
again the same frame size as C-Python.
akruis added a commit that referenced this issue Jun 22, 2021
Eliminate duplicated code in ceval.c to improve maintainability.
Update changelog.txt.
@akruis
Copy link
Author

akruis commented Jun 22, 2021

All done. All in all a real improvement.

@akruis akruis closed this as completed Jun 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants