Open lysnikolaou opened 1 year ago
Hi @lysnikolaou, thank you very much for opening this issue.
I don't have access to a macOS machine, so I will not be able to help with this problem.
However I noticed that you reproducer script uses sys.gettotalrefcount
and
sys.getunicodeinternedsize
don't seem to be defined as public functions on the Python docs for 3.11. They also don't seem to be available on Linux.
Are those undocumented properties that only exist on macOS?
However I noticed that you reproducer script uses
sys.gettotalrefcount
andsys.getunicodeinternedsize
don't seem to be defined as public functions on the Python docs for 3.11. They also don't seem to be available on Linux.
It's an attribute available (on all platforms) in debug builds only: https://docs.python.org/3/using/configure.html#python-debug-build
Hi @erlend-aasland, thank you very much for the clarification. sys.getunicodeinternedsize
still does not seem to be available on Linux, Python 3.11.3, even when --with-pydebug
is added to the config args:
But I still get an AttributeError: module 'sys' has no attribute 'getunicodeinternedsize'
exception.
Sorry for insisting in this topic (even when I will not be able to test on a macOS machine). I just want to make sure that anyone that can investigate this issue has all the parameters for running the reproducer correctly.
@lysnikolaou would it be possible to edit the initial post to improve the instructions on how to reproduce the problem?
I did some tests on Linux (buildpack-deps:bullseye
base docker image) using Python 3.12.0b2 and I could observe similar numbers for the rc_delta
and alloc_delta
reported.
I am not sure if this is also supposed to happen for this combination of system / Python version as they are different from the initial report.
I noticed one thing:
There are some idioms used in setuptools/distutils:
Distribution
object will hold references to command objects. Command objects hold a reference to the Distribution
object. The same is also true for a setuptools.discovery.ConfigDiscovery
object.weakref.proxy
the number of rc_delta
goes down:run_build_between=False: rc_delta=3 alloc_delta=3
run_build_between=True: rc_delta=66 alloc_delta=121
Is this something relevant? Is this idiom of using "circular references" something that the garbage collector has problems to deal with and should be manually replaced by weakrefs?
On top of this test with weakref
, I tried to edit the script with the following:
- from setuptools import Extension, Distribution
+ import _distutils_hack.override
+ from distutils.core import Extension, Distribution
After running the script, the obtained rc_delta
and alloc_delta
were very similar to the result with the weakref
replacement:
run_build_between=False: rc_delta=3 alloc_delta=3
run_build_between=True: rc_delta=66 alloc_delta=118
My conclusion is that the rest of the references were related to the MetaPathFinder
used for redirecting distutils
.
In particular, the MetaPathFinder
imports setuptools._distutils
, which imply that setuptools/__init__.py
runs.
On top of that, I did another experiment: commenting the monkey.patch_all()
line on the bottom of the setuptools/__init__.py
file. When I run the modified benchmark script again, rc_delta
and alloc_delta
goes down again:
run_build_between=False: rc_delta=3 alloc_delta=3
run_build_between=True: rc_delta=9 alloc_delta=9
So it seems that the remaining objects are related to the monkey patching that setuptools does of distutils. Is there any chance the use of importlib_metadata.entry_points
is involved with that? (It does use some caching, right?)
(I am using a git checkout of setuptools in an environment without setuptools installed, adding the benchmark script to the root of the project)
Hi @erlend-aasland, thank you very much for the clarification.
sys.getunicodeinternedsize
still does not seem to be available on Linux, Python 3.11.3, even when--with-pydebug
is added to the config args:
Yes, that API was added in 3.12.
Yes, that API was added in 3.12.
Ok, that was a bit confusing because the original issue post specifies Python 3.11.3 on macOS.
Is there any chance the use of
importlib_metadata.entry_points
is involved with that? (It does use some caching, right?)
I was investigating this possibility, so I did run a modified version of the benchmark script:
# script.py -- modified to test `entry_points`
import sys
from importlib.metadata import entry_points
int_pool = {value: value for value in range(-1000, 1000)}
def get_pooled_int(value):
return int_pool.setdefault(value, value)
def build():
group = 'setuptools.finalize_distribution_options'
entry_points(group=group)
def main(run_build_between=False):
build()
build()
build()
getallocatedblocks = sys.getallocatedblocks
gettotalrefcount = sys.gettotalrefcount
getunicodeinternedsize = sys.getunicodeinternedsize
interned_before = getunicodeinternedsize()
alloc_before = getallocatedblocks() - interned_before
rc_before = gettotalrefcount() - interned_before * 2
if run_build_between:
build()
interned_after = getunicodeinternedsize()
alloc_after = getallocatedblocks() - interned_after
rc_after = gettotalrefcount() - interned_after * 2
rc_delta = get_pooled_int(rc_after - rc_before)
alloc_delta = get_pooled_int(alloc_after - alloc_before)
print(f"{run_build_between=}: {rc_delta=} {alloc_delta=}")
if __name__ == "__main__":
main(run_build_between=False)
main(run_build_between=True)
As a result, I can see
run_build_between=False: rc_delta=3 alloc_delta=3
run_build_between=True: rc_delta=31 alloc_delta=30
So some part of the increase in rc_delta
and alloc_delta
seem to be related to setuptools use of importlib.metadata
.
Based on the previous results I ran the following test:
# script.py
import sys
from importlib.metadata import entry_points
from setuptools import Extension, Distribution
int_pool = {value: value for value in range(-1000, 1000)}
def get_pooled_int(value):
return int_pool.setdefault(value, value)
def importlib_test():
group = 'setuptools.finalize_distribution_options'
entry_points(group=group)
group = 'setuptools.setup_keywords'
entry_points(group=group)
def build():
with open("hello.c", "w") as f:
f.write("int spam() {\n")
f.write(" return 1;\n")
f.write("}\n")
extension = Extension(
"something",
sources=["hello.c"],
)
Distribution({"name": "something", "ext_modules": [extension]})
getallocatedblocks = sys.getallocatedblocks
gettotalrefcount = sys.gettotalrefcount
getunicodeinternedsize = sys.getunicodeinternedsize
def _test_rc(test_fn, run_in_between):
rc_delta, alloc_delta = 0, 0
while rc_delta <= 0 or alloc_delta <= 0:
test_fn()
test_fn()
test_fn()
interned_before = getunicodeinternedsize()
alloc_before = getallocatedblocks() - interned_before
rc_before = gettotalrefcount() - interned_before * 2
if run_in_between:
test_fn()
interned_after = getunicodeinternedsize()
alloc_after = getallocatedblocks() - interned_after
rc_after = gettotalrefcount() - interned_after * 2
rc_delta = get_pooled_int(rc_after - rc_before)
alloc_delta = get_pooled_int(alloc_after - alloc_before)
return rc_delta, alloc_delta
def test_rc(test_fn, run_in_between, n=100):
results = [_test_rc(test_fn, run_in_between) for _ in range(n)]
return [int(round(sum(x) / len(x))) for x in zip(*results)]
def main():
without_setuptools_rc_delta, without_setuptools_alloc_delta = test_rc(build, False)
with_setuptools_rc_delta, with_setuptools_alloc_delta = test_rc(build, True)
importlib_rc_delta, importlib_alloc_delta = test_rc(importlib_test, True)
print(f"{without_setuptools_rc_delta=} {without_setuptools_alloc_delta=}")
print(f"{with_setuptools_rc_delta=} {with_setuptools_alloc_delta=}")
print(f"{importlib_rc_delta=} {importlib_alloc_delta=}")
if __name__ == "__main__":
main()
# patch.diff
diff --git a/setuptools/dist.py b/setuptools/dist.py
index e5a7b8ed..4494fa77 100644
--- a/setuptools/dist.py
+++ b/setuptools/dist.py
@@ -5,6 +5,7 @@ import sys
import re
import os
import numbers
+import weakref
import distutils.log
import distutils.core
import distutils.cmd
@@ -512,7 +513,7 @@ class Distribution(_Distribution):
self._orig_install_requires = []
self._tmp_extras_require = defaultdict(ordered_set.OrderedSet)
- self.set_defaults = ConfigDiscovery(self)
+ self.set_defaults = ConfigDiscovery(weakref.proxy(self))
self._set_metadata_defaults(attrs)
@@ -991,6 +992,9 @@ class Distribution(_Distribution):
else:
return _Distribution.get_command_class(self, command)
+ def get_command_obj(self, command, create=1):
+ return _Distribution.get_command_obj(weakref.proxy(self), command, create)
+
def print_commands(self):
for ep in metadata.entry_points(group='distutils.commands'):
if ep.name not in self.cmdclass:
As a result, I obtain the following:
without_setuptools_rc_delta=3 without_setuptools_alloc_delta=3
with_setuptools_rc_delta=55 with_setuptools_alloc_delta=28
importlib_rc_delta=55 importlib_alloc_delta=25
Which seems to indicate that (after replacing the "circular references" with weakref.proxy
) the increase in rc_delta
and alloc_delta
of setuptools are roughly equivalent to the ones found by importlib.metadata.entry_points
.
@lysnikolaou is replacing the "circular references" (as explained in https://github.com/pypa/setuptools/issues/3938#issuecomment-1589837377) with weakrefs something the users are expected to do? My other question is: have you investigated importlib.metadata.entry_points
(it seems to be the other main responsible for the increase in rc_delta
and alloc_delta
in setuptools)?
setuptools version
67.8.0
Python version
3.11.3
OS
macOS
Description
Some background info: When adding the new PEG parser to CPython back in 3.9, we also added some tests that test the new parser generator. In order to do so, the tests build an extension module with a lot of different grammar, use that to parse a piece of code, and then compare the resulting syntax trees with the expected one.
Up until 3.11, we were using
distutils
to build the extension module. After the removal of distutils in 3.12, we recently migrated into using setuptools in python/cpython#104798. Since that PR was merged, some buildbots fail due to this test cause of some refleaks (example). Me and @pablogsal were able to bisect that down to the call tosetuptools.Distribution
insetup.py
.We're still not sure whether that's a true refleak in setuptools or some caching that setuptools does, which leads to the CPython test suite to fail, so I'm opening this issue to hopefully start a discussion on whether CPython needs to change something or setuptools.
I'm also including below a little script that reproduces this without the need of the CPython test suite or anything else.
Expected behavior
No refleaks.
How to Reproduce
Run the following script:
Output