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

Rearranged WebPImagePlugin _accept for speed #2217

Closed
wants to merge 1 commit into from

Conversation

radarhere
Copy link
Member

Minor rearranging for speed - if the first condition fails, the other conditions will not be checked.

@hugovk
Copy link
Member

hugovk commented Nov 12, 2016

This looks a bit like premature optimisation and the old code is self-documenting. What speed is gained here? I'd guess slicing three strings isn't particularly slow, but have you measured it and found this to be a problem?

@radarhere
Copy link
Member Author

I just thought it was a minor improvement. Feel free to close.

Copy link
Member

@homm homm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm for it because this code is executed for every file, not only WebP images.

@wiredfool
Copy link
Member

I've just done some stepping through this area of the code, and I think this is of minimal use.

The order that image plugins are tried is:

['BMP', 'GIF', 'TIFF', 'JPEG', 'PPM', 'PNG', 'BUFR', 'CUR', 'PCX', 'DCX', 'DDS', 'EPS', 'FITS', 'FLI', 'FPX', 'FTEX', 'GBR', 'GRIB', 'HDF5', 'JPEG2000', 'ICNS', 'ICO', 'IM', 'IMT', 'IPTC', 'MCIDAS', 'MIC', 'MPEG', 'MSP', 'PCD', 'PIXAR', 'PSD', 'SGI', 'SPIDER', 'SUN', 'TGA', 'WEBP', 'WMF', 'XBM', 'XPM', 'XVTHUMB']

There are several image formats that don't even have _accept functions in the list before webp. Every single one of them makes at least one read to the file pointer, some of them read field by field. That's potentially a disk hit.

 for format in Image.ID: print ("%s, %s" % (format, Image.OPEN[format][1]))
... 
BMP, <function _accept at 0x7f76364f8830>
GIF, <function _accept at 0x7f7635b460e0>
TIFF, <function _accept at 0x7f7635a8fdd0>
JPEG, <function _accept at 0x7f7635b684d0>
PPM, <function _accept at 0x7f7635a58320>
PNG, <function _accept at 0x7f76359fc9e0>
BUFR, <function _accept at 0x7f7635a177a0>
CUR, <function _accept at 0x7f7635a17cb0>
PCX, <function _accept at 0x7f7635a17290>
DCX, <function _accept at 0x7f7635a17560>
DDS, <function _validate at 0x7f76359eae60>
EPS, <function _accept at 0x7f76359e57a0>
FITS, <function _accept at 0x7f76359e5950>
FLI, <function _accept at 0x7f7635ab7440>
FPX, <function _accept at 0x7f7635ab7b00>
FTEX, <function _validate at 0x7f7635a7d5f0>
GBR, <function _accept at 0x7f7635a7d950>
GRIB, <function _accept at 0x7f7635a7def0>
HDF5, <function _accept at 0x7f763575f560>
JPEG2000, <function _accept at 0x7f763572a3b0>
ICNS, <function <lambda> at 0x7f76357497a0>
ICO, <function _accept at 0x7f763574e170>
IM, None
IMT, None
IPTC, None
MCIDAS, <function _accept at 0x7f7635757950>
MIC, <function _accept at 0x7f7635757e60>
MPEG, None
MSP, <function _accept at 0x7f763575c3b0>
PCD, None
PIXAR, <function _accept at 0x7f7634cfb290>
PSD, <function _accept at 0x7f7634cfb8c0>
SGI, <function _accept at 0x7f7634cfbef0>
SPIDER, None
SUN, <function _accept at 0x7f7634d053b0>
TGA, None
WEBP, <function _accept at 0x7f7634d059e0>
WMF, <function _accept at 0x7f7634d0b3b0>
XBM, <function _accept at 0x7f7634d0b680>
XPM, <function _accept at 0x7f7634d0b9e0>
XVTHUMB, <function _accept at 0x7f7634d0bcb0>

@wiredfool wiredfool closed this Feb 17, 2017
@radarhere radarhere deleted the webpimageplugin branch February 17, 2017 22:53
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

4 participants