[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Mark Shannon]

Mark Shannon  added the comment:

Nothing left to do. This is now obsolete.

--
resolution:  -> out of date
stage: patch review -> resolved
status: pending -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Aren't they were moved in issue25612? Is there something that should be done in 
this issue?

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
status: open -> pending

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Mark Shannon]

Mark Shannon added the comment:

This issue is rather old, so I will create a new GitHub PR for the code change. 
This issue can be closed.

Note that https://bugs.python.org/issue25612 is a manifestation of the problem 
that this was intended to solve.

--
nosy: +Mark Shannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Mark Shannon]

Mark Shannon added the comment:

Updated patch to support exception chaining and to merge cleanly.

--
versions: +Python 3.6 -Python 3.4
Added file: http://bugs.python.org/file40015/exc_info3.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Terry J. Reedy]

Changes by Terry J. Reedy tjre...@udel.edu:


--
versions: +Python 3.4 -Python 3.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Stefan Behnel]

Stefan Behnel sco...@users.sourceforge.net added the comment:

 As long as there is a way to access these fields directly from the
 struct (with the usual preprocessor conditional), I don't think Cython
 will actually start to use the PyErr_[GS]etExcInfo() functions in
 CPython - simply for performance reasons.

 Isn't this premature optimization? Do you have any workload where you
 measured a performance degradation?

To be honest - I don't know if it makes a measurable difference. However, the 
code is there, it's required for all currently released CPython versions (i.e. 
those still being supported for years to come), and it is used in seriously 
performance critical places, such as each generator/coroutine iteration - 
twice. Why should we add overhead to those without need?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Mark Shannon]

Mark Shannon m...@hotpy.org added the comment:

According to PEP 384 (Defining a Stable ABI) the thread state object is opaque, 
so we should be free to add or remove fields.

Nevertheless, I have added a new patch that simply moves the f_exc... fields 
from the frame object to the generator. It is not as efficient as the first 
patch, but at least it moves fields relevant only to generators into the 
generator object.

--
Added file: http://bugs.python.org/file25177/exc_info2.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

 According to PEP 384 (Defining a Stable ABI) the thread state object
 is opaque, so we should be free to add or remove fields.

Mmh, be aware the stable ABI is only a restricted part of the official
API. There are APIs which are not part of the stable ABI, but are still
public and officially supported.

That said, I agree the thread state *structure* shouldn't be part of the
official API. If necessary, we can provide accessors for some of its
members.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

This looks rather good on the principle. I think PyExcState needn't be public, 
it should be _PyExcState.
I haven't checked the detailed mechanics of the patch, I hope someone else can.

--
nosy: +pitrou

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Stefan Behnel]

Stefan Behnel sco...@users.sourceforge.net added the comment:

FWIW, Cython keeps the exception state in the generator struct and that works 
nicely.

Note that Amaury is right in that extensions use tstate-exc_value and friends. 
Cython does so quite extensively, for example. I don't see any use in changing 
the plain fields into a struct, but it will definitely break code, and not just 
some. This is also unrelated to the topic of this issue, so it should be a 
separate issue in the first place, and that should then be rejected IMHO.

Also note that there is a separate issue 14098 (with patch) on providing a 
public C-API for these three fields - as long as there is none but direct 
access to these public fields, a change that basically removes them should not 
even be seriously considered.

--
nosy: +scoder

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

 Note that Amaury is right in that extensions use tstate-exc_value and
 friends. Cython does so quite extensively, for example.

I understand for Cython, but why would pedestrian extension code look up
tstate-exc_value?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Mark Shannon]

Mark Shannon m...@hotpy.org added the comment:

3rd party code should not be accessing fields in the threadstate object,
but without the accessors proposed in issue 14098 there may be no alternative.

Once the patch for issue 14098 has been applied it, would it then be acceptable 
to remove the surperfluous fields from the frameobject?
The logic for accessing sys.exc_info can be moved to into the new 
accessor fucntions.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Stefan Behnel]

Stefan Behnel sco...@users.sourceforge.net added the comment:

I can't speak for much outside of Cython, and Cython generated modules would 
best be regenerated with a newer Cython version anyway in order to work with 
Py3.3. I'm not sure that's currently required, though.

As long as there is a way to access these fields directly from the struct (with 
the usual preprocessor conditional), I don't think Cython will actually start 
to use the PyErr_[GS]etExcInfo() functions in CPython - simply for performance 
reasons. Inlining the access is just way faster than calling into external 
functions, especially when that happens more than once.

I'm still surprised about the general lack of resistence against incompatible 
changes to a public struct, though, especially when they come without an 
obvious gain. Is this really just for code beautification? I agree that this 
would be nice, but how can it be worth breaking code?

I don't see any objection to the generator related changes in this patch.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Antoine Pitrou]

Antoine Pitrou pit...@free.fr added the comment:

 As long as there is a way to access these fields directly from the
 struct (with the usual preprocessor conditional), I don't think Cython
 will actually start to use the PyErr_[GS]etExcInfo() functions in
 CPython - simply for performance reasons.

Isn't this premature optimization? Do you have any workload where you
measured a performance degradation?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Mark Shannon]

Mark Shannon m...@hotpy.org added the comment:

Jim Jewett wrote:
  http://bugs.python.org/review/13897/diff/4186/14521
  File Python/sysmodule.c (right):
 
  http://bugs.python.org/review/13897/diff/4186/14521#newcode211
  Python/sysmodule.c:211: while ((exc_info-exc_type == NULL ||
  exc_info-exc_type == Py_None) 
  This while loop is new, but it isn't clear how it is related to
  encapsulating the exception state.  Is this fixing an unrelated bug, or
  is it from generators, or ..?
 
  http://bugs.python.org/review/13897/show

Running generators form a stack, much like frames.
Calling a generator with next or send, pushes it onto the stack,
yielding pops it.

Now consider, if you will, the threadstate object as a sort of
non-yielding (it cannot be popped) generator which forms the base
of this stack.

In this patch, rather than swapping the exception state between 
generator-owned frame and threadstate whenever entering or leaving a 
generator, each generator (and the threadstate) has its own exception state.

It order to find the topmost exception state, sys.exc_info searches the 
stack of generators until it finds one.
In practice the generator stack will be very shallow, only 1 or 2 deep,
as it is rare to have generators calling other generators
(although this will become a bit more common with PEP 380).

--
title: Move fields relevant to sys.exc_info out of frame into 
generator/threadstate - Move fields relevant to sys.exc_info out of frame into 
  generator/threadstate

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Jim Jewett]

Jim Jewett jimjjew...@gmail.com added the comment:

I think the latest patch is indeed cleaner.

--
nosy: +Jim.Jewett

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Mark Shannon]

Mark Shannon m...@hotpy.org added the comment:

This issue was too broad. The new patch is focussed on sys.exc_info.
All hints of coroutines have been removed and f_yieldfrom is untouched.

New patch reduces code by 65 lines and does not conflict with issue 13607.

--
title: Move fields relevant to coroutine/generators out of  frame into 
generator/threadstate - Move fields relevant to sys.exc_info out of frame into 
generator/threadstate
Added file: http://bugs.python.org/file24508/exc_info.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue13897] Move fields relevant to sys.exc_info out of frame into generator/threadstate

[Mark Shannon]

Changes by Mark Shannon m...@hotpy.org:


Removed file: http://bugs.python.org/file24350/coro.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue13897
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com