Title: Expose called function on frame object
Type: enhancement Stage:
Components: Interpreter Core Versions: Python 3.5
Status: open Resolution:
Dependencies: Superseder:
Assigned To: eric.snow Nosy List: Arfrever, benjamin.peterson, cvrebert, daniel.urban, eric.snow, jcea, meador.inge, ncoghlan, yselivanov
Priority: normal Keywords: patch

Created on 2011-08-30 05:54 by eric.snow, last changed 2016-11-01 02:45 by BreamoreBoy.

File name Uploaded Description Edit
called_function.diff eric.snow, 2011-08-30 05:54 review
called_function_2.diff eric.snow, 2011-09-14 05:03 review
Messages (11)
msg143201 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2011-08-30 05:54
This patch adds f_func to PyFrameObject and sets it for functions that get called (in PyFrame_New).  For classes and modules it is set to None.  The difference in performance was not noticable, as far as I could tell.  However, I am willing to do more than just time 'make test' a few times if there is any concern.

A couple weeks ago a thread on python-ideas centered on the subject matter of PEP 3130[1].  The discussion started with, and mainly involved, the idea of adding __function__ to the frame locals during execution of the function.  __function__ would point to the function that was called, which had resulted in the frame.

I spent quite a bit of time getting this to work using a closure, but the result was overkill.  It also made it too easy to use __function__ for recursion, which Guido did not like (and I agree).

At this point it dawned on me that it would be much simpler to just add the called function to the frame object.  This patch is the result.  In the end it is much more efficient than the locals approach I had been taking.

msg143248 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2011-08-31 04:43
Thanks for the review, Nick.  I'll be uploading a new patch in a couple hours with your recommended fixes.

Regarding the comments on python-ideas, would it be better to use a weakref proxy around the function, to help with the reference cycles?  That's what I was doing with my closure-based solution.  I didn't do it here just to see what would happen and I didn't see any problems in my very limited testing (basically just 'make test').  I don't mind using weakrefs and, if it matters, I could pre-allocate the weakref proxy in PyFunction_New to save a little overhead at each call. 

For the moment I left in the code to limit f_func to only functions.  I'll respond to that on python-ideas.

Finally, how does this patch relate to the ABI?  I'm not too familiar with it (read through PEP 384) and want to make sure I'm okay here.
msg143249 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2011-08-31 05:47
On second thought, I probably won't be able to get an updated patch tonight.  I need to mull over the PyEval_EvalFunction implementation and the interaction with fast_function.
msg144008 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2011-09-14 05:03
Finally had a chance to get back to this.  Here's a new patch in response to Nick's review.

My only concern is the new _PyEval_EvalFunctionCode function.  It is basically the existing PyEval_EvalCodeEx function with an extra parameter.  I've turned PyEval_EvalCodeEx() into a very light wrapper around _PyEval_EvalFunctionCode().  This is how I tackled the backwards incompatibility of my previous patch.  However, I am nervous about messing around with something like PyEval_EvalCodeEx().

I was toying with the idea of doing a more comprehensive refactor involving call_function, fast_function, and do_call, which seems like it would be a better fix.  However, I'm even more reticent to make that scale of changes, especially on something so critical, even if it seems right to me currently.

I figured the safe thing for now would be to name the new function with a leading underscore to mark it as private.

Here are the results from my cursory check before and after my latest patch:

3 tests failed:
    test_distutils test_packaging test_socket
[293234 refs]
real	14m40.578s
user	11m43.547s
sys	0m52.096s

3 tests failed:
    test_distutils test_packaging test_socket
[293171 refs]
real	14m33.785s
user	11m55.437s
sys	0m53.850s
msg144064 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2011-09-15 05:38
Nick, does thing look better?
msg150280 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2011-12-28 04:23
What would you use the functionality provided by this patch for?
msg150282 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2011-12-28 06:30
My response to a similar query:

What it enables is the ability to introspect the *actual* function object belonging to the currently executing code, whether in the same execution frame or in a later one on the stack.  We don't have that now.  The best we can do is guess, and I'm trying to refuse the temptation by building in an alternative that doesn't get in the way (and doesn't provide an attractive nuisance).

My motivation for this patch was to be able to find out, in one of my functions, what function called it and to interact with that function object.  The name or file represented in the code object on the stack didn't cut it; and decorators and function factories make the whole thing a mess.  Guido sounded rather positive to the idea, which is why I pursued it.  From Guido's post to which I linked in the issue[1]:

   > For me the use case involves determining what function called my
   > function.  Currently you can tell in which execution frame a function
   > was called, and thereby which code object, but reliably matching that
   > to a function is not so simple.  I recognize that my case is likely
   > not a general one.

   But it is a nice one. It solves some issues that pdb currently solves
   by just using a file/line reference.

Not that it necessarily matters, but I could dig up the bit of code that motivated the whole thing.  I recall it was something related to writing an enum library (a la flufl.enum).  Anyway, I'd love to see this move forward.

msg156961 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-03-28 01:36
It's been a while and I probably need to clean up that last patch a little.  I also need to address the questions of:

1. f_func for class/module execution
2. thread-safety

msg156962 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-03-28 01:48
for reference, a related issue:
msg222158 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-07-03 08:28
Note that #13672 referred to in msg156962 refers to #13855.
msg279846 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-11-01 01:29
This topic came up in a discussion I was having with Yury, and thanks to generators and coroutines, I don't think "f_func" would be the right name for an attribute like this, with something more neutral like "f_origin" potentially being preferable

The specific runtime debugging use case I have in mind is this:

1. Given a frame reference, navigate to the generator-iterator or coroutine that spawned that frame
2. Given gi_back and cr_back doubly-linked counterparts to the current gi_yieldfrom and cr_await introspection attributes on generators and coroutines, expose the relevant generator and/or coroutine stack in addition to the frame stack

These secondary stacks could then potentially be incorporated into traceback outputs by default
Date User Action Args
2016-11-01 02:45:26BreamoreBoysetnosy: - BreamoreBoy
2016-11-01 01:29:20ncoghlansetnosy: + yselivanov
messages: + msg279846
2014-07-03 08:28:15BreamoreBoysetnosy: + BreamoreBoy

messages: + msg222158
versions: + Python 3.5, - Python 3.3
2013-06-25 05:29:09eric.snowsetassignee: eric.snow
2012-03-28 01:48:31eric.snowsetmessages: + msg156962
2012-03-28 01:36:25eric.snowsetmessages: + msg156961
2011-12-28 22:37:19jceasetnosy: + jcea
2011-12-28 20:35:04Arfreversetnosy: + Arfrever
2011-12-28 06:30:47eric.snowsetmessages: + msg150282
2011-12-28 04:23:20benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg150280
2011-12-27 23:34:34meador.ingesetnosy: + meador.inge
2011-12-27 19:02:59cvrebertsetnosy: + cvrebert
2011-09-15 05:38:07eric.snowsetnosy: + ncoghlan
messages: + msg144064
2011-09-14 05:03:21eric.snowsetfiles: + called_function_2.diff

messages: + msg144008
2011-08-31 05:47:14eric.snowsetmessages: + msg143249
2011-08-31 04:43:51eric.snowsetmessages: + msg143248
2011-08-30 09:49:03daniel.urbansetnosy: + daniel.urban
2011-08-30 05:54:27eric.snowcreate