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

Fix performance issue with adding large images with FlateDecode image filter #644

Merged
merged 2 commits into from
Dec 25, 2022

Conversation

Markovvn1
Copy link

@Markovvn1 Markovvn1 commented Dec 24, 2022

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) and black (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.

return zlib.compress(data)
row_size = img.size[0] * channels_count

data_with_padding = bytearray()
Copy link
Member

@Lucas-C Lucas-C Dec 25, 2022

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

Copy link
Author

@Markovvn1 Markovvn1 Dec 25, 2022

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.

Copy link
Member

@Lucas-C Lucas-C Dec 25, 2022

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

Copy link
Member

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 😄

@Lucas-C
Copy link
Member

Lucas-C commented Dec 25, 2022

@allcontributors please add @Markovvn1 for code

@allcontributors
Copy link

@Lucas-C

I've put up a pull request to add @Markovvn1! 🎉

@codecov
Copy link

codecov bot commented Dec 25, 2022

Codecov Report

Base: 93.96% // Head: 93.96% // No change to project coverage 👍

Coverage data is based on head (adc5d2e) compared to base (d3df9b0).
Patch coverage: 100.00% of modified lines in pull request are covered.

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           
Impacted Files Coverage Δ
fpdf/image_parsing.py 88.49% <100.00%> (ø)

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Lucas-C Lucas-C merged commit a961d74 into py-pdf:master Dec 25, 2022
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.

Performance issue with adding large images with FlateDecode image filter
2 participants