-
-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Modify the definition of SDImageFormat to extern to avoid creating multiple global variables #3699
base: master
Are you sure you want to change the base?
Conversation
…ing multiple global variables
Looks good for me. By the way, I want to talk about this This shit design (using a enum) is from the history SDWebImage 2.x, and changed into The most stupid is that this is actually not In SD 6.0, I'll totally remove this bad design, use class instead. // In SDWebImage Core repo
@interface SDImageFormat : NSObject
@property (readonly, nonnull) NSString *UTI;
@property (readonly, nonnull) NSString *preferredFilenameExtension;
@property (readonly, nonnull) NSString *preferredMIMEType;
@end
// Each coder repo, like JPEGXL
@interface SDImageFormat (JPEGXL)
@property (class, readonly) SDImageFormat *JPEGXL;
@end
// WebP repo
@interface SDImageFormat (WebP)
@property (class, readonly) SDImageFormat *WebP;
@end
// APNG
@interface SDImageFormat (APNG)
@property (class, readonly) SDImageFormat *APNG;
@end |
return [self.class canEncodeToFormat:SDImageFormatWebP]; | ||
default: | ||
return NO; | ||
if (format == SDImageFormatWebP) { |
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.
Compiler can do optimization based on enum cases number, these changes is no needed.
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.
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.
So, is this cause API break (assume user use code like this in switch case?)
If so, why not totally break and use class declarations instead
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.
I remembered...In 5.c
This static declaration is written by me to allows "switch case" works
This code is indeed, so seems this is not possible in 5.x
So, at least, we need to fix the 10+ individual coder repo as well (they use static declaration) Can we wait for the total break changes to use class based |
This is just a small optimization, looking forward to version 6.0 |
095d3ad
to
78fe228
Compare
I found that several formats about SDImageFormat are static variables defined in a header file. When the program is compiled and linked, any .m file that imports or indirectly imports this header will create a static variable inside the file. This results in an increase in useless binary size. In fact, only need to declare these variables as extern and assign values inside a certain .m file, so that there will only be one copy of these variables in the entire binary.