bpo-45340: Don't create object dictionaries unless actually needed by markshannon · Pull Request #28802 · python/cpython · GitHub

A "normal" Python objects is conceptually just a pair of pointers, one to the class, and one to the dictionary.

With shared keys, the dictionary is redundant as it is no more than a pair of pointers, one to the keys and one to the values.

By adding a pointer to the values to the object and fetching the keys via the class, we can avoid creating a dictionary for many objects.

See faster-cpython/ideas#72 for more details.

About 1% faster, which is nice, but the main benefit is the reduced memory consumption.

https://bugs.python.org/issue45340

… keys more freely.

… objects.

…e (yet)

Needs NEWS and some additional documentation.

Some comments about LOAD_METHOD.

Does this reduce the specialization rates by much?

I didn't check. Overall performance is up a bit, so that's good enough for this PR 🙂

We will need to recheck the specialization rates and kinds of failures for LOAD_ATTR, STORE_ATTR and LOAD_METHOD. From my superficial scan, it appears that the kinds of failures are more clustered, which bodes well.

But changing all that in this PR would just confuse matters.

Wait is this due to the new scheme that objects shouldn't have dict unless really required?

Yes, it is. We assume that having no __dict__ is the most common case, so it isn't worth bothering to check for the shared key dict case.

Out of curiosity: is there a reason why we don't need to do this immediately anymore? Are the new dictionary operations atomic/don't call any _PyType_Lookup?

We used to call _Py_dict_lookup, which could do anything, now we call _PyDictKeys_StringLookup which cannot.

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

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

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

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

The buildbot failures are:

buildbot/AMD64 Windows10 PR: Not enough memory on that machine. Lots of MemoryErrors.
buildbot/s390x RHEL7 LTO PR: Segfault due to stack overflow. Pre-existing issue (although one that needs fixing if we are going to support s390)
buildbot/x86 Gentoo Installed with X PR: Tk issue
buildbot/x86 Gentoo Non-Debug with X PR: Tk issue

Benchmark of final version: 1% faster

@markshannon I'm somewhat confused why anything with a dict pointer is now an attribute, even without looking into said dict. Is there a reason elsewhere (or is this so that we don't need another opcode for the other dict form)?

Anything will a dict pointer will be a miss for LOAD_INSTANCE_VALUE, so I'm assuming it isn't worth optimizing.
Maybe we should attempt to optimize this using LOAD_ATTR_WITH_HINT at this point?

This code seems to be for LOAD_METHOD_CACHED. I'll try to play around with it and see if the dictionary matters (versus LOAD_ATTR_INSTANCE_VALUE/LOAD_ATTR_WITH_HINT and such.

markshannon added 17 commits September 22, 2021 14:54

Add insertion order bit-vector to dict values to allow dicts to share…

8214006

… keys more freely.

Add NEWS

45c9814

Never change types' cached keys. It could invalidate inline attribute…

c3de09e

… objects.

Lazily create object dictionaries. Work in progress.

58dd2a0

Restore specialization for inline attributes.

769183a

Turn on stats

1e6cd1f

Fix specialization of LOAD/STORE_ATTR.

a8fbb34

Turn off stats.

37d8cc6

Don't update shared keys version for deletion of value.

0e022f9

Merge main into inline-attributes-with-opt

ddd2e2c

Fix refrence leak.

e979575

Change 'InlineAttribute' to 'InstanceAttribute' as they are not inlin…

42361ad

…e (yet)

Merge branch 'main' into inline-attributes-with-opt

e9b09d5

Update gdb support to handle instance values.

70dc34f

Rename SPLIT_KEYS opcodes to INSTANCE_VALUE.

022f2c5

Tidy test_gc

4ec5f11

Don't lose errors.

1818a5a

markshannon requested a review from methane as a code owner October 7, 2021 14:31

bedevere-bot added the awaiting core review label Oct 7, 2021

the-knights-who-say-ni added the CLA signed label Oct 7, 2021

markshannon added 3 commits October 8, 2021 00:25

Make sure order is updated when deleting from values array.

bfc7be0

Update test_dict.

62a3a59

Add NEWS

7978220

Fidget-Spinner reviewed Oct 8, 2021

View reviewed changes

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

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

methane reviewed Oct 12, 2021

View reviewed changes

Comment thread Include/cpython/object.h Outdated

Merge branch 'main' into inline-attributes-with-opt

b25b1d9

markshannon added 4 commits October 12, 2021 09:45

Merge branch 'main' into inline-attributes-with-opt

7b2efa4

Make sure that instance dicts are GC tracked when necessary.

459e29e

Support older versions of Python in gdb.

83b90c5

Rename function and move to internal header.

8c894f3

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

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

markshannon merged commit a8b9350 into python:main Oct 13, 2021

bedevere-bot removed the awaiting core review label Oct 13, 2021

Fidget-Spinner reviewed Oct 22, 2021

View reviewed changes

markshannon mentioned this pull request Dec 2, 2021

Compact layout of Plain Python Objects faster-cpython/ideas#72

Closed

geryogam mentioned this pull request May 4, 2022

bpo-43857: Improve the AttributeError message when deleting a missing attribute #25424

Merged

markshannon mentioned this pull request Nov 29, 2022

Lazily create dictionaries for plain Python objects #89503

Closed

gertjanvanzwieten mentioned this pull request Dec 2, 2022

serialized evalf evalf/nutils#734

Merged

markshannon deleted the inline-attributes-with-opt branch September 26, 2023 12:47

markshannon mentioned this pull request Feb 21, 2024

Use static analysis to assist inlining values and compact object layout. #115775

Open

+                          assert(self_cls->tp_dictoffset > 0);
+                          assert(self_cls->tp_inline_values_offset > 0);
+                          PyDictObject *dict = *(PyDictObject **)(((char *)self) + self_cls->tp_dictoffset);
+                          DEOPT_IF(dict != NULL, LOAD_METHOD);

                   PyObject *descr = NULL;
                   DesciptorClassification kind = 0;
                   kind = analyze_descriptor(owner_cls, name, &descr, 0);
-                  // Store the version right away, in case it's modified halfway through.

+                  if (owner_cls->tp_inline_values_offset) {
+                      PyObject **owner_dictptr = _PyObject_DictPointer(owner);
+                      assert(owner_dictptr);
+                      if (*owner_dictptr) {

+                      if (*owner_dictptr) {
+                          SPECIALIZATION_FAIL(LOAD_METHOD, SPEC_FAIL_IS_ATTR);
                           goto fail;

bpo-45340: Don't create object dictionaries unless actually needed by markshannon · Pull Request #28802 · python/cpython · GitHub

Uh oh!

Uh oh!

bpo-45340: Don't create object dictionaries unless actually needed - #28802

bpo-45340: Don't create object dictionaries unless actually needed#28802markshannon merged 25 commits intopython:mainpython/cpython:mainfrom faster-cpython:inline-attributes-with-optfaster-cpython/cpython:inline-attributes-with-optCopy head branch name to clipboard

Conversation

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

Uh oh!

Uh oh!

markshannon commented Oct 7, 2021

Uh oh!

Fidget-Spinner left a comment

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!

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!

bedevere-bot commented Oct 9, 2021

Uh oh!

Uh oh!

bedevere-bot commented Oct 12, 2021

Uh oh!

markshannon commented Oct 13, 2021

Uh oh!

markshannon commented Oct 13, 2021

Uh oh!

Fidget-Spinner Oct 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bpo-45340: Don't create object dictionaries unless actually needed#28802
markshannon merged 25 commits into
python:mainfrom
faster-cpython:inline-attributes-with-opt

markshannon commented Oct 7, 2021 •
edited by bedevere-bot
Loading

Fidget-Spinner Oct 22, 2021 •
edited
Loading