Skip to content
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

TST: Remove memory leak test #27732

Merged
merged 1 commit into from
Feb 5, 2024
Merged

TST: Remove memory leak test #27732

merged 1 commit into from
Feb 5, 2024

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Feb 1, 2024

PR summary

It is flaky, and our memory "limit" is pretty much arbitrary. Testing with tools/memleak.py shows that memory usage generally rises and only stabilizes close to 200 figures, while this test only does a warmup of 2 figures. Waiting for equilibrium would take too long for a general test, so we might move this to a weekly job later.

For example, with just Agg:
agg

and with the flaky wxAgg:
wxagg

It's true that the latter uses more memory, but it still takes about 200 or so figure to reach equilibrium.

Closes #27635

PR checklist

It is flaky, and our memory "limit" is pretty much arbitrary. Testing
with `tools/memleak.py` shows that memory usage generally rises and only
stabilizes close to 200 figures, while this test only does a warmup of
2 figures. Waiting for equilibrium would take too long for a general
test, so we might move this to a weekly job later.
@QuLogic QuLogic added this to the v3.9.0 milestone Feb 1, 2024
@richardsheridan
Copy link
Contributor

I'd be on board with a weekly run that's not tied to PRs. After all we're not really worried about any specific threshold, just if things are growing excessively, and a weekly trend would be easy to bisect, at least compared to bug reports.

Maybe it would be worth putting together a whole pythonspeed thing with benchmarks and memory usage?

@dstansby
Copy link
Member

dstansby commented Feb 4, 2024

I presume the x-axis in those plots is elapsed time in seconds? I'm 👍 to removing these to stop the CI failing for now given it doesn't seem like there's an easy or quick fix to the tests, but would be worth opening an issue to discuss ways to fix these tests or add replacement memory leak tests.

@tacaswell
Copy link
Member

I presume the x-axis in those plots is elapsed time in seconds?

The x-axis is cycle count (so how many Figures we have created and destroyed).

@ksunden ksunden merged commit e470c70 into matplotlib:main Feb 5, 2024
39 of 41 checks passed
@QuLogic QuLogic deleted the no-leak-test branch February 5, 2024 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: test_figure_leak_20490 repeatedly failing on CI
5 participants