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

Performance: Do not store attribute in TIFF new API #365

Closed
wants to merge 1 commit into from

Conversation

Glandos
Copy link
Contributor

@Glandos Glandos commented Feb 11, 2019

The end of my current performance improvement, after #362, #363 and #364.

This one should not be merged as-is for the following reasons:

  • It is removing the use of the v2 TIFF API in the v1 API. It is completely safe for JpegImagePlugin, which makes only use of it, but I don't know the status of other plugins.
  • It is monkey patching.

However, I exposed this to show that on my gallery with 536 images and 46 videos, I'm experiencing a boost of 32% in speed for idle builds.

With all other mentionned patches, I'm going from idle build in 3.23 seconds in master to 1.06 second. It is roughly one-third of the current situation.

Sometimes, it seems that Pillow is not focused on speed, so that's why monkey patching could be allowed (e.g. for #363). This one could be proposed as an option in the configuration.

Since JpegImagePlugin does not make use of it, it feel safe here.
However, other plugin might be using it…
@codecov
Copy link

codecov bot commented Feb 11, 2019

Codecov Report

Merging #365 into master will increase coverage by 0.07%.
The diff coverage is 94.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #365      +/-   ##
==========================================
+ Coverage   87.38%   87.46%   +0.07%     
==========================================
  Files          18       18              
  Lines        1443     1460      +17     
==========================================
+ Hits         1261     1277      +16     
- Misses        182      183       +1
Impacted Files Coverage Δ
sigal/__init__.py 86.09% <100%> (+0.09%) ⬆️
sigal/utils.py 91.78% <93.75%> (+0.55%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86397a2...b216c3d. Read the comment docs.

@Glandos
Copy link
Contributor Author

Glandos commented Feb 18, 2019

See python-pillow/Pillow#3663

The above PR is much better, since it includes this current patch, along with other improvements.

@Glandos Glandos closed this Feb 18, 2019
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.

None yet

1 participant