new option gc-after-test
#133
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The option does not work with On |
|
I am pretty packed this week, but still I had quick look yesterday night. Back then when I ran tox on all zopefoundation projects, I found quite some with resource warnings. So I picked this one to zopefoundation/zc.recipe.deployment#8 to test drive your changes. But I was a bit surprised when I ran the current version of zope.testrunner and finding this output: So it looks like that resource warnings are already decently shown even before applying your PR. I then installed your changes: and ran and there was no difference in output (except for paths for the tmp files).
So this was a "black box test" - without reading the implementation. I'll have a look at the implementation on the weekend. |
…cyclic garbage referenced by the test
|
@jugmac00 Did you find the time to look into this PR? |
|
Michael Howitz wrote at 2022-5-25 09:18 -0700:
***@***.*** Did you find the time to look into this PR?
I plan to extend it a bit further:
add an implementation of the Tarjan SCC (Strongly Connected Component)
algorithm and use it for the analysis of cyclic garbage
(at verbosity level 3+ together with the `gc-after-test` option).
I used this approach manually to analyse the cyclic
garbage left by the ZEO implementation.
Restricting the analysis to the cycles only (they are
the non trivial strongly connected components) drastically
facilitates the detection of the causes for those cycles.
Therefore, it might be good to wait with a review
until this addition is in place.
|
I am afraid no. Both private and work life are currently super busy. @d-maurer Could you give us a clearer scope of this PR? In your first comment you mention that the new option helps to analyze resource warnings, but as seen in my comment, resource warnings are already very well visible with the current state. |
|
Jürgen Gmach wrote at 2022-5-25 09:46 -0700:
> @jugmac00 Did you find the time to look into this PR?
I am afraid no. Both private and work life are currently super busy.
***@***.*** Could you give us a clearer scope of this PR?
In your first comment you mention that the new option helps to analyze resource warnings, but as seen in my comment, resource warnings are already very well visible with the current state.
I did explain it already (recently, `github` lost some of my email
replies -- maybe, this one too):
`ResourceWarning`s are generated when the object is finalized.
For resources referenced from a cycle, the finalization
happens when the garbage collector is run.
The PR allows the user to request a garbage collection at the end
of each test. This ensures that `ResourceWarning`s are reported
in the context of the test responsible for it.
Without this option, the `ResourceWarning` may be reported in a
following test (or not at all) depending on when the garbage collector
happens to kick in.
|
|
@jugmac00 To visualize the effect of zope-testrunner --path src/zope/testrunner/tests/testrunner-ex --tests-pattern gc-after-test -vv >~/tmp/wo.log 2>&1
zope-testrunner --path src/zope/testrunner/tests/testrunner-ex --tests-pattern gc-after-test -vv --gc-after-test >~/tmp/wi.log 2>&1i.e. --- /home/dieter/tmp/wo.log 2022-05-31 08:17:19.263474317 +0200
+++ /home/dieter/tmp/wi.log 2022-05-31 08:20:13.237727483 +0200
@@ -2,8 +2,10 @@
Running zope.testrunner.layer.UnitTests tests:
Set up zope.testrunner.layer.UnitTests in 0.000 seconds.
Running:
- test_cycle_with_resource (gc-after-test.GcAfterTestTests)
- test_cycle_without_resource (gc-after-test.GcAfterTestTests)
+ test_cycle_with_resource (gc-after-test.GcAfterTestTests)zope/testrunner/tests/testrunner-ex/gc-after-test.py:60: ResourceWarning: not closed
+ warn(ResourceWarning("not closed"))
+ [3]
+ test_cycle_without_resource (gc-after-test.GcAfterTestTests) [2]
test_exception (gc-after-test.GcAfterTestTests)
Error in test test_exception (gc-after-test.GcAfterTestTests)
@@ -35,11 +37,9 @@
test_okay (gc-after-test.GcAfterTestTests)
- test_test_holds_cycle (gc-after-test.GcAfterTestTests)
- test_traceback_cycle (gc-after-test.GcAfterTestTests)
-zope/testrunner/tests/testrunner-ex/gc-after-test.py:60: ResourceWarning: not closed
- warn(ResourceWarning("not closed"))
- Ran 7 tests with 1 failures, 1 errors and 0 skipped in 0.001 seconds.
+ test_test_holds_cycle (gc-after-test.GcAfterTestTests) [3]
+ test_traceback_cycle (gc-after-test.GcAfterTestTests) [7]
+ Ran 7 tests with 1 failures, 1 errors and 0 skipped in 0.100 seconds.
Tearing down left over layers:
Tear down zope.testrunner.layer.UnitTests in 0.000 seconds.This shows: with |
…umber of objects on traceback cycles for test `gc-after-test`
|
Example for the new feature to analyze cyclic garbage: The test left 208 garbage objects in 3 cycles. |
navytux
pushed a commit
to zopefoundation/ZEO
that referenced
this pull request
Aug 2, 2022
The commit target and cyclic garbage created during tests. For the analysis, a `zope.testrunner` enhancement ("zopefoundation/zope.testrunner#133") has been important.
...
I believe that the ZEO implementation no longer creates cyclic garbage of its own. However, most tests still produce cyclic garbage. The analysis found 3 causes:
1. Python 3 (at least) creates a reference cycle for each class definition (between the class and its `mro'). This means that (most) local class definitions result in cyclic garbage. `unittest.mock` (prominently used by some tests) makes extensive use of local class definitions.
2. `asyncio` (at least in Python 3.9) can create cyclic garbage
3. `zope.interface` can create cyclic garbage in its implementation of `alsoProvides`
The cyclic garbage collector can free the cyclic structures; thus, there is no serious problem.
Extracted from #195
More details at: #195 (comment)
navytux
pushed a commit
to zopefoundation/ZEO
that referenced
this pull request
Aug 2, 2022
The commit target and cyclic garbage created during tests. For the analysis, a `zope.testrunner` enhancement ("zopefoundation/zope.testrunner#133") has been important.
...
I believe that the ZEO implementation no longer creates cyclic garbage of its own. However, most tests still produce cyclic garbage. The analysis found 3 causes:
1. Python 3 (at least) creates a reference cycle for each class definition (between the class and its `mro'). This means that (most) local class definitions result in cyclic garbage. `unittest.mock` (prominently used by some tests) makes extensive use of local class definitions.
2. `asyncio` (at least in Python 3.9) can create cyclic garbage
3. `zope.interface` can create cyclic garbage in its implementation of `alsoProvides`
The cyclic garbage collector can free the cyclic structures; thus, there is no serious problem.
Extracted from #195
More details at: #195 (comment)
|
Jürgen Gmach wrote at 2022-4-25 23:06 -0700:
I am pretty packed this week, but still I had quick look yesterday night.
Thank you for investing your time!
...
So it looks like that resource warnings are already decently shown even before applying your PR.
I then installed your changes:
```
? .tox/py39/bin/pip install ***@***.***
```
and ran
```
? tox -e py39 -- --gc-after-test
```
and there was no difference in output (except for paths for the tmp files).
`ResourceWarning` can be reported in two situations:
* (mostly) locally
This typically happens when the resource is released
because the reference count reaches 0.
Example: `open(filename).read()`
* non locally
This happens when the resource is released during
`gc.collect` (i.e. the resource was referenced from
a reference cycle)
The PR does nothing for the first kind.
For the second kind, the new option ensures that `gc.collect` is called
after each test and thereby
associates a 'ResourceWarning' with the test which created
the resource.
Without the new option, the `ResourceWarning` is raised
wheneven the next garbage collection happens -- potentially during
a later test or not at all.
My motivation for this PR comes from my hunting down
`ResourceWarning`s in the context of
"zopefoundation/ZEO#195".
It has been plagued by many `ResourceWarning`s of the second
kind and often those warnings have been raised several tests
after the one actually responsible.
Being able to precisely associate the warning with the
resource creating test helped a lot (mainly by allowing to
use the `-t` option to run this test and verify whether a fix
was successful).
|
navytux
pushed a commit
to zopefoundation/ZEO
that referenced
this pull request
Dec 22, 2022
The commit target and cyclic garbage created during tests. For the analysis, a `zope.testrunner` enhancement ("zopefoundation/zope.testrunner#133") has been important.
...
I believe that the ZEO implementation no longer creates cyclic garbage of its own. However, most tests still produce cyclic garbage. The analysis found 3 causes:
1. Python 3 (at least) creates a reference cycle for each class definition (between the class and its `mro'). This means that (most) local class definitions result in cyclic garbage. `unittest.mock` (prominently used by some tests) makes extensive use of local class definitions.
2. `asyncio` (at least in Python 3.9) can create cyclic garbage
3. `zope.interface` can create cyclic garbage in its implementation of `alsoProvides`
The cyclic garbage collector can free the cyclic structures; thus, there is no serious problem.
Extracted from #195
More details at: #195 (comment)
navytux
pushed a commit
to zopefoundation/ZEO
that referenced
this pull request
Dec 30, 2022
The commit target and cyclic garbage created during tests. For the analysis, a `zope.testrunner` enhancement ("zopefoundation/zope.testrunner#133") has been important.
...
I believe that the ZEO implementation no longer creates cyclic garbage of its own. However, most tests still produce cyclic garbage. The analysis found 3 causes:
1. Python 3 (at least) creates a reference cycle for each class definition (between the class and its `mro'). This means that (most) local class definitions result in cyclic garbage. `unittest.mock` (prominently used by some tests) makes extensive use of local class definitions.
2. `asyncio` (at least in Python 3.9) can create cyclic garbage
3. `zope.interface` can create cyclic garbage in its implementation of `alsoProvides`
The cyclic garbage collector can free the cyclic structures; thus, there is no serious problem.
Extracted from #195
More details at: #195 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
to analyze
ResourceWarnings and the creation of cyclic garbage.Fixes #131.
The PR introduces the option
gc-after-test. Its setting causesgc.collectto be called after each test and its return value to be recorded. If the return value rv is nonzero,!is output on verbosity level 1 and[rv]on verbosity levels above 1. This makes it easy to detect whether a test has created cyclic garbage.gc-after-testcan also help in the analysis of theResourceWarnings not raised locally but only as a side effect of garbage collection. Havinggc.collectcalled after each test allows to associate aResourceWarningwhich the test responsible for the warning; otherwise, it may not be reported at all or reported in the context of a different test (when garbage collection happens to be activated).