Skip to content

bpo-33058: Enhanced COUNT_ALLOCS to be ABI Compatible#6091

Closed
eduardo-elizondo wants to merge 3 commits into
python:masterfrom
eduardo-elizondo:master
Closed

bpo-33058: Enhanced COUNT_ALLOCS to be ABI Compatible#6091
eduardo-elizondo wants to merge 3 commits into
python:masterfrom
eduardo-elizondo:master

Conversation

@eduardo-elizondo

@eduardo-elizondo eduardo-elizondo commented Mar 12, 2018

Copy link
Copy Markdown
Contributor

This modifies the COUNT_ALLOCS build to be ABI Compatible. This means that there's no need to rebuild all extensions to be able to use this feature.

To make this ABI Compatible, I had to pull out the extra members from PyTypeObject and create another object - called "PyTypeObjectExt". Now, on every alloc/free, given a PyTypeObject we find/create the correct PyTypeObejctExt.

The only drawback is that on every alloc/free increase, this will now do an O(n) linked list traversal where n is the total number of different types seen so far. However, COUNT_ALLOCS is not meant to be a production build, thus the runtime perf hit won't matter as much - granted that this will allow us to gather object allocation data.

Furthermore, we don't expect the memory footprint to be that big as well, as this only allocates an object in memory once per every type.

Note: This also gets rid of a couple of places with extra bookkeeping as this doesn't modify the ref counts of the object themselves.

To test:
test.py:

import sys
import pprint
pprint.pprint(counts)

// On a vanilla cpython:

make EXTRA_CFLAGS=-DCOUNT_ALLOCS
./python test.py > a.diff
git patch change.diff
make EXTRA_CFLAGS=-DCOUNT_ALLOCS
./python test.py > b.diff
diff a.diff b.diff

https://bugs.python.org/issue33058

@eduardo-elizondo

Copy link
Copy Markdown
Contributor Author

Once accepted, I will add the News.d change.

@pppery

pppery commented Mar 14, 2018

Copy link
Copy Markdown

Doesn't this cause a change in behavior for code that modifies a type's __name__ attribute at runtime?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants