FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-36710: Add 'ceval' local variable to ceval.c by vstinner · Pull Request #12934 · python/cpython · GitHub

/ cpython Public

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

Merged
vstinner merged 1 commit into
python:masterfrom
vstinner:ceval_runtime
May 10, 2019
Merged

bpo-36710: Add 'ceval' local variable to ceval.c#12934
vstinner merged 1 commit into
python:masterfrom
vstinner:ceval_runtime

Conversation

vstinner commented Apr 24, 2019
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
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.

Copy link
Copy Markdown
Member Author

@jdemeyer: Would you mind to review my change?

jdemeyer commented Apr 26, 2019
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

@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 commented May 2, 2019

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
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 merged commit 09532fe into python:master May 10, 2019
vstinner deleted the ceval_runtime branch May 10, 2019 21:39
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL