This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Simplify the interpreter's (type, val, tb) exception representation
Type: performance Stage: resolved
Components: Interpreter Core Versions: Python 3.11
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Mark.Shannon, brandtbucher, da-woods, gvanrossum, iritkatriel, scoder, terry.reedy, vstinner
Priority: normal Keywords: patch

Created on 2021-11-04 11:29 by iritkatriel, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 29404 merged iritkatriel, 2021-11-04 13:24
PR 29495 merged iritkatriel, 2021-11-09 17:32
PR 29518 merged iritkatriel, 2021-11-10 19:17
PR 29528 merged brandtbucher, 2021-11-11 23:41
PR 29627 merged iritkatriel, 2021-11-18 19:25
PR 29780 merged iritkatriel, 2021-11-25 17:45
PR 29890 merged iritkatriel, 2021-12-02 12:37
PR 29922 merged iritkatriel, 2021-12-05 17:31
PR 30122 merged iritkatriel, 2021-12-15 14:40
PR 30289 open iritkatriel, 2022-01-01 14:03
PR 30849 merged iritkatriel, 2022-01-24 10:29
Messages (31)
msg405681 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-11-04 11:29
Exceptions are represented in the interpreter as (type, val, tb) triplets which most of the time contain redundant information (the type is the type of val and the tb is also on the exception). This complicates the code and is inefficient as opcodes that manage exceptions push and pop 3 items for each exception. 

We will change the internal representation to be (1) just the exception value if it is normalised and (2) a tuple of the 3 values for the uncommon case where they are all needed.

See also https://github.com/faster-cpython/ideas/issues/106.
msg405839 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-11-06 01:34
Would there be any change at the Python level?
msg405849 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-11-06 07:53
Initially not, neither in python nor in the c api.

It would be nice to replace PyErr_Fetch/Restore by a version that takes just an exception but that’s a long deprecation.
msg406114 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-11-10 16:57
New changeset 05fbd60147456d77a7aecf29dddd86c5bde5872f by Irit Katriel in branch 'main':
bpo-45711: Use _PyErr_ClearExcState instead of setting only exc_value to NULL (GH-29404)
https://github.com/python/cpython/commit/05fbd60147456d77a7aecf29dddd86c5bde5872f
msg406120 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2021-11-10 18:08
New changeset 4cdeee5978ee3f8ea7fe95172ae04d866cd88177 by Irit Katriel in branch 'main':
bpo-45711: remove unnecessary DUP_TOP and POP in exception handling (GH-29495)
https://github.com/python/cpython/commit/4cdeee5978ee3f8ea7fe95172ae04d866cd88177
msg406209 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-11-12 13:21
New changeset 8f1b71de731dda668aede7c9b34d0ad7afb8f6a8 by Brandt Bucher in branch 'main':
bpo-45711: Re-bump the magic number and update doc (GH-29528)
https://github.com/python/cpython/commit/8f1b71de731dda668aede7c9b34d0ad7afb8f6a8
msg406238 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-11-12 21:37
New changeset de3db1448b1b983eeb9f4498d07e3d2f1fb6d29d by Irit Katriel in branch 'main':
bpo-45711: assert that the type of exc_info is redundant (GH-29518)
https://github.com/python/cpython/commit/de3db1448b1b983eeb9f4498d07e3d2f1fb6d29d
msg406983 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-11-25 09:41
New changeset c456dfafe9f9f6614fbcf2213a93707f0e101f4e by Irit Katriel in branch 'main':
bpo-45711: use exc_value instead of exc_type to determine if exc_info is valid. Add more assertions. (GH-29627)
https://github.com/python/cpython/commit/c456dfafe9f9f6614fbcf2213a93707f0e101f4e
msg406984 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-11-25 09:53
Following the analysis/discussion on https://github.com/faster-cpython/ideas/issues/106:

* exc_info is always normalized, so we actually will never need to create a tuple of these three values (exc_curinfo, the exception raised but not yet caught can be unnormalized, but it is not pushed/popped on the stack). 

* We will reduce the interpreter's exc_info representation to just the exception instance.

* There are two APIs that are impacted, both in non-documented edge cases:

1. sys.exc_info()[2] can currently be different from sys.exc_info()[1].__traceback__ because changes to the latter (while an except clause is executing) don't show up in the former. This is arguably a bug, we will change it so that the type and traceback are always consistent with the exception.

2. PyErr_SetExcInfo does no arg checking, and will set exc_info to an inconsistent triplet if you ask it to. However, the exc_value arg must be an exception instance so the only thing you can do is pass in nonsensical args where the type/traceback do not match the exception. This function's purpose is to save/restore exc_info. We will make it ignore the type and traceback and document that change.
msg407402 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-11-30 22:37
New changeset 8a45ca542a65ea27e7acaa44a4c833a27830e796 by Irit Katriel in branch 'main':
bpo-45711: Change exc_info related APIs to derive type and traceback from the exception instance (GH-29780)
https://github.com/python/cpython/commit/8a45ca542a65ea27e7acaa44a4c833a27830e796
msg407597 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-12-03 19:05
New changeset 2ff758bd1a144ee712e96ae1db91f476c3b252bb by Irit Katriel in branch 'main':
bpo-45711: [asyncio] Normalize exceptions immediately after Fetch, before they are stored as StackItem, which should be normalized (GH-29890)
https://github.com/python/cpython/commit/2ff758bd1a144ee712e96ae1db91f476c3b252bb
msg408043 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-12-08 20:08
New changeset 2109f7880b65755329a877da3a7f8a362de07350 by Irit Katriel in branch 'main':
bpo-45711: Remove unnecessary normalization of exc_info (GH-29922)
https://github.com/python/cpython/commit/2109f7880b65755329a877da3a7f8a362de07350
msg408768 - (view) Author: Stefan Behnel (scoder) * (Python committer) Date: 2021-12-17 10:45
FYI, we track the Cython side of this in
https://github.com/cython/cython/issues/4500
msg408786 - (view) Author: Mark Shannon (Mark.Shannon) * (Python committer) Date: 2021-12-17 14:46
New changeset 396b58345f81d4c8c5a52546d2288e666a1b9b8b by Irit Katriel in branch 'main':
bpo-45711: Remove type and traceback from exc_info (GH-30122)
https://github.com/python/cpython/commit/396b58345f81d4c8c5a52546d2288e666a1b9b8b
msg409453 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2022-01-01 10:37
This is done, there will be more cleanups that this enables but they can have their own issues.
msg411497 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2022-01-24 17:45
New changeset 80e1def9ded2a1d017410394e50c88aa39135029 by Irit Katriel in branch 'main':
bpo-45711: move whatsnew entries which are incorrectly listed under New Features (GH-30849)
https://github.com/python/cpython/commit/80e1def9ded2a1d017410394e50c88aa39135029
msg412264 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-02-01 14:19
Would it be possible to revert the change until Cython and numpy are ready for it?

> bpo-45711: Remove type and traceback from exc_info (GH-30122)

This change broke numpy which uses C code generated by Cython for coroutines in numpy/random/_mt19937.c (file generated from numpy/random/_mt19937.pyx).

Example of Cython code which no fails with a compiler error since exc_info->exc_type has been removed:

/* GetTopmostException */
#if CYTHON_USE_EXC_INFO_STACK
static _PyErr_StackItem *
__Pyx_PyErr_GetTopmostException(PyThreadState *tstate)
{
    _PyErr_StackItem *exc_info = tstate->exc_info;
    while ((exc_info->exc_type == NULL || exc_info->exc_type == Py_None) &&
           exc_info->previous_item != NULL)
    {
        exc_info = exc_info->previous_item;
    }
    return exc_info;
}
#endif

I propose this plan:

* (1) Revert this specific change
* (2) Wait until a change is merged in Cython 0.29.x
* (3) Wait until a new Cython 0.29.x version is released
* (4) Wait until numpy is released with regenerated code compatible with this change
* (5) Apply again the change

It should increase the number of packages when Python 3.11 will be released.

In Cython, the issue is tracked as: https://github.com/cython/cython/issues/4500
msg412265 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2022-02-01 14:28
I can probably just put back the two fields in _PyErr_StackItem and make sure they get updated when exc_value is set. 

Reverting the whole thing would include big changes in compile.c and ceval.c, so I'd rather avoid that if we can.
msg412270 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2022-02-01 14:52
Does unsetting CYTHON_USE_EXC_INFO_STACK still work?



#if CYTHON_USE_EXC_INFO_STACK
// See  https://bugs.python.org/issue25612
#define __Pyx_ExcInfoStruct  _PyErr_StackItem
#else
// Minimal replacement struct for Py<3.7, without the Py3.7 exception state stack.
typedef struct {
    PyObject *exc_type;
    PyObject *exc_value;
    PyObject *exc_traceback;
} __Pyx_ExcInfoStruct;
#endif
msg412272 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-02-01 15:07
> In Cython, the issue is tracked as: https://github.com/cython/cython/issues/4500

I don't understand the Cython status. The code was updated in the master branch:

* https://github.com/cython/cython/pull/4584
* https://github.com/cython/cython/commit/776957022d062ed24edea192b719720118ee3576

https://github.com/cython/cython/issues/4500 was closed even if the change was not backported to 0.29.x.

I understand that there is no released Cython 0.29.x version compatible with this change.
msg412273 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-02-01 15:12
I commented the Cython issue:
https://github.com/cython/cython/issues/4500#issuecomment-1026949365

I also opened a discussion on python-dev: "Please update Cython before introcuding C API incompatible changes in Python".
https://mail.python.org/archives/list/python-dev@python.org/thread/RS2C53LDZPXHRR2VCY2G2YSPDVA4LNQU/
msg412275 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-02-01 15:19
Irit Katriel:
> Reverting the whole thing would include big changes in compile.c and ceval.c, so I'd rather avoid that if we can.

It was a good idea to split changes of this issue into multiple commits :-) I'm only proposing to revert the one which introduces the C API incompatible changes: the commit 396b58345f81d4c8c5a52546d2288e666a1b9b8b "bpo-45711: Remove type and traceback from exc_info (GH-30122)".

Maybe Cython can be quickly updated, but the situation seems to be complicated :-( https://github.com/cython/cython/issues/4500
msg412281 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2022-02-01 15:47
That commit has significant changes in ceval.c and compile.c. They don't need to be reverted to unbreak cython.  I'm working on a PR for a simpler change.

Have you tried with CYTHON_USE_EXC_INFO_STACK undefined?
msg412289 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2022-02-01 16:46
If this is still the position of cython maintainers:

https://github.com/cython/cython/issues/4581#issuecomment-1016503683

then I will need to revert the change until 3.12.
msg412290 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2022-02-01 16:50
Time to insist on directly communicating with the Cython team (esp. @scoder) and broker some kind of compromise.
msg412292 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-02-01 17:08
> __Pyx_PyErr_GetTopmostException(PyThreadState *tstate)

Python provides a *private* _PyErr_GetTopmostException(tstate) function, but Cython reimplements its own function. I'm not sure why.

GH-30531 proposes adding PyErr_GetActiveException() function which has no parameter, but Cython __Pyx_PyErr_GetTopmostException() has a tstate parameter.

Simplified example numpy/random/_mt19937.c code:

static CYTHON_INLINE void
__Pyx__ExceptionSave(PyThreadState *tstate, PyObject **type,
                     PyObject **value, PyObject **tb)
{
    _PyErr_StackItem *exc_info = __Pyx_PyErr_GetTopmostException(tstate);
    *type = exc_info->exc_type;
    *value = exc_info->exc_value;
    *tb = exc_info->exc_traceback;
    Py_XINCREF(*type);
    Py_XINCREF(*value);
    Py_XINCREF(*tb);
}

static CYTHON_INLINE void
__Pyx__ExceptionReset(PyThreadState *tstate, PyObject *type,
                      PyObject *value, PyObject *tb)
{
    PyObject *tmp_type, *tmp_value, *tmp_tb;
    _PyErr_StackItem *exc_info = tstate->exc_info;
    tmp_type = exc_info->exc_type;
    tmp_value = exc_info->exc_value;
    tmp_tb = exc_info->exc_traceback;
    exc_info->exc_type = type;
    exc_info->exc_value = value;
    exc_info->exc_traceback = tb;
    Py_XDECREF(tmp_type);
    Py_XDECREF(tmp_value);
    Py_XDECREF(tmp_tb);
}

Cython saves/restores the current exception of tstate. Maybe we need to provide a high-level API for that as well?
msg412303 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2022-02-01 19:32
This is a backport of @scoder's patch to 0.29.x. (I don't know if this is helpful).

https://github.com/cython/cython/compare/master...iritkatriel:exc_info?expand=1
msg412304 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2022-02-01 19:52
> GH-30531 proposes adding PyErr_GetActiveException() function which has no parameter, but Cython __Pyx_PyErr_GetTopmostException() has a tstate parameter.


I've now updated it to follow the pattern of other functions, where the is a private function that takes tstate and the public function calls it.

So it adds in Include/pyerrors.h 

PyAPI_FUNC(PyObject*) PyErr_GetActiveException(void);
PyAPI_FUNC(void) PyErr_SetActiveException(PyObject *);

and in Include/cpython/pyerrors.h

PyAPI_FUNC(PyObject*) _PyErr_GetActiveException(PyThreadState *);
PyAPI_FUNC(void) _PyErr_SetActiveException(PyThreadState *, PyObject *);
msg412316 - (view) Author: (da-woods) * Date: 2022-02-01 22:02
Just a quick comment on Cython and these changes:

Cython 0.29 can build itself on Python 3.11a4 with `CFLAGS="-DCYTHON_FAST_THREAD_STATE=0 -DCYTHON_USE_EXC_INFO_STACK=0" python3.11 setup.py build_ext`.

I think there's some coroutine code left that isn't fixed by those flags, but a large chunk of Cython libraries won't use coroutines and so will work fine with these flags. Hopefully that unblocks some stuff for you.
msg414482 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2022-03-03 22:14
lxml does crash on the current Cython 0.29.x development branch.
msg415086 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2022-03-13 19:33
We have agreed on python-dev [1] that the cpython changes will not be reverted, and the issue will be fixed in cython. So I am closing this again.

[1] https://mail.python.org/archives/list/python-dev@python.org/message/BHIQL4P6F7OPMCAP6U24XEZUPQKI62UT/
History
Date User Action Args
2022-04-11 14:59:52adminsetgithub: 89874
2022-03-13 19:33:16iritkatrielsetstatus: open -> closed
resolution: fixed
messages: + msg415086
2022-03-03 22:14:23vstinnersetmessages: + msg414482
2022-02-01 22:02:02da-woodssetnosy: + da-woods
messages: + msg412316
2022-02-01 19:52:16iritkatrielsetmessages: + msg412304
2022-02-01 19:32:23iritkatrielsetmessages: + msg412303
2022-02-01 17:08:38vstinnersetmessages: + msg412292
2022-02-01 16:50:45gvanrossumsetmessages: + msg412290
2022-02-01 16:46:59iritkatrielsetmessages: + msg412289
2022-02-01 15:47:03iritkatrielsetmessages: + msg412281
2022-02-01 15:19:00vstinnersetmessages: + msg412275
2022-02-01 15:12:45vstinnersetmessages: + msg412273
2022-02-01 15:07:35vstinnersetmessages: + msg412272
2022-02-01 14:52:41iritkatrielsetmessages: + msg412270
2022-02-01 14:28:12iritkatrielsetmessages: + msg412265
2022-02-01 14:19:59vstinnersetstatus: closed -> open

nosy: + vstinner
messages: + msg412264

resolution: fixed -> (no value)
2022-01-24 17:45:15iritkatrielsetmessages: + msg411497
2022-01-24 10:29:59iritkatrielsetpull_requests: + pull_request29031
2022-01-01 14:03:04iritkatrielsetpull_requests: + pull_request28539
2022-01-01 10:37:51iritkatrielsetpull_requests: - pull_request28217
2022-01-01 10:37:40iritkatrielsetpull_requests: - pull_request28193
2022-01-01 10:37:29iritkatrielsetstatus: open -> closed
resolution: fixed
messages: + msg409453

stage: patch review -> resolved
2022-01-01 10:36:19iritkatrielsetpull_requests: - pull_request28503
2021-12-29 14:37:17iritkatrielsetpull_requests: + pull_request28503
2021-12-17 14:46:33Mark.Shannonsetmessages: + msg408786
2021-12-17 10:45:39scodersetmessages: + msg408768
2021-12-17 09:49:45scodersetnosy: + scoder
2021-12-15 14:40:12iritkatrielsetpull_requests: + pull_request28341
2021-12-08 22:31:08iritkatrielsetpull_requests: + pull_request28217
2021-12-08 20:08:11iritkatrielsetmessages: + msg408043
2021-12-07 17:47:35iritkatrielsetpull_requests: + pull_request28193
2021-12-05 17:31:12iritkatrielsetpull_requests: + pull_request28146
2021-12-03 19:05:23iritkatrielsetmessages: + msg407597
2021-12-02 12:37:21iritkatrielsetpull_requests: + pull_request28114
2021-11-30 22:37:29iritkatrielsetmessages: + msg407402
2021-11-25 17:45:45iritkatrielsetpull_requests: + pull_request28017
2021-11-25 09:53:52iritkatrielsetmessages: + msg406984
2021-11-25 09:41:41iritkatrielsetmessages: + msg406983
2021-11-18 19:25:08iritkatrielsetpull_requests: + pull_request27859
2021-11-12 21:37:36iritkatrielsetmessages: + msg406238
2021-11-12 13:21:53iritkatrielsetmessages: + msg406209
2021-11-11 23:41:36brandtbuchersetnosy: + brandtbucher
pull_requests: + pull_request27778
2021-11-10 19:17:44iritkatrielsetpull_requests: + pull_request27770
2021-11-10 18:08:41Mark.Shannonsetmessages: + msg406120
2021-11-10 16:57:23iritkatrielsetmessages: + msg406114
2021-11-09 17:32:00iritkatrielsetpull_requests: + pull_request27745
2021-11-06 07:53:27iritkatrielsetmessages: + msg405849
2021-11-06 01:34:06terry.reedysettype: performance

messages: + msg405839
nosy: + terry.reedy
2021-11-04 13:24:54iritkatrielsetkeywords: + patch
stage: patch review
pull_requests: + pull_request27660
2021-11-04 11:29:47iritkatrielcreate