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

Reduce copying in ASTextNode2 stack #1065

Merged
merged 8 commits into from
Aug 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
- Optimize display node accessibility by not creating attributed & non-attributed copies of hint, label, and value. [Adlai Holler](https://github.com/Adlai-Holler)
- Add an experimental feature that reuses CTFramesetter objects in ASTextNode2 to improve performance. [Adlai Holler](https://github.com/Adlai-Holler)
- Add NS_DESIGNATED_INITIALIZER to ASViewController initWithNode: [Michael Schneider](https://github.com/maicki) [#1054](https://github.com/TextureGroup/Texture/pull/1054)
- Optimize text stack by removing unneeded copying. [Adlai Holler](https://github.com/Adlai-Holler)

## 2.7
- Fix pager node for interface coalescing. [Max Wang](https://github.com/wsdwsd0829) [#877](https://github.com/TextureGroup/Texture/pull/877)
Expand Down
18 changes: 13 additions & 5 deletions Source/ASTextNode2.mm
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,17 @@ - (CGSize)calculateSizeThatFits:(CGSize)constrainedSize

ASLockScopeSelf();

ASTextContainer *container = [_textContainer copy];
NSAttributedString *attributedText = self.attributedText;
container.size = constrainedSize;
ASTextContainer *container;
if (!CGSizeEqualToSize(container.size, constrainedSize)) {
container = [_textContainer copy];
container.size = constrainedSize;
[container makeImmutable];
} else {
container = _textContainer;
}
[self _ensureTruncationText];

NSMutableAttributedString *mutableText = [attributedText mutableCopy];
NSMutableAttributedString *mutableText = [_attributedText mutableCopy];
[self prepareAttributedString:mutableText];
ASTextLayout *layout = [ASTextNode2 compatibleLayoutWithContainer:container text:mutableText];

Expand Down Expand Up @@ -365,9 +370,12 @@ - (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer
{
ASLockScopeSelf();
[self _ensureTruncationText];

// Unlike layout, here we must copy the container since drawing is asynchronous.
ASTextContainer *copiedContainer = [_textContainer copy];
copiedContainer.size = self.bounds.size;
NSMutableAttributedString *mutableText = [self.attributedText mutableCopy] ?: [[NSMutableAttributedString alloc] init];
[copiedContainer makeImmutable];
NSMutableAttributedString *mutableText = [_attributedText mutableCopy] ?: [[NSMutableAttributedString alloc] init];

[self prepareAttributedString:mutableText];

Expand Down
3 changes: 3 additions & 0 deletions Source/Private/TextExperiment/Component/ASTextLayout.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ AS_EXTERN const CGSize ASTextContainerMaxSize;
/// Creates a container with the specified path. @param path The path.
+ (instancetype)containerWithPath:(nullable UIBezierPath *)path NS_RETURNS_RETAINED;

/// Mark this immutable, so you get free copies going forward.
- (void)makeImmutable;

/// The constrained size. (if the size is larger than ASTextContainerMaxSize, it will be clipped)
@property CGSize size;

Expand Down
45 changes: 31 additions & 14 deletions Source/Private/TextExperiment/Component/ASTextLayout.m
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#import <AsyncDisplayKit/ASTextLayout.h>

#import <AsyncDisplayKit/ASAssert.h>
#import <AsyncDisplayKit/ASConfigurationInternal.h>
#import <AsyncDisplayKit/ASTextUtilities.h>
#import <AsyncDisplayKit/ASTextAttribute.h>
Expand Down Expand Up @@ -134,26 +135,36 @@ - (instancetype)init {
return self;
}

- (id)copyWithZone:(NSZone *)zone {
ASTextContainer *one = [self.class new];
- (id)copyForced:(BOOL)forceCopy
{
dispatch_semaphore_wait(_lock, DISPATCH_TIME_FOREVER);
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why we use a semaphore as lock in this class?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was inherited from YYText where he did that. I actually really like it for small lock scopes. It's a lot faster than pthread mutex and there is a risk of priority inversion but it's much smaller since the waiting thread sleeps instead of spinning.

if (_readonly && !forceCopy) {
dispatch_semaphore_signal(_lock);
return self;
}

ASTextContainer *one = [self.class new];
one->_size = _size;
one->_insets = _insets;
one->_path = _path;
one->_exclusionPaths = _exclusionPaths.copy;
one->_exclusionPaths = [_exclusionPaths copy];
one->_pathFillEvenOdd = _pathFillEvenOdd;
one->_pathLineWidth = _pathLineWidth;
one->_verticalForm = _verticalForm;
one->_maximumNumberOfRows = _maximumNumberOfRows;
one->_truncationType = _truncationType;
one->_truncationToken = _truncationToken.copy;
one->_truncationToken = [_truncationToken copy];
one->_linePositionModifier = [(NSObject *)_linePositionModifier copy];
dispatch_semaphore_signal(_lock);
return one;
}

- (id)mutableCopyWithZone:(nullable NSZone *)zone {
return [self copyWithZone:zone];
- (id)copyWithZone:(NSZone *)zone {
return [self copyForced:NO];
}

- (id)mutableCopyWithZone:(NSZone *)zone {
return [self copyForced:YES];
}

- (void)encodeWithCoder:(NSCoder *)aCoder {
Expand Down Expand Up @@ -189,18 +200,25 @@ - (id)initWithCoder:(NSCoder *)aDecoder {
return self;
}

- (void)makeImmutable
{
dispatch_semaphore_wait(_lock, DISPATCH_TIME_FOREVER);
_readonly = YES;
dispatch_semaphore_signal(_lock);
}

#define Getter(...) \
dispatch_semaphore_wait(_lock, DISPATCH_TIME_FOREVER); \
__VA_ARGS__; \
dispatch_semaphore_signal(_lock);

#define Setter(...) \
if (_readonly) { \
@throw [NSException exceptionWithName:NSInternalInconsistencyException \
reason:@"Cannot change the property of the 'container' in 'ASTextLayout'." userInfo:nil]; \
return; \
} \
dispatch_semaphore_wait(_lock, DISPATCH_TIME_FOREVER); \
if (__builtin_expect(_readonly, NO)) { \
ASDisplayNodeFailAssert(@"Attempt to modify immutable text container."); \
dispatch_semaphore_signal(_lock); \
return; \
} \
__VA_ARGS__; \
dispatch_semaphore_signal(_lock);

Expand Down Expand Up @@ -407,11 +425,10 @@ + (ASTextLayout *)layoutWithContainer:(ASTextContainer *)container text:(NSAttri
if (lineRowsIndex) free(lineRowsIndex); \
return nil; }

text = text.mutableCopy;
container = container.copy;
container = [container copy];
if (!text || !container) return nil;
if (range.location + range.length > text.length) return nil;
container->_readonly = YES;
[container makeImmutable];
maximumNumberOfRows = container.maximumNumberOfRows;

// It may use larger constraint size when create CTFrame with
Expand Down