Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-36710: Add 'ceval' local variable to ceval.c #12934

Merged
merged 1 commit into from
May 10, 2019
Merged

bpo-36710: Add 'ceval' local variable to ceval.c #12934

merged 1 commit into from
May 10, 2019

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Apr 24, 2019

Add "struct _ceval_runtime_state *ceval = &_PyRuntime.ceval;" local
variables to function to better highlight the dependency on the
global variable _PyRuntime and to point directly to _PyRuntime.ceval
field rather than on the larger _PyRuntime.

Changes:

  • Add _PyRuntimeState_GetThreadState()
  • _PyThreadState_GET() now calls _PyRuntimeState_GetThreadState()
  • Add an explicit 'ceval' parameter to macros like
    COMPUTE_EVAL_BREAKER() or SIGNAL_PENDING_SIGNALS().
  • Add 'tstate' parameter to call_function(), do_call_core() and
    do_raise().
  • Add 'runtime' parameter to _Py_CURRENTLY_FINALIZING().
  • Declare 'runtime', 'tstate', 'ceval' and 'eval_breaker' variables
    as constant.

https://bugs.python.org/issue36710

@vstinner
Copy link
Member Author

This change adds a "tstate" parameter to call_function(), do_call_core() and do_raise(): @jdemeyer proposed a similar change in PR #12839. Here my intent is not optimization, but more correctness: ensure that runtime, interp and tstate are consistent in ceval.c.

@vstinner
Copy link
Member Author

@jdemeyer: Would you mind to review my change?

@jdemeyer
Copy link
Contributor

jdemeyer commented Apr 26, 2019

@jdemeyer: Would you mind to review my change?

As discussed on bpo-36710 (let's keep that discussion there), I'm missing the why: which problem does this PR solve? Some of these changes do make sense independently to clean things up (for example, reducing the number of _PyThreadState_GET() calls like #12839). But some of the changes also make things more complicated, so they need to be justified.

@vstinner
Copy link
Member Author

vstinner commented May 2, 2019

cc @ericsnowcurrently: I was talking about this change with you.

@vstinner
Copy link
Member Author

I rebased my PR on top of the master branch.

Add "struct _ceval_runtime_state *ceval = &_PyRuntime.ceval;" local
variables to function to better highlight the dependency on the
global variable _PyRuntime and to point directly to _PyRuntime.ceval
field rather than on the larger _PyRuntime.

Changes:

* Add _PyRuntimeState_GetThreadState(runtime) macro.
* Add _PyEval_AddPendingCall(ceval, ...) and
  _PyThreadState_Swap(gilstate, ...) functions.
* _PyThreadState_GET() macro now calls
  _PyRuntimeState_GetThreadState() using &_PyRuntime.
* Add 'ceval' parameter to COMPUTE_EVAL_BREAKER(),
  SIGNAL_PENDING_SIGNALS(), _PyEval_SignalAsyncExc(),
  _PyEval_SignalReceived() and _PyEval_FiniThreads() macros and
  functions.
* Add 'tstate' parameter to call_function(), do_call_core() and
  do_raise().
* Add 'runtime' parameter to _Py_CURRENTLY_FINALIZING(),
  _Py_FinishPendingCalls() and _PyThreadState_DeleteExcept()
  macros and functions.
* Declare 'runtime', 'tstate', 'ceval' and 'eval_breaker' variables
  as constant.
@vstinner vstinner merged commit 09532fe into python:master May 10, 2019
@vstinner vstinner deleted the ceval_runtime branch May 10, 2019 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants