bpo-45947: Place dict and values pointer at fixed (negative) offset just before GC header.#29879

markshannon · 2021-12-01T14:04:54Z

Produces a solid 1% speedup
See faster-cpython/ideas#80 for full discussion.

https://bugs.python.org/issue45947

…s and plain Python objects. Work in progress.

…ters.

… classes.

bedevere-bot · 2021-12-02T11:13:10Z

🤖 New build scheduled with the buildbot fleet by @markshannon for commit 79e61bf 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

bedevere-bot · 2021-12-02T18:16:44Z

🤖 New build scheduled with the buildbot fleet by @markshannon for commit ce0f65b 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

tiran · 2021-12-02T22:25:34Z

Include/cpython/objimpl.h

 # define_PyGC_FINALIZED(o) PyObject_GC_IsFinalized(o)
 #endif

-PyAPI_FUNC(PyObject*) _PyObject_GC_Malloc(size_tsize);


_PyObject_GC_Malloc is part of stable ABI. AFAIK the function cannot be removed.

It's not part of the stable ABI. It starts with an underscore.
https://www.python.org/dev/peps/pep-0384/#excluded-functions

Misc/stable_abi.txt define it as stable ABI.
function _PyObject_GC_Malloc added 3.2 abi_only
On the other hand, no public macro in Python/C API use it. So I doubt it is actually stable abi.
As far as this repo, only one package in top4000 packages uses it.
https://github.com/hpyproject/top4000-pypi-packages/search?q=_PyObject_GC_Malloc

I sent an email to python-dev asking for clarification of the status of this functions and others that start with _ but are listed in Misc/stable_abi.txt. (It is not listed in Doc/data/stable_abi.dat.)

Include/object.h

gvanrossum

Here's a first pass at a review, mostly nits. I think I'm following everything, though I'm not necessarily fully aware of the whole big picture.

gvanrossum · 2021-12-06T19:30:54Z

Include/cpython/objimpl.h

 # define_PyGC_FINALIZED(o) PyObject_GC_IsFinalized(o)
 #endif

-PyAPI_FUNC(PyObject*) _PyObject_GC_Malloc(size_tsize);


I sent an email to python-dev asking for clarification of the status of this functions and others that start with _ but are listed in Misc/stable_abi.txt. (It is not listed in Doc/data/stable_abi.dat.)

Modules/_testcapimodule.c

gvanrossum · 2021-12-06T19:47:32Z

Modules/gcmodule.c

+_PyObject_GC_Link(PyObject*op)
+{
+PyGC_Head*g=AS_GC(op);
+assert(((uintptr_t)g& (sizeof(uintptr_t)-1)) ==0); // g must be correctly aligned


Wow, you can use & on pointers. I didn't know that. :-)

Only after you've cast it to an int. You can do anything with a cast 🙂
NULL is usually just 0 cast to a pointer, after all.

Oh, somehow I thought uintptr_t was a pointer. :-/

Modules/gcmodule.c

Objects/object.c

Python/ceval.c

gvanrossum · 2021-12-06T21:15:53Z

Tools/gdb/libpython.py

+Py_TPFLAGS_MANAGED_DICT= (1<<4)
 Py_TPFLAGS_HEAPTYPE= (1<<9)


Please realign these like the rest?

gvanrossum · 2021-12-06T23:48:24Z

Python/specialize.c

+else{
+if (!PyDict_CheckExact(dict)){


I'm guessing that if we have a class of objects and only some of those have a materialized __dict__ that will wreak havoc with our specialization, right? Because it will flop-flop as it encounters instances with and without __dict__. In this case we have to hope that the vast majority don't have a __dict__.

Yes.
The specialized case is so much faster though, that it might not be any worse than not specializing at all, even in that case.

Python/ceval.c

gvanrossum

LGTM.

python/cpython#29879 changed the layout of user classes, and the code is no longer able to find the location of __dict__. Do a quick fix for now, as we don't support pretty-printing user defined classes.

markshannon added 12 commits August 5, 2021 12:06

Remove unsafe _PyObject_GC_Calloc function.
ee59772

Place __dict__ immediately before GC header for variable sized object…
72b71cc
…s and plain Python objects. Work in progress.

Restore documented behavior of tp_dictoffset.
cd22dac

Merge branch 'main' into regular-dict-placement
4c83f77

Fix up lazy dict creation logic to use managed dict pointers.
8a8593c

Manage values pointer, placing them directly before managed dict poin…
34b5cea
…ters.

Refactor a bit.
e8c74ab

Fix specialization of managed values.
1bf13b0

Convert hint-based load/store attr specialization target managed dict…
a025dfb
… classes.

Specialize LOAD_METHOD for managed dict objects.
5a012a8

Merge branch 'main' into regular-dict-placement
e7734b8

Use newer API internally.
14d41ab

markshannon requested review from methane and pablogsal as code owners December 1, 2021 14:04

the-knights-who-say-ni added the CLA signed label Dec 1, 2021

bedevere-bot added the awaiting core review label Dec 1, 2021

markshannon added 2 commits December 1, 2021 14:06

Add NEWS.
123171a

Use inline functions instead of magic constants.
48d6a58

markshannon requested a review from a team as a code owner December 1, 2021 16:59

Remove unsafe _PyObject_GC_Malloc() function.
79e61bf

markshannon force-pushed the regular-dict-placement branch from a76861c to 79e61bfCompare December 1, 2021 17:04

markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 2, 2021

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 2, 2021

Remove invalid assert.
ce0f65b

markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 2, 2021

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 2, 2021

tiran reviewed Dec 2, 2021
View reviewed changes

Add comment explaning use of Py_TPFLAGS_MANAGED_DICT.
98ddaed

gvanrossum self-requested a review December 6, 2021 18:10

gvanrossum approved these changes Dec 6, 2021
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Dec 6, 2021

markshannon added 4 commits December 7, 2021 11:55

Use inline function, not magic constant.
0f376b5

Tidy up struct layout a bit.
d724812

Tidy up gdb/libpython.py.
9435bae

Fix whitespace.
302f46f

gvanrossum approved these changes Dec 7, 2021
View reviewed changes

markshannon merged commit 8319114 into python:mainDec 7, 2021

markshannon deleted the regular-dict-placement branch December 7, 2021 16:02

bedevere-bot removed the awaiting merge label Dec 7, 2021

corona10 mentioned this pull request Feb 9, 2023
gh-101430: Update tracemalloc to handle presize properly. #101745
Merged

jackyk02 mentioned this pull request Oct 21, 2023
Fix Segfault for Python 3.11 lf-lang/reactor-c#293
Merged

jackyk02 mentioned this pull request Nov 9, 2023
[Python 3.11] Fix Segfault lf-lang/reactor-c#304
Closed

malor mentioned this pull request Nov 17, 2024
cpython3.11: fix pretty-printing of UserDict, UserString, UserList malor/cpython-lldb#74
Merged

		else{
		if (!PyDict_CheckExact(dict)){

Uh oh!

bpo-45947: Place dict and values pointer at fixed (negative) offset just before GC header.#29879

bpo-45947: Place dict and values pointer at fixed (negative) offset just before GC header. #29879

Uh oh!

Conversation

markshannon commented Dec 1, 2021• edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Dec 2, 2021

Uh oh!

bedevere-bot commented Dec 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

markshannon commented Dec 1, 2021•
edited by bedevere-bot
Loading