Call `gc.collect` after each test only if the user asks for it #6707

Merged
merged 1 commit into from Jul 12, 2016

Conversation

Projects
None yet
6 participants
Member

Kojoley commented Jul 8, 2016 edited

Fixes #6705

mdboom added the needs_review label Jul 8, 2016

Member

WeatherGod commented Jul 8, 2016

I have restarted the py2.7 test two times, and it has always failed in the same spot. The other tests are also taking longer to run. I wonder if the memory usage isn't staying low and is wasting time swap thrashing?

Member

WeatherGod commented Jul 8, 2016

The particular failure is one of those weird ones that we have been noticing (image size mismatch for the pdf comparison for the hatching test). Perhaps the gc.collect() was masking a deeper problem?

@QuLogic QuLogic and 1 other commented on an outdated diff Jul 8, 2016

lib/matplotlib/testing/enforcedgc.py
@@ -0,0 +1,26 @@
+from __future__ import (absolute_import, division, print_function,
+ unicode_literals)
+
+import gc
+import os
+from nose.plugins import Plugin
+
+
+class EnforcedGC(Plugin):
+ """This plugin adds option to call ``gc.collect`` after each test"""
+ enabled = False
+
+ def options(self, parser, env=os.environ):
+ env_opt = 'COLLECT_GC_AFTER_TEST'
@QuLogic

QuLogic Jul 8, 2016

Member

The name is a bit weird; you don't collect the GC, the GC does the collecting. Maybe just drop collect from the name.

@Kojoley

Kojoley Jul 8, 2016

Member

It was CALL_GC_COLLECT_AFTER_TEST and then I have reduced it.
I can't name things. T_T

tacaswell added this to the 2.1 (next point release) milestone Jul 8, 2016

Owner

jenshnielsen commented Jul 10, 2016

The failure was caused by an image in test_axes leaking into the test_backendpdf tests which runs right after the axes tests. See #6720 for the fix. It seems like the garbage collect mostly ensured that the figure was destroyed before the next test masking the missing cleanup decorator

Owner

jenshnielsen commented Jul 10, 2016

#6113 which is the same fix as #6120 has now been merged to master. If you rebase on top of current master this will hopefully start working as expected

@Kojoley Kojoley Call `gc.collect` after each test only if the user asks for it
e6def4e
Member

Kojoley commented Jul 10, 2016

All tests have passed after rebasing. 👍

Owner

jenshnielsen commented Jul 11, 2016

I'm 👍 on merging this now. It seems that the speedup is most significant on the single core tests (python 3.5 and OSX) but that is certainly important too

@tacaswell tacaswell merged commit 7eab7d9 into matplotlib:master Jul 12, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.0007%) to 70.348%
Details

tacaswell removed the needs_review label Jul 12, 2016

Kojoley deleted the Kojoley:do-not-call-gc-collect-by-default branch Jul 12, 2016

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