Allow extensions to set a callback to be invoked when a type is modified

[mpage]

BPO	46895
Nosy	@carljm, @DinoV, @itamaro, @mpage

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2022-03-01.22:16:51.046>
labels = ['interpreter-core', 'expert-C-API', 'type-feature', '3.11']
title = 'Allow extensions to set a callback to be invoked when a type is modified'
updated_at = <Date 2022-03-01.22:21:58.988>
user = 'https://github.com/mpage'

bugs.python.org fields:

activity = <Date 2022-03-01.22:21:58.988>
actor = 'mpage'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core', 'C API']
creation = <Date 2022-03-01.22:16:51.046>
creator = 'mpage'
dependencies = []
files = []
hgrepos = []
issue_num = 46895
keywords = []
message_count = 1.0
messages = ['414305']
nosy_count = 4.0
nosy_names = ['carljm', 'dino.viehland', 'itamaro', 'mpage']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue46895'
versions = ['Python 3.11']

Linked PRs

• gh-91051: fix segfault when using all 8 type watchers #107853
• [3.12] gh-91051: fix segfault when using all 8 type watchers (GH-107853) #107876
• gh-91051: fix type watcher test to be robust to existing watcher #107989
• [3.12] gh-91051: fix type watcher test to be robust to existing watcher (GH-107989) #108053

[mpage]

CPython extensions providing optimized execution of Python bytecode (e.g. the Cinder JIT), or even CPython itself (e.g. the faster-cpython project) may wish to cache access to lookups in the class hierarchy (e.g. when resolving the target of a method call). Extensions that perform these optimizations need to know when to invalidate the cached values. CPython already has a mechanism to invalidate its internal state (e.g. the global method cache) when a type is modified: _PyType_Modified. We propose adding an API to allow extensions to set a callback that will be invoked by _PyType_Modified whenever a type, or any ancestor of the type in the class hierarchy, changes.

Proposed API:

// A callback to be invoked with the modified type and optionally the name of
// the attribute that was modified.
typedef void(*PyType_ModifiedCallback)(PyTypeObject* type, PyObject* attr);

// Set or get the callback. The callback may be cleared by supplying a NULL callback.
void PyType_SetModifiedCallback(PyType_ModifiedCallback callback);
PyType_ModifiedCallback PyType_GetModifiedCallback();

[gvanrossum]

I don't see any discussion here. Are you still interested in getting this in 3.12? What should happen next? Is it a simple PR, or should we have a design discussion first?

[itamaro]

Are you still interested in getting this in 3.12?

yes, that's the plan!

What should happen next? Is it a simple PR, or should we have a design discussion first?

I think this is a simple PR (similar to #91054), so the next step is to write that PR if the proposed API looks reasonable

[gvanrossum]

I guess this would go into Include/object.h, where PyType_Modified lives (it has no leading underscore BTW).

It would be interesting to see how frequently this callback would be called -- I'm a little more worried about the cost than for #91054.

Before we go for this, where would the attr argument come from? PyType_Modified doesn't take that, so presumably we would have to add a new version, e.g. PyType_ModifiedAttr, that takes such an argument, and update the relevant callers. Is there already an API like this in Cinder? Where is it used?

[carljm]

It would be interesting to see how frequently this callback would be called -- I'm a little more worried about the cost than for #91054.

I'll work on a PR and we can see what pyperformance says.

Before we go for this, where would the attr argument come from? PyType_Modified doesn't take that, so presumably we would have to add a new version, e.g. PyType_ModifiedAttr, that takes such an argument, and update the relevant callers. Is there already an API like this in Cinder? Where is it used?

Cinder currently just updates PyType_Modified to call into our JIT, and we don't take or use the attr argument. It was more of a future-proofing idea, that someday we (or someone) may want to do more fine-grained invalidation of e.g. inline caches depending what exactly was changed on the type. And it would be more difficult to add in the future than if we add it from the start. But it's also not critical to us right now.

I think it's not hard to do this while keeping PyType_Modified as-is. Like you say, we just need to add a new (probably internal-only) function for internals to report type changes to, and that new function can call the user-provided callback and also call the existing PyType_Modified.

I'll work on a PR so we have something concrete to discuss.

[gvanrossum]

Okay, I'm still a little skeptical -- there are a lot of places where PyType_Modified is called, and they would all be changed, right?

But I'd be happy to wait until there's a PR to look at, it's quite possible that I'm missing something.

[carljm]

@gvanrossum On further thought, I think you were right to be skeptical :) The problem with what I suggested above is that third-party extension code may modify a type and then (if it's well-behaved) call PyType_Modified. We can't magically change all those calls, and we do want them to result in a call to the user-set callback. We could do something where the attr is optional and may or may not be supplied with the callback (and for now would only be supplied in changes happening in core CPython code), but that will make the code quite a bit more complex for unclear benefit. So I went ahead and submitted a PR without attr, which makes it pretty simple and straightforward.

[gvanrossum]

Earlier I wrote:

It would be interesting to see how frequently this callback would be called -- I'm a little more worried about the cost than for #91054.

I'm still a tad worried about this. I would guess that the pyperformance benchmarks don't trigger this much, but it does seem it's called whenever any class attribute is modified, including on any successful type_setattro() calls (unless I am misreading the code).

[carljm]

it does seem it's called whenever any class attribute is modified

Yes, this is true. I think if pyperformance is fine, that's a reasonable signal that modifying type attributes is very rarely done at all in a hot path (which is what I would think anyway.) Also, the overhead here if no callback is set is not that much.

[gvanrossum]

The only case I could think of would be if someone keeps statistics in a class variable. I’ve done that. But I’m not too worried it’ll be noticeable.

[carljm]

Fixed in #91051.

[serhiy-storchaka]

Reopened because not all PRs were merged yet.