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

Support the use of the managed pre-header in builtin classes. #95707

Open
2 of 3 tasks
markshannon opened this issue Aug 5, 2022 · 2 comments
Open
2 of 3 tasks

Support the use of the managed pre-header in builtin classes. #95707

markshannon opened this issue Aug 5, 2022 · 2 comments
Assignees
Labels
3.12 interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage type-feature A feature request or enhancement

Comments

@markshannon
Copy link
Member

markshannon commented Aug 5, 2022

Currently Py_TPFLAGS_MANAGED_DICT is an internal-only flag, in fact setting in third-party code is likely to lead to a crash.
We would like to expose it, and a weakref equivalent Py_TPFLAGS_MANAGED_WEAKREFS to allow builtin classes to take advantage of compact object layout.
Compact layout uses less memory, performs better and allow more robust subclassing. So everyone should be able to use it.

  • Fix crashes when Py_TPFLAGS_MANAGED_DICT is used
  • Add Py_TPFLAGS_MANAGED_WEAKREFS
  • Document how to use these flags, and the upgrade path from tp_dictoffset and tp_weakreflist
@markshannon markshannon added type-feature A feature request or enhancement performance Performance or resource usage interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.12 labels Aug 5, 2022
@markshannon markshannon self-assigned this Aug 5, 2022
markshannon added a commit that referenced this issue Aug 15, 2022
* Make sure that tp_dictoffset is correct with Py_TPFLAGS_MANAGED_DICT is set.

* Avoid traversing managed dict twice when subclassing class with Py_TPFLAGS_MANAGED_DICT set.
@tiran
Copy link
Member

tiran commented Aug 15, 2022

The changeset broke wasm32-emscripten builds.

$ ./Tools/wasm/wasm_build.py emscripten-node-dl-debug
$ cd builddir/emscripten-node-dl-debug/
$ $ make test TESTOPTS="test_capi"
...
0:00:03 load avg: 3.51 [1/1/1] test_capi crashed (Exit code 1)
warning: unsupported syscall: __syscall_prlimit64

exiting due to exception: RuntimeError: null function or function signature mismatch,RuntimeError: null function or function signature mismatch
    at gc_collect_main (wasm://wasm/02f6208e:wasm-function[4421]:0x321118)
    at _PyObject_GC_Link (wasm://wasm/02f6208e:wasm-function[4431]:0x321c49)
    at _PyObject_GC_New (wasm://wasm/02f6208e:wasm-function[4432]:0x321d0b)
    at wrapperdescr_get (wasm://wasm/02f6208e:wasm-function[1036]:0x1aeaa2)
    at super_getattro (wasm://wasm/02f6208e:wasm-function[2415]:0x2157fc)
    at PyObject_GetAttr (wasm://wasm/02f6208e:wasm-function[2060]:0x1fbb08)
    at _PyEval_EvalFrameDefault (wasm://wasm/02f6208e:wasm-function[3281]:0x2a7e37)
    at _PyEval_Vector (wasm://wasm/02f6208e:wasm-function[3282]:0x2a9185)
    at _PyFunction_Vectorcall (wasm://wasm/02f6208e:wasm-function[870]:0x1a2a57)
    at _PyObject_FastCallDictTstate (wasm://wasm/02f6208e:wasm-function[857]:0x1a1f22)

== Tests result: FAILURE ==

@tiran
Copy link
Member

tiran commented Aug 15, 2022

The clear function signature is incorrect. The PR defines it as static void heapmanaged_clear but it must be static int heapmanaged_clear.

I'm also getting warnings on X86_64:

comparison of integer expressions of different signedness: ‘Py_ssize_t’ {aka ‘long int’} and ‘long unsigned int’ [-Wsign-compare]

tiran added a commit to tiran/cpython that referenced this issue Aug 15, 2022
tiran added a commit to tiran/cpython that referenced this issue Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants