Add `PyUnstable_InterpreterFrame_GetLocals()`: read a frame’s locals from the interpreter-frame C API

Core Development C API
1

I work on TorchDynamo, PyTorch’s JIT compiler. Dynamo relies on the eval frame API (PEP 523), and one of the things it needs to do when a frame is traced is to read the frame’s local variables.

CPython has 3 public APIs for reading frame locals, but none of them are suitable to be used in Dynamo:

  • PyFrame_GetLocals(PyFrameObject*)
  • PyFrame_GetVar(PyFrameObject*, PyObject*)
  • PyEval_GetFrameLocals(void)

The first two take a PyFrameObject* as first argument and there’s no public way to get one from a _PyInterpreterFrame*. The last one (PyEval_GetFrameLocals) reads the current frame, and the frame might not be ready when Dynamo gets it.

So today PyTorch implements its own version of *_GetLocals, which relies on an interface that is fragile and hard to maintain: it requires defining Py_BUILD_CORE, uses internal CPython headers (Include/internal/pycore_*), copies CPython code (PyFrame_Clear, PyFrame_MakeAndSetFrameObject), and pokes at private struct fields such as frame->localsplus. As a result, each CPython update requires manual re-auditing and maintenance before PyTorch can support it.

I would like to propose a new PyUnstable_* function (PyUnstable_InterpreterFrame_GetLocals()), to provide downstream projects (namely PyTorch) a supported way to access interpreter-frame locals without reimplementing CPython’s internal frame-local logic.

There is precedent for this: the interpreter frame API already exposes 3 PyUnstable_InterpreterFrame_* accessors:

  • PyUnstable_InterpreterFrame_GetCode(_PyInterpreterFrame*) [ref]
  • PyUnstable_InterpreterFrame_GetLasti(_PyInterpreterFrame*) [ref]
  • PyUnstable_InterpreterFrame_GetLine(_PyInterpreterFrame*) [ref]

PyUnstable_InterpreterFrame_GetLocals(...) would be the natural fourth.

Spec

PyAPI_FUNC(int) PyUnstable_InterpreterFrame_GetLocals(
	struct _PyInterpreterFrame *frame, PyObject **out);

out is a caller-allocated array of exactly co_nlocalsplus PyObject* slots. On success, the function fills out[0 .. co_nlocalsplus) with the frame’s local variables indexed by localsplus index, and returns co_nlocalsplus. Every non-NULL slot holds a strong reference that the caller must release. A NULL means the slot has no value (an unbound, deleted, or hidden local). Cell and free variables are unboxed and the slots receive the cell’s contents, matching what locals() exposes.

On error, the function sets ValueError and returns -1. The only error condition is an invalid output buffer (out is NULL).

Example usage

A proposal of PyUnstable_InterpreterFrame_GetLocals was implemented in forks of CPython and PyTorch at:

2

@Dino would cinderx have a use for such an API?

3

IMO, this should come with adding a PyUnstable opaque typedef for _PyInterpreterFrame. Keeping the type private while adding public APIs around it is contradictory.

You’ll likely get opposition from @markshannon for exposing the internals and @steve.dower for having to manage an array of PyObject*.

AFAIK this might need allocation you can also get a MemoryError. (It’s best not to put allowed errors in a spec; you always assume an unknown exception can happen.)

Consider taking that size as an argument & checking it, unless this would slow things down measurably. Makes more sense than a NULL check, IMO.

Adding one (_PyFrame_GetFrameObject but returning a strong ref) might be another option. But that’s expensive: it involves a allocating the refcounted Python object and lots of copying, and the PyFrame_GetLocals call will do a bunch more of that.

On the other hand, AFAIK the PyUnstable_InterpreterFrame_GetLocals proposed here will need to convert locals from stack references to strong PyObject*s, so it isn’t super cheap either.

1 Like
4

@Dino would cinderx have a use for such an API?

I don’t know that we immediately would. At some point we’ll probably start doing on-stack replacement where we could potentially use this API. But we’re also tightly bound to _PyInterpreterFrame elsewhere and it’ll always need to be non-opaque to us (we create them on the C stack and embed them in JIT generator objects).

If this got PyTorch to the place where they’re not defining Py_BUILD_CORE though that seems huge. But FWIW PyFrame_MakeAndSetFrameObject and _PyFrame_ClearExceptCode are now exported in 3.15 so at the very least you can stop copying the code when you do build w/ Py_BUILD_CORE on newer versions.

I guess one question is how performance sensitive this is for PyTorch. Would _PyFrame_GetFrameObject work if it was available in the unstable API? If PyTorch is about to kick off tracing of a method does the overhead of creating the frame matter?

1 Like
5

Eh, it’s not so bad as the other cases where I was opposed to that. At least here the array already exists and so we’re unlikely to hit value-related failures during creation, plus it’s on our side so we can do the extended checks necessary to clean up.[1]

And the failure just results in nothing needing to be cleaned up, success results in everything needing to be cleaned up, and if the array was successfully created then it gets deallocated unconditionally. So the switch matrix after the call is much simpler.

Still, I’d prefer a “get-by-index” type API. You already need to know the number of locals in this case, so iterating with that is possible, and it would greatly optimise the case where you already know which one you want. Realising all the locals at once could be expensive, and I suspect in the cases where it’s “necessary” it isn’t actually the best way to achieve what you’re trying to achieve.

Pretty sure this is just the result of not doing a broad rename of everything that existed prior to inventing PyUnstable. For the sake of not breaking existing code, I wouldn’t bother.

  1. For context, I’ve been strongly opposed to APIs for constructing dict from C arrays of PyObject *, because the user is going to have to potentially handle failures as they generate each element, which likely means a forward loop to fill it out plus a partial backward loop to clean up on error as well as potentially a full loop to clean up on the dict API failure, plus choosing whether or not to release individual objects in the array before deallocating the array. Too many choices for the user, in other words. ↩︎

1 Like
6

AFAIK this might need allocation you can also get a MemoryError. (It’s best not to put allowed errors in a spec; you always assume an unknown exception can happen.)

Got it. Thanks for the tip.

Consider taking that size as an argument & checking it, unless this would slow things down measurably. Makes more sense than a NULL check, IMO.

Measured and the time difference is negligible.

But FWIW PyFrame_MakeAndSetFrameObject and _PyFrame_ClearExceptCode are now exported in 3.15 so at the very least you can stop copying the code when you do build w/ Py_BUILD_CORE on newer versions.

Oh, nice to hear that. We will definitely try this. Do you know why they are being exposed? Would it be possible to expose a few more APIs as well?

I guess one question is how performance sensitive this is for PyTorch. Would _PyFrame_GetFrameObject work if it was available in the unstable API? If PyTorch is about to kick off tracing of a method does the overhead of creating the frame matter?

I think _PyFrame_GetFrameObject will not work for us. Dynamo intercepts a frame before it starts executing, and on a non-yet-started frame PyFrame_GetLocals → frame_init_get_vars mutates it.

On top of that, PyFrame_GetLocals can be a bit slow:\

route ns/call
A — our GetLocals (fills array, extracts all values as strong refs) 291 usable locals
B — _PyFrame_GetFrameObject + PyFrame_GetLocals (proxy) 260 lazy — no values extracted
C — B + materialize the proxy’s values into a dict 4095 usable locals, ~14×

Still, I’d prefer a “get-by-index” type API. You already need to know the number of locals in this case, so iterating with that is possible, and it would greatly optimise the case where you already know which one you want. Realising all the locals at once could be expensive, and I suspect in the cases where it’s “necessary” it isn’t actually the best way to achieve what you’re trying to achieve.

That works for us. Dynamo do materialize all the locals today, but that’s mostly a consequence of how the code is written than a hard requirement. I’ll switch the proposal to PyUnstable_InterpreterFrame_GetLocal(frame, idx).

One other question, should I introduce the opaque InterpreterFrame typedef? And what would be the next steps? Can I open the PR on GitHub with the patch?

7

If that’s the only issue we could add a PyFrame_GetVarByIndex (after PyFrame_GetVar.)

Sounds good.

Nah, that can go in a different PR.

Guess this is related to your post from an unrelated thread: “The unavoidable premise is that the docs are the definitive source of truth on what’s “public” (aka. intended to be used), because that’s how it’s always been done.”

Except… that’s not unavoidable, and I think that in the C API, we can get to a state where the leading underscore means “don’t touch”. Adding PyUnstable versions for documented-but-underscored API has been argued, and even approved in PEP 689. Also, PEP 387 very explicitly excludes underscored names from public API.

For the sake of not breaking existing code, we can soft-deprecate the old name instead of using Py_DEPRECATED.

8

Oddly enough, I try to be consistent across different threads :wink:

But no, it’s not related. We literally discussed and decided when we came up with PyUnstable that we wouldn’t automatically go through and rename names that already used the other convention. You should remember, you were driving that whole thing. _PyInterpreterFrame existed before PyUnstable, therefore it’s in that category of “didn’t get renamed even if we would’ve chosen PyUnstable at the time” and not “we need to change this from private to unstable”, which means it can be implicitly unstable without needing the churn of a rename.

9

Oh, nice to hear that. We will definitely try this. Do you know why they are being exposed? Would it be possible to expose a few more APIs as well?

It was to make it possible to implement a PEP 523 interpreter replacement w/o borrowing CPython’s code. It’s possible to use CPython’s interpreter generator and plug in overrides for bytecodes to get your own interpreter loop generated but with the default behavior of CPython. But the interpreter was using functionality which wasn’t exported so if you wanted to do that you had to copy these functions like PyTorch does. It’s still all hidden behind Py_BUILD_CORE though.

It’s definitely possible to expose some more of these if necessary but right now everything that’s required to implement an interpreter is exported so it’s possible what you need is already there (and there’s a test to validate it - https://github.com/python/cpython/commit/4d5a676aa0811563ea78ae58ef89cdc0295bf7ed). Did you have something specific in mind? The ideal outcome here would be to get PyTorch away from needing to define Py_BUILD_CORE at all. I wish we could do it for Cinder some day but I’m guessing we reach into the internals a lot more than PyTorch is. If we want to export existing things as PyUnstable that’s possible too but might involve a small amount of more discussion.

1 Like
10

That is the opposite of what the accepted PEP says.

11

I didn’t actually realise it was documented (since the C-API ToC doesn’t include the page :wink: ), but okay.

It’s not the exact opposite of what it says, since this wouldn’t be automatic, but if you’re prepared to run the deprecation process or (preferably and) rename it with an alias that doesn’t cause compilers to complain about passing different types to the existing functions for people who haven’t updated their sources, then sure.

12

Did you have something specific in mind?

Thanks for the tip, Dino. We just tried and I think almost all code copied from CPython can be replaced with the newly exported symbols from Python 3.15.

Also, I submitted this proposal to CPython in gh-156133: Add `PyUnstable_InterpreterFrame_GetLocals` by guilhermeleobas · Pull Request #156134 · python/cpython · GitHub