[issue46896] add support for watching writes to selected dictionaries

Carl Meyer added the comment: Thanks for the extended example. I think in order for this example to answer the question I asked, a few more assumptions should be made explicit: 1) Either `spam_var` and/or `eggs_var` are frequently re-bound to new values in a hot code path somewhere. (Given

[issue46896] add support for watching writes to selected dictionaries

Mark Shannon added the comment: Let me give you an example. #module eggs eggs_var = 0 # a variable, maybe a counter or similar EGGS_CONST # a constant #module spam import eggs spam_var # Another variable def foo(): use(eggs.EGGS_CONST) - We will want to treat

[issue46896] add support for watching writes to selected dictionaries

Carl Meyer added the comment: > There should not be much of a slowdown for this code when watching `CONST`: How and when (and based on what data?) would the adaptive interpreter make the decision that for this code sample the key `CONST`, but not the key `var`, should be watched in the

[issue46896] add support for watching writes to selected dictionaries

Mark Shannon added the comment: Another use of this is to add watch points in debuggers. To that end, it would better if the callback were a Python object. The overhead is relatively small if using the vectorcall protocol. If the call overhead matters that much, there is something wrong as

[issue46896] add support for watching writes to selected dictionaries

Mark Shannon added the comment: You might not like global variables, they may not show up much in benchmarks, but people do use them. I suspect a lot of jupyter notebooks have quite a few global variables. There should not be much of a slowdown for this code when watching `CONST`: CONST =

[issue46896] add support for watching writes to selected dictionaries

Carl Meyer added the comment: I've updated the PR to split `PyDict_EVENT_MODIFIED` into separate `PyDict_EVENT_ADDED`, `PyDict_EVENT_MODIFIED`, and `PyDict_EVENT_DELETED` event types. This allows callbacks only interested in e.g. added keys (case #2) to more easily and cheaply skip

[issue46896] add support for watching writes to selected dictionaries

Carl Meyer added the comment: Thanks for outlining the use cases. They make sense. The current PR provides a flexible generic API that fully supports all three of those use cases (use cases 2 and 3 are strict subsets of use case 1.) Since the callback is called before the dict is modified,

[issue46896] add support for watching writes to selected dictionaries

Mark Shannon added the comment: There are three kinds of changes that we might want to watch (that I can think of right now): 1. Any change. Rather coarse and potentially expensive. Used by Cinder. 2. A new key being added (or a change to the keys version as a proxy). Useful for detect

[issue46896] add support for watching writes to selected dictionaries

Carl Meyer added the comment: > have you considered watching dict keys rather than whole dicts? Just realized that I misunderstood this suggestion; you don't mean per-key watching necessarily, you just mean _not_ notifying on dict values changes. Now I understand better how that connects to

[issue46896] add support for watching writes to selected dictionaries

Carl Meyer added the comment: Hi Dennis, thanks for the questions! > A curiosity: have you considered watching dict keys rather than whole dicts? There's a bit of discussion of this above. A core requirement is to avoid any memory overhead and minimize CPU overhead on unwatched dicts.

[issue46896] add support for watching writes to selected dictionaries

Change by Guido van Rossum : -- nosy: +gvanrossum ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue46896] add support for watching writes to selected dictionaries

Dennis Sweeney added the comment: A curiosity: have you considered watching dict keys rather than whole dicts? That way, changing global values would not have to de-optimize, only adding new global keys would. Indexing into dict values array wouldn't be as efficient as embedding direct

[issue46896] add support for watching writes to selected dictionaries

Carl Meyer added the comment: Draft PR is up for consideration. Perf data in https://gist.github.com/carljm/987a7032ed851a5fe145524128bdb67a Overall it seems like the base implementation is perf neutral -- maybe a slight impact on the pickle benchmarks? With all module global dicts

[issue46896] add support for watching writes to selected dictionaries

Change by Carl Meyer : -- keywords: +patch pull_requests: +29891 stage: -> patch review pull_request: https://github.com/python/cpython/pull/31787 ___ Python tracker ___

[issue46896] add support for watching writes to selected dictionaries

Carl Meyer added the comment: Thanks for the feedback! > Why so coarse? Simplicity of implementation is a strong advantage, all else equal :) And the coarse version is a) at least somewhat proven as useful and usable already by Cinder / Cinder JIT, and b) clearly doable without introducing

[issue46896] add support for watching writes to selected dictionaries

Brandt Bucher added the comment: > Why so coarse? > Getting a notification for every change of a global in module, is likely to > make use the use of global variables extremely expensive. Perhaps a compromise is possible here: one global group/chain of callbacks registered for all

[issue46896] add support for watching writes to selected dictionaries

Mark Shannon added the comment: Why so coarse? Getting a notification for every change of a global in module, is likely to make use the use of global variables extremely expensive. ``` var = 0 CONST = 1 def foo(...): ... ``` I may well want to be notified if `foo` or `CONST` gets

[issue46896] add support for watching writes to selected dictionaries

Gregory P. Smith added the comment: Per interpreter seems best. If someone using this feature writes a buggy implementation of a callback that doesn't chain reliably, that is a bug in their code and all of the fallout from that is "just" a bug to be fixed in said code. Think of it like a C

[issue46896] add support for watching writes to selected dictionaries

Carl Meyer added the comment: > Could we (or others) end up with unguarded stale caches if some buggy > extension forgets to chain the calls correctly? Yes. I can really go either way on this. I initially opted for simplicity in the core support at the cost of asking a bit more of clients,

[issue46896] add support for watching writes to selected dictionaries

Brandt Bucher added the comment: Also, when you say "only one global callback": does that mean per-interpreter, or per-process? -- ___ Python tracker ___

[issue46896] add support for watching writes to selected dictionaries

Brandt Bucher added the comment: > CPython will track only one global callback; it is a well-behaved client’s > responsibility to check if a callback is already set when setting a new one, > and daisy-chain to the previous callback if so. Hm, this is a bit scary. Could we (or others) end up

[issue46896] add support for watching writes to selected dictionaries

Change by Gregory P. Smith : -- nosy: +Mark.Shannon ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue46896] add support for watching writes to selected dictionaries

Change by Brandt Bucher : -- nosy: +brandtbucher ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue46896] add support for watching writes to selected dictionaries

Carl Meyer added the comment: Thanks gps! Working on a PR and will collect pyperformance data as well. We haven't observed any issues in Cinder with the callback just being called at shutdown, too, but if there are problems with that it should be possible to just have CPython clear the

[issue46896] add support for watching writes to selected dictionaries

Gregory P. Smith added the comment: At first quick glance, this makes sense and the API looks reasonable. Question: what happens on interpreter shutdown? Shutdown obviously finalized and clears out most all dicts. I guess the C callback simply gets called for each of these? That makes

[issue46896] add support for watching writes to selected dictionaries

Change by Barry A. Warsaw : -- nosy: +barry ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue46896] add support for watching writes to selected dictionaries

Change by Carl Meyer : -- title: add support for watching writes to selecting dictionaries -> add support for watching writes to selected dictionaries ___ Python tracker ___