X Tutup
The Wayback Machine - https://web.archive.org/web/20220314204203/https://github.com/python/cpython/pull/31787
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-46896: Add C API for watching dictionaries #31787

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

carljm
Copy link
Contributor

@carljm carljm commented Mar 9, 2022

Notes on the PR for reviewers:

  • Since this API is intended for use by extensions, and _Py* APIs are internal-only, I added the API as public (Py*). But I think it should perhaps be CPython-specific, so I put it in Include/cpython/dictobject.h, not Include/dictobject.h. Not sure about any of this, though, so happy to change as requested.
  • The STORE_ATTR_WITH_HINT opcode implementation in ceval.c pokes directly at dict internals. This meant I had to duplicate a bit of code that otherwise would be private to dictobject.c. I think the best option here would be to add higher-level (if private) API in dictobject (something like _PyDict_SetWithHint) for the needs of STORE_ATTR_WITH_HINT so it doesn't have to poke directly at dict internals anymore (though that may be difficult due to the needs of deopt), but that seemed a bit far afield for this PR, so for now I just added the duplication with a comment.
  • Having dict_notify_event return the new dict version reduces the overhead in the unwatched case to a single testb/jne (checking only once if the dict is watched.)
  • Pyperformance data in https://gist.github.com/carljm/987a7032ed851a5fe145524128bdb67a

https://bugs.python.org/issue46896

Objects/dictobject.c Show resolved Hide resolved
Objects/dictobject.c Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
X Tutup