classification
Title: PyType_GenericAlloc might over-allocate memory
Type: performance Stage: patch review
Components: Interpreter Core Versions:
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: nascheme
Priority: low Keywords:

Created on 2019-06-07 19:32 by nascheme, last changed 2019-06-07 23:11 by nascheme.

Files
File name Uploaded Description Edit
generic_alloc_sentinel.txt nascheme, 2019-06-07 19:32
generic_alloc_sentinel_v2.txt nascheme, 2019-06-07 23:11
Messages (2)
msg345002 - (view) Author: Neil Schemenauer (nascheme) * (Python committer) Date: 2019-06-07 19:32
In the process of working on some garbage collector/obmalloc experiments, I noticed what seems to be a quirk about PyType_GenericAlloc().  It calls:

size = _PyObject_VAR_SIZE(type, nitems+1);

Note the "+1" which is documented as "for the sentinel".  That code dates back to change "e5c691abe3946ddbaa00730b92f3b96f96903f7d" when Guido added support for heap types.  This extra item is not added by _PyObject_GC_NewVar().  Also, the documentation for tp_alloc says that the size of the allocated block should be:

  tp_basicsize + nitems*tp_itemsize, rounded up to a multiple of sizeof(void*);

The "+1" for the sentinel is definitely needed in certain cases.  I think it might only be needed if 'type' is a subtype of 'type'.  I.e. if Py_TPFLAGS_TYPE_SUBCLASS is set on 'type'.

I haven't done enough analysis to fully understand this quirk yet but I think we are allocating extra memory quite regularly.  Quite a lot of types use tp_alloc = PyType_GenericAlloc.  E.g. the 'list' type or a subclass of the tuple type.

It seems with the attached patch, unit tests still pass.  Perhaps the +1 could be removed on the non-GC branch of the code as well.
msg345014 - (view) Author: Neil Schemenauer (nascheme) * (Python committer) Date: 2019-06-07 23:11
Updated patch is attached.  It appears that the extra item is only needed if Py_TPFLAGS_TYPE_SUBCLASS set.  In all other cases, it seems we don't need the extra space for the sentinel.  At least, the unit tests pass with this change.  It could be that some extension module depends on this extra allocated spaced.

The number of types affected by this change seem relatively small.  The 'list' type is not affected because tp_itemsize is 0.  I did a little test by running some unit tests, here are some type names that have a smaller amount of memory allocated for them:

   _NamedIntConstant
   CodecInfo
   Point
   TestResults
   madtuple
History
Date User Action Args
2019-06-07 23:11:45naschemesetpriority: normal -> low
files: + generic_alloc_sentinel_v2.txt
messages: + msg345014

stage: patch review
2019-06-07 19:32:16naschemecreate