-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Reduce usage of autorelease pools #968
Conversation
_lock.unlock(); | ||
for (auto ref : q) { | ||
// NOTE: Could check that retain count is 1 and retry later if not. | ||
CFRelease(ref); |
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 was silly for putting this in in the first place. Creating autorelease pools takes a lot of time and there's absolutely no chance of anything getting added to it in here 🤦♂️
} | ||
} | ||
return result; | ||
} |
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.
Unused
@@ -159,7 +159,7 @@ - (UICollectionViewLayoutAttributes *)layoutAttributesForElement:(ASCollectionEl | |||
} | |||
|
|||
// Use a set here because some items may span multiple pages | |||
NSMutableSet<UICollectionViewLayoutAttributes *> *result = [NSMutableSet set]; | |||
auto result = [NSMutableSet<UICollectionViewLayoutAttributes *> set]; |
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.
Use alloc init
in here too?
@@ -1959,7 +1959,7 @@ - (NSArray *)selectionRectsForRange:(ASTextRange *)range { | |||
range = [self _correctedRangeWithEdge:range]; | |||
|
|||
BOOL isVertical = _container.verticalForm; | |||
NSMutableArray *rects = [NSMutableArray array]; | |||
NSMutableArray *rects = [[NSMutableArray<NSValue *> alloc] init]; |
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.
We may should decide on a standard way to initialize these. E.g. we are now using different ways across the framework:
NSMutableArray *rects = [[NSMutableArray<NSValue *> alloc] init];
auto rects = [[NSMutableArray<NSValue *> alloc] init];
NSMutableArray<NSValue *> rects = [[NSMutableArray alloc] init];
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 completely agree. auto
isn't available in C, so I've been aiming for:
C++: auto rects = [[NSMutableArray<NSValue *> alloc] init];
C: NSMutableArray *rects = [[NSMutableArray<NSValue *> alloc] init];
We could introduce a macro that evaluates to __auto_type
like as_auto
or autotype
. The only drawback is we can't name the macro auto
because the old C keyword takes precedence over macros.
I think I'll add as_auto
in a follow up diff and see what people think.
Unfortunately, Foundation and UIKit are not going to be compiled with ARC any time soon.
So until then, calling methods like
+[NSArray array]
under ARC will not use the fast-return path and will result inalloc -> autorelease -> retain
to get a +1 instead of justalloc
. Plus draining the pool takes time! Why waste it?