Use test cache for test result images too.#15932
Merged
tacaswell merged 1 commit intomatplotlib:masterfrom Sep 25, 2020
Merged
Conversation
Member
|
If we have a few tests that are not deterministic, will this cause the cache to increase without bound? I think that we cache these folders on at least travis so we would want to make sure we don't start spending more time moving around invalid images than we save by not computing them. |
Contributor
Author
|
Fair enough, added a simple (automatic) cache eviction mechanism. |
Member
|
I don't fully follow this, but it seems useful. But needs a rebase, and probably a review from @tacaswell or @QuLogic |
For image comparison tests in svg/pdf/ps formats, the result images are converted to png for comparison. Previously the conversion results were cached for the baseline images, but not for the test-generated images (because of non-deterministic svg/pdf/etc. results, due to hash-salting, dict ordering, etc.). Now that the test-generated images are generally deterministic, we can enable the cache for baseline images as well. This speeds up `pytest -k '[svg]'` by ~30% (81s initially -> 55s on a seeded cache) and `pytest -k '[pdf]'` by ~10% (62s -> 55s) (there are too few (e)ps image comparison tests to see an effect). Also add logging regarding the cache which may help troubleshooting determinacy problems. A simple cache eviction mechanism prevents the cache from growing without bounds, limiting it to 2x the size of the baseline_images directory. This is a much simpler version of PR7764, which added more sophisticated reporting of cache hits and misses and cache eviction.
Contributor
Author
|
rebased |
QuLogic
approved these changes
Sep 24, 2020
7 tasks
This file contains hidden or 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
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.
For image comparison tests in svg/pdf/ps formats, the result images are
converted to png for comparison. Previously the conversion results were
cached for the baseline images, but not for the test-generated images
(because of non-deterministic svg/pdf/etc. results, due to
hash-salting, dict ordering, etc.).
Now that the test-generated images are generally deterministic, we can
enable the cache for baseline images as well. This speeds up
pytest -k '[svg]'by ~30% (81s initially -> 55s on a seeded cache) andpytest -k '[pdf]'by ~10% (62s -> 55s) (there are too few (e)ps imagecomparison tests to see an effect). Also add logging regarding the
cache which may help troubleshooting determinacy problems.
This is a much simpler version of PR#7764, which added more sophisticatedreporting of cache hits and misses, as well as cache invalidation (I
think the cache can reasonably be cleaned manually, at least for now).
A simple cache eviction mechanism prevents the cache from growing
without bounds, limiting it to 2x the size of the baseline_images
directory.
This is a much simpler version of PR7764, which added more sophisticated
reporting of cache hits and misses and cache eviction.
Supersedes (mostly) #7764 (@jkseppan).
(Note that the gain in speed is less than reported than in #7764 (comment), because I have also optimized the svg and gs converters in the meantime by running a single process and interacting with it over stdin/stdout, rather that launching a new instance of inkscape/ghostscript for each image.)
PR Summary
PR Checklist