joerick
Hide tracebackhidden frames
Fixes #190
This adds a generic "hidden" property to the frame record objects. Currently this is only set to true if the frame has __tracebackhide__==True in it's locals dict, as discussed in #190. Also adds a default processor that hides any frames with .hidden==True.
Not sure if this is the right approach overall, and very not sure if the changes to the c extension were the right ones. @joerick Feedback would be much appreciated.
I also added the .devcontainer dir that I came up with while developing these changes. This allows for developing pyinstrument in a container when using vscode (and maybe other ides?). Specifically, this allowed me to do stuff like simultaneously debug the python and c extension code (even though I'm developing on a mac), which was super useful. Dunno if you want this in the repo, I can remove it if you prefer.
Thanks telemonian, this is really cool. Before you continue, though, it would be worth taking a look at #203 because I think this feature you have here would work better in the new format that i've been working on over there.
Mostly, I've been trying to avoid
- adding runtime information to the frame identifier (that has caused issues in the past, because if a frame identifier changes at runtime, it splits the tree in two at that point)
- calling fast-to-locals in the c extension - my concern is that doing so will distort the timings because it changes how CPython internally stores locals.
In #203, there's a new format for the frame identifier, which is renamed to 'frame info', where the first part is an 'identifier', and then there are extra fields afterwards that store runtime information. The existence of traceback_hide would fit neatly as one of those fields.
The other thing I can think of is that perhaps it's not necessary to actually read __traceback_hide__, merely the existence of the variable could be enough to trigger the behaviour. So you could just check for existence of a variable with that name in co_varnames. That would avoid the fast-to-locals step.
Note also, that a lot of these apis have changed in Python 3.11, there might need to be some #if defines to toggle behaviours. But I think the required APIs are still available, e.g. https://docs.python.org/3.11/c-api/code.html#c.PyCode_GetVarnames
So, if you're able, do you think you'd be able to rebase this onto #203?
So, if you're able, do you think you'd be able to rebase this onto https://github.com/joerick/pyinstrument/pull/203?
Probably. Two questions:
- what's the likely eta for #203 getting pulled into main?
- can you give me a few pointers/links to relevant lines in the
invocation-attributesbranch?
calling fast-to-locals in the c extension - my concern is that doing so will distort the timings because it changes how CPython internally stores locals.
What exactly is fastToLocals and why do I need it to populate the equivalent of the locals dict in the c extension? I still don't totally grok all the fine points of the c extension.
The other thing I can think of is that perhaps it's not necessary to actually read traceback_hide, merely the existence of the variable could be enough to trigger the behaviour
I initially had it like this, but then I was like "what if some perverse person has __tracebackhide__==False explicitly, or hacks in some stupid ipython-esque automatic hiding scheme? I should support the dumb edge case". But I also didn't know about co_varnames, so if it's more efficient to skip fastToLocals and use that instead I'm fine with it
- what's the likely eta for Refactor to track information other than just time #203 getting pulled into main?
I need to devote some brain-time to the Python 3.11 support, based on the responses in this forum post. The latest tranche of API changes landed in 3.11rc1, but it's stable now, so I'll give it another try at some point in the next few weeks.
- can you give me a few pointers/links to relevant lines in the
invocation-attributesbranch?
Sure. Your code to add the hidden flag to frame info should go in the C extension around here (use the class name as a reference):
https://github.com/joerick/pyinstrument/blob/2c69b0241385339f041b13297488fcee8db3848f/pyinstrument/low_level/stat_profile.c#L419-L430
Then the code to parse the attribute should go around here:
https://github.com/joerick/pyinstrument/blob/2c69b0241385339f041b13297488fcee8db3848f/pyinstrument/frame.py#L289-L292
After that, the processor should be updated to use frame_ops, like this:
https://github.com/joerick/pyinstrument/blob/2c69b0241385339f041b13297488fcee8db3848f/pyinstrument/processors.py#L25-L38
I don't think the rest will change much. But it's a tricky merge, for sure. Sorry about that! It might be easier to start with #203 and add back components from this branch manually.
What exactly is fastToLocals and why do I need it to populate the equivalent of the locals dict in the c extension?
(disclaimer, I am no expert, I only understand a few things from reading the source code here-and-there)
In CPython, local variables are stored on the 'frame' object. There are two ways that CPython does this - either in 'fast' mode, or in a dict. The fast mode stores pointers to the values in a C array on the frame, and there's a mapping in the 'code' object that specifies which variables are stored at which index. But CPython can also store locals in a dict, that's how you can do things like:
>>> a = 1
>>> locals()['a'] = 2
>>> a
2
Once the frame object has a locals dict, it just uses that, and doesn't use the 'fast' locals anymore.
But I also didn't know about co_varnames, so if it's more efficient to skip fastToLocals and use that instead I'm fine with it
Yeah, it seems like an okay trade-off in favour of simplicity and performance to me.
Just a note, I'm gonna be working on the Python 3.11 support for #203 soon, so it might be worth holding off on that aspect of your PR until I've done the refactor.
#203 is now merged, so you can rebase onto main instead.
Awesome! Thank you @joerick for getting that pulled in so quickly. That should make things quite a bit simpler for me, or at least for my git workflow