-
Notifications
You must be signed in to change notification settings - Fork 254
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
Fix performance issue with adding large images with FlateDecode
image filter
#644
Conversation
return zlib.compress(data) | ||
row_size = img.size[0] * channels_count | ||
|
||
data_with_padding = bytearray() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Nice! Well done!
Could this improvement allow us to reduce the duration of this performance test?
https://github.com/PyFPDF/fpdf2/blob/master/test/test_perfs.py#L10
This is ready to be merged for me,
I'll just wait for your answer @Markovvn1 to see if we can lower this threshold
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My changes have almost no effect on the speed of this test because this test works with small images (less than 100x100 pixels). At these sizes the asymptotics of O(N) is not much different from O(N^2) in comparison with other operations.
However, personally on my computer, comparable in power to Github CI, it takes only 11.6 seconds to run. So it is probably possible to reduce the threshold from 40 seconds to 20 seconds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, thank you for your feedback!
I'm going to merge this and see in a later PR if I can lower this threshold
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like 30s is too low for GitHub Actions runners:
#647
Nevermind 😄
@allcontributors please add @Markovvn1 for code |
I've put up a pull request to add @Markovvn1! 🎉 |
Codecov ReportBase: 93.96% // Head: 93.96% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #644 +/- ##
=======================================
Coverage 93.96% 93.96%
=======================================
Files 25 25
Lines 6457 6457
Branches 1156 1156
=======================================
Hits 6067 6067
Misses 223 223
Partials 167 167
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Fixes #643
Reduced the time required to add large images with the
FlateDecode
filter. Now adding an 8000x8000 RGB image takes only 1.3 seconds (was 55 seconds - see #643)Checklist:
The GitHub pipeline is OK (green),
meaning that both
pylint
(static code analyzer) andblack
(code formatter) are happy with the changes of this PR.A unit test is covering the code added / modified by this PR
This PR is ready to be merged
In case of a new feature, docstrings have been added, with also some documentation in the
docs/
folder (N/A)A mention of the change is present in
CHANGELOG.md
By submitting this pull request, I confirm that my contribution is made under the terms of the GNU LGPL 3.0 license.