-
-
Notifications
You must be signed in to change notification settings - Fork 433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Poor performance in Python 3.12 #1665
Comments
The first thing to check is whether 3.12 has the C extension available.
or:
|
It apparently does: https://github.com/Quansight-Labs/ndindex/actions/runs/5766363872/job/15634144314?pr=161#step:5:30. It looks like there are 3.12 manylinux builds on pypi https://pypi.org/project/coverage/#files, so it is grabbing that. |
Here's
(from https://github.com/Quansight-Labs/ndindex/actions/runs/5766385439/job/15634216577?pr=161#step:5:48) |
This is a mystery. I forked your repo (https://github.com/nedbat/ndindex) and hacked away at it. Now I have a file (justdoit.py) that sets a do-nothing trace function, and runs one of your tests: https://github.com/nedbat/ndindex/actions/runs/5767689409. On 3.11, it takes 14sec. On 3.12, it takes 43sec. I don't know why. We could toss this over to CPython to diagnose. They will probably want to take a look. |
I've reported this in CPython: python/cpython#107674 |
This is with the C extension? At CPython they had a look and didn't do anything for 3.12.0. There are some hopes that coveragepy will switch to the new monitoring API for 3.12 but I didn't find any trace supporting that hope here in coveragepy repo. Is there hope :)? python/cpython#107841 (comment) |
I've pinged the core dev on the CPython issue. I have started on code to use the new PEP 669 monitoring, but I haven't made much progress on it. I'd welcome help. |
For anyone else still struggling with this and who missed the changelog: coverage 7.4.0 includes experimental support for PEP 669 that can be enabled with |
Thanks for the tip. This would be a lot easier to use if that variable were just ignored for Python 3.11 and lower. Right now, setting it causes those versions to raise an exception. |
Yes, I've gotten that feedback, and there's a PR: #1747. I'll figure out how to make that right soon. |
This flag has indeed made the coverage runs for 3.12 way faster. They now run in about the same amount of time as when coverage is disabled! https://github.com/Quansight-Labs/ndindex/actions/runs/7892990262/usage |
Python 3.12 introduced a new (much faster) way of tracking and monitoring execution of python code by tools like coverage tracking using sysmon (PEP 669). This however also apparently heavily impacted performance of coverage tracking for Python 3.12 when PEP 669 is not used. The coverage library since 7.4.0 has an experimental support for PEP 669 that can be enabled with COVERAGE_CORE=sysmon env variable and a number of users confirmed it fixes the problem. We are using 7.4.4 coverage already so we should enable this mode to speed up our coverage tracking. That should also allow us to remove databricks from excluded providers. See databricks/databricks-sql-python#369 for databricks case and nedbat/coveragepy#1665 for coverage bug.
) Python 3.12 introduced a new (much faster) way of tracking and monitoring execution of python code by tools like coverage tracking using sysmon (PEP 669). This however also apparently heavily impacted performance of coverage tracking for Python 3.12 when PEP 669 is not used. The coverage library since 7.4.0 has an experimental support for PEP 669 that can be enabled with COVERAGE_CORE=sysmon env variable and a number of users confirmed it fixes the problem. We are using 7.4.4 coverage already so we should enable this mode to speed up our coverage tracking. That should also allow us to remove databricks from excluded providers. See databricks/databricks-sql-python#369 for databricks case and nedbat/coveragepy#1665 for coverage bug.
Solution from nedbat/coveragepy#1665 (comment) Thanks @trent-codecov for the suggestion.
* copy changes from #285 I'm copying the changes because that PR is far behind `main`. We will probably wait for the base image change to happen before merging this update work though * upgrade packages until project builds and tests run * fix tests * disable mutation testing * fix: Fix pytest-cov performance Solution from nedbat/coveragepy#1665 (comment) Thanks @trent-codecov for the suggestion.
Contrary to what others reported, the |
…194) Python 3.12 introduced a new (much faster) way of tracking and monitoring execution of python code by tools like coverage tracking using sysmon (PEP 669). This however also apparently heavily impacted performance of coverage tracking for Python 3.12 when PEP 669 is not used. The coverage library since 7.4.0 has an experimental support for PEP 669 that can be enabled with COVERAGE_CORE=sysmon env variable and a number of users confirmed it fixes the problem. We are using 7.4.4 coverage already so we should enable this mode to speed up our coverage tracking. That should also allow us to remove databricks from excluded providers. See databricks/databricks-sql-python#369 for databricks case and nedbat/coveragepy#1665 for coverage bug. GitOrigin-RevId: dffc9e3fa83e3cd910a83e7c505fe23b26feddf2
…194) Python 3.12 introduced a new (much faster) way of tracking and monitoring execution of python code by tools like coverage tracking using sysmon (PEP 669). This however also apparently heavily impacted performance of coverage tracking for Python 3.12 when PEP 669 is not used. The coverage library since 7.4.0 has an experimental support for PEP 669 that can be enabled with COVERAGE_CORE=sysmon env variable and a number of users confirmed it fixes the problem. We are using 7.4.4 coverage already so we should enable this mode to speed up our coverage tracking. That should also allow us to remove databricks from excluded providers. See databricks/databricks-sql-python#369 for databricks case and nedbat/coveragepy#1665 for coverage bug. GitOrigin-RevId: dffc9e3fa83e3cd910a83e7c505fe23b26feddf2
…194) Python 3.12 introduced a new (much faster) way of tracking and monitoring execution of python code by tools like coverage tracking using sysmon (PEP 669). This however also apparently heavily impacted performance of coverage tracking for Python 3.12 when PEP 669 is not used. The coverage library since 7.4.0 has an experimental support for PEP 669 that can be enabled with COVERAGE_CORE=sysmon env variable and a number of users confirmed it fixes the problem. We are using 7.4.4 coverage already so we should enable this mode to speed up our coverage tracking. That should also allow us to remove databricks from excluded providers. See databricks/databricks-sql-python#369 for databricks case and nedbat/coveragepy#1665 for coverage bug. GitOrigin-RevId: dffc9e3fa83e3cd910a83e7c505fe23b26feddf2
Describe the bug
I recently added a Python 3.12-dev build to a project where I enable coverage on all the CI test runs, and the 3.12 build has consistently been taking nearly twice a long to complete as the 3.8-3.11 builds.
I made a test PR disabling coverage and the issue went away. Obviously without coverage the tests run faster, but the point is that the different Python versions all ran in about the same time.
Compare some builds (with coverage):
https://github.com/Quansight-Labs/ndindex/actions/runs/5734923621/usage?pr=155
https://github.com/Quansight-Labs/ndindex/actions/runs/5734722635/usage
A build with coverage disabled:
https://github.com/Quansight-Labs/ndindex/actions/runs/5765483809/usage?pr=161
To Reproduce
I haven't yet tried to install Python 3.12 to see if I can reproduce this locally, but the performance hit is pretty consistent on CI (you can see it on pretty much every recent build in Quansight-Labs/ndindex#155 for example).
Happy to provide more info, and try to repro locally if you can suggest an easy way to install a Python 3.12 environment. The package in question is pure Python, but the tests depend heavily on both numpy (I've been installing 3.12-compatible dev wheels from https://pypi.anaconda.org/scientific-python-nightly-wheels/simple) and hypothesis.
I mostly just wanted to make sure was on your radar since I didn't see any other issues about it.
The text was updated successfully, but these errors were encountered: