This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Switch dict and set structures to PyVarObject
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.7
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: methane, rhettinger, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2016-12-16 15:01 by serhiy.storchaka, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
dict-set-var-object.patch serhiy.storchaka, 2016-12-16 15:01 review
Messages (7)
msg283404 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-16 15:01
In issue28959 Raymond said that a dict structure can be switched to PyVarObject. Victor guessed that it makes sense to switch a set structure too. Proposed patch implements this. The layout of a dict structure is not changed. The number of used and filled entries in a set structure are swapped, therefore extensions that use PySet_GET_SIZE or access fields of PySetObject directly (both are not in a stable API) should be recompiled.

I don't see a benefit from this patch except some unification.
msg283405 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-12-16 15:07
Copy/paste of my msg283176:

> dict doesn't end with array.

Right, but...

Recently I looked at dict internals. As a newcomer, I have to confess that it's currently really hard to understand the purpose of each dict field: they are many "sizes": size of the hash table, number of usable entries, number of used entries, number of items in the dictionary, etc.

I like the idea of using the standard ob_size field (PyVarObject) to make it more explicitl that this field is the expected result of len(obj).

Technically, I don't know any function inside Python core which rely on the fact that object data is at the end of the main memory block.

Other builtin Python types:

* tuple: PyVarObject
* list: PyVarObject
* bytes: PyVarObject
* bytearray: PyVarObject
* memoryview: PyVarObject
* set: "used" field
* str: "length" field

The str type is super optimized for memory footprint, there are technical reasons for not used PyVarObject here, like backward compatibility.

It may make sense to modify PySetObject to use PyVarObject as well, but that's a different topic :-)
msg283679 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-12-20 11:04
LGTM
msg283719 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-12-21 05:38
Please leave the set object patch for me.  I would like to do it a bit differently.
msg283733 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-12-21 09:57
Raymond Hettinger:
> Please leave the set object patch for me.  I would like to do it a bit differently.

Differently? How?
msg283735 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-21 10:07
Since I don't see a benefit from this change I leave both patches to you Raymond.
msg350175 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2019-08-22 09:17
> Since I don't see a benefit from this change I leave both patches
> to you Raymond.

After more reflection, I also don't see any point of doing this.  While there is a minor whiff of consistency when using Py_SIZE(), it makes the assignment look weird and inconsistent between filled and used.  I now favor local consistency and clarity (with in setobject.c) over trying to make it look exactly like some other modules.
History
Date User Action Args
2022-04-11 14:58:40adminsetgithub: 73174
2019-08-22 09:17:24rhettingersetstatus: open -> closed
resolution: rejected
messages: + msg350175

stage: patch review -> resolved
2016-12-21 10:07:30serhiy.storchakasetmessages: + msg283735
2016-12-21 09:57:41vstinnersetmessages: + msg283733
2016-12-21 05:38:51rhettingersetmessages: + msg283719
2016-12-20 11:04:20methanesetnosy: + methane
messages: + msg283679
2016-12-16 15:07:22vstinnersetmessages: + msg283405
2016-12-16 15:01:05serhiy.storchakacreate