[issue28959] Add a macro for dict size

2016-12-16 Thread Serhiy Storchaka
Serhiy Storchaka added the comment: After further analysis I conclude that instance dict can be only NULL or a dict. Therefore PyDict_Size() is correctly used in typeobject.c and _datetimemodule.c. It never fails, and additional PyDict_Check() is not needed. Committed changes don't include swi

[issue28959] Add a macro for dict size

2016-12-16 Thread Roundup Robot
Roundup Robot added the comment: New changeset dbf72357cb4a by Serhiy Storchaka in branch 'default': Issue #28959: Added private macro PyDict_GET_SIZE for retrieving the size of dict. https://hg.python.org/cpython/rev/dbf72357cb4a -- nosy: +python-dev __

[issue28959] Add a macro for dict size

2016-12-14 Thread STINNER Victor
STINNER Victor added the comment: Ignore my previous comment, PyDict_GET_SIZE-3.patch LGTM. > I don't think the changes in _datetimemodule.c and typeobject.c change the > behavior. Oh wait, it seems like I misunderstood the new code. Modules/_datetimemodule.c: dictptr = _PyObject_G

[issue28959] Add a macro for dict size

2016-12-14 Thread Serhiy Storchaka
Serhiy Storchaka added the comment: I don't think the changes in _datetimemodule.c and typeobject.c change the behavior. -- ___ Python tracker ___ __

[issue28959] Add a macro for dict size

2016-12-14 Thread STINNER Victor
STINNER Victor added the comment: PyDict_GET_SIZE-3.patch: Except of the changes in _datetimemodule.c and typeobject.c on *dictptr type, the patch LGTM. I suggest to open a new issue for these two specific changes, since it changes the behaviour and it's a weird corner case unrelated to this i

[issue28959] Add a macro for dict size

2016-12-14 Thread STINNER Victor
STINNER Victor added the comment: I wrote a short script to check the size of dict: --- import sys def size(obj): print(sys.getsizeof(obj)) size({}) size({i:i for i in range(3)}) size({i:i for i in range(10)}) size({i:i for i in range(100)}) --- On Linux x86_64, the sizes don't change with

[issue28959] Add a macro for dict size

2016-12-14 Thread Serhiy Storchaka
Serhiy Storchaka added the comment: Updated patch addresses Victor's issue. _PyDict_GET_SIZE is renamed to PyDict_GET_SIZE and now it includes an assertion. This is good argument for introducing this macro against using PyDict_Size (without checking the result for error) and Py_SIZE (which doe

[issue28959] Add a macro for dict size

2016-12-14 Thread STINNER Victor
STINNER Victor added the comment: > 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 en

[issue28959] Add a macro for dict size

2016-12-14 Thread STINNER Victor
STINNER Victor added the comment: > Should we use Py_SIZE or _PyDict_GET_SIZE? I prefer _PyDict_GET_SIZE() just to make the code more readable: it helps to repeat the type of variables. Moreover, it can give you a free check on the type if you agree my suggestion to add an assertion into the

[issue28959] Add a macro for dict size

2016-12-14 Thread STINNER Victor
STINNER Victor added the comment: > It looks like the struct of a dict is already arranged in a way that it could > switch to PyObject_VAR_HEAD Can someone check if it has an impact of the size of the structure (because of the complex rules of alignment)? If the structure has the same size, I'

[issue28959] Add a macro for dict size

2016-12-13 Thread Serhiy Storchaka
Serhiy Storchaka added the comment: list doesn't end with array too, but uses PyObject_VAR_HEAD. tp_itemsize is 0, therefore no additional space at the end is allocated. Maybe this is overuse of PyObject_VAR_HEAD. -- ___ Python tracker

[issue28959] Add a macro for dict size

2016-12-13 Thread INADA Naoki
INADA Naoki added the comment: I didn't know about PyObject_VAR_HEAD much. The comment of the macro says: /* PyObject_VAR_HEAD defines the initial segment of all variable-size * container objects. These end with a declaration of an array with 1 * element, but enough space is malloc'ed so that

[issue28959] Add a macro for dict size

2016-12-13 Thread Serhiy Storchaka
Serhiy Storchaka added the comment: Following patch also switches the struct of a dict to PyObject_VAR_HEAD. Should we use Py_SIZE or _PyDict_GET_SIZE? It is easier to replace _PyDict_GET_SIZE with Py_SIZE than restore dict-specific name from some of Py_SIZE-s. -- Added file: http://b

[issue28959] Add a macro for dict size

2016-12-13 Thread Raymond Hettinger
Raymond Hettinger added the comment: It looks like the struct of a dict is already arranged in a way that it could switch to PyObject_VAR_HEAD which would let you use Py_SIZE everywhere. -- nosy: +rhettinger ___ Python tracker

[issue28959] Add a macro for dict size

2016-12-13 Thread Serhiy Storchaka
New submission from Serhiy Storchaka: In additional to C API functions PyTuple_Size() and PyList_Size() there are fast macros PyTuple_GET_SIZE() and PyList_GET_SIZE() for getting the size of a tuple or a list. But for dicts there is just PyDict_Size(). It is not just slower than a macro, but c