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.

Sign up for GitHub

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

Jump to bottom

Replace +load initializers with __attribute__((constructor)) functions #1425

Merged
merged 2 commits into from
Mar 29, 2019
Merged

Replace +load initializers with __attribute__((constructor)) functions #1425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca18fbc
Replace +load initializers with __attribute__((constructor)) functions
farktronix Mar 26, 2019
afbd703
Updating the documentation for textureDidInitialize, per @nguyenhuy
farktronix Mar 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Source/ASConfigurationDelegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ NS_ASSUME_NONNULL_BEGIN
/**
* Texture framework initialized. This method is called synchronously
* on the main thread from ASInitializeFrameworkMainThread if you defined
* AS_INITIALIZE_FRAMEWORK_MANUALLY or from the default initialization point
* (currently +load) otherwise.
* AS_INITIALIZE_FRAMEWORK_MANUALLY or otherwise from the default initialization point
* (currently a static constructor, called before main()).
*/
- (void)textureDidInitialize;

Expand Down
2 changes: 1 addition & 1 deletion Source/ASDisplayNode.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ + (void)initialize
}

#if !AS_INITIALIZE_FRAMEWORK_MANUALLY
+ (void)load
__attribute__((constructor)) static void ASLoadFrameworkInitializer(void)
{
ASInitializeFrameworkMainThread();
Copy link
Member

@nguyenhuy nguyenhuy Mar 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Agree that using static attribute initializer is the best option here.

We should update the documentation of -[ASConfigurationDelegate textureDidInitialize] (here) to state that it's called "pre-main" instead of in a +load method.

@Adlai-Holler @maicki Do you know if -textureDidInitialize is useful for any clients? If it can be removed, and if we can shut down ASExperimentalLayerDefaults experiment (i.e gather allowsGroupOpacityFromUIKitOrNil and allowsEdgeAntialiasingFromUIKitOrNil on demand instead), then we can remove ASInitializeFrameworkMainThread() completely and reduce pre-main time of every app by a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching that, @nguyenhuy! I noticed that comment when I was debugging this but forgot to update it with this patch. I'll push an update in a minute.

Copy link
Member

Choose a reason for hiding this comment

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

Awesome, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

We do rely on -textureDidInitialize currently so I don't want to get rid of it right now but let me think of how we can remove our reliance on it.

}
Expand Down
4 changes: 2 additions & 2 deletions Source/Private/ASTipsController.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ @implementation ASTipsController

#pragma mark - Singleton

+ (void)load
Copy link
Member

Choose a reason for hiding this comment

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

While we're on this, can we wrap this initializer in the AS_ENABLE_TIPS compiler flag and get back a tiny bit of pre-main time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the code in this file is wrapped in #if AS_ENABLE_TIPS already, so I think we're good here.

Copy link
Member

Choose a reason for hiding this comment

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

👍 Good call. I missed that.

__attribute__((constructor)) static void ASLoadTipsControllerNotification(void)
{
[NSNotificationCenter.defaultCenter addObserver:self.shared
[NSNotificationCenter.defaultCenter addObserver:ASTipsController.shared
selector:@selector(windowDidBecomeVisibleWithNotification:)
name:UIWindowDidBecomeVisibleNotification
object:nil];
Expand Down