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

Try to remove global lock when initialising TextKit components #1455

Merged
merged 5 commits into from
Apr 22, 2019
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
4 changes: 3 additions & 1 deletion Schemas/configuration.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
"exp_image_downloader_priority",
"exp_text_drawing",
"exp_fix_range_controller",
"exp_oom_bg_dealloc_disable"
"exp_oom_bg_dealloc_disable",
"exp_transaction_operation_retain_cycle",
"exp_remove_textkit_initialising_lock"
]
}
}
Expand Down
1 change: 1 addition & 0 deletions Source/ASExperimentalFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) {
ASExperimentalFixRangeController = 1 << 12, // exp_fix_range_controller
ASExperimentalOOMBackgroundDeallocDisable = 1 << 13, // exp_oom_bg_dealloc_disable
ASExperimentalTransactionOperationRetainCycle = 1 << 14, // exp_transaction_operation_retain_cycle
ASExperimentalRemoveTextKitInitialisingLock = 1 << 15, // exp_remove_textkit_initialising_lock
ASExperimentalFeatureAll = 0xFFFFFFFF
};

Expand Down
3 changes: 2 additions & 1 deletion Source/ASExperimentalFeatures.mm
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
@"exp_text_drawing",
@"exp_fix_range_controller",
@"exp_oom_bg_dealloc_disable",
@"exp_transaction_operation_retain_cycle"]));
@"exp_transaction_operation_retain_cycle",
@"exp_remove_textkit_initialising_lock"]));
if (flags == ASExperimentalFeatureAll) {
return allNames;
}
Expand Down
2 changes: 1 addition & 1 deletion Source/Layout/ASAbsoluteLayoutSpec.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
typedef NS_ENUM(NSInteger, ASAbsoluteLayoutSpecSizing) {
/** The spec will take up the maximum size possible. */
ASAbsoluteLayoutSpecSizingDefault,
/** Computes a size for the spec that is the union of all childrens' frames. */
/** Computes a size for the spec that is the union of all children's frames. */
Copy link
Member

Choose a reason for hiding this comment

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

👍

ASAbsoluteLayoutSpecSizingSizeToFit,
};

Expand Down
2 changes: 1 addition & 1 deletion Source/Layout/ASLayoutElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ typedef NS_ENUM(NSUInteger, ASLayoutElementType) {
* The base implementation of -layoutThatFits:parentSize: does the following for you:
* 1. First, it uses the parentSize parameter to resolve the nodes's size (the one assigned to the size property).
* 2. Then, it intersects the resolved size with the constrainedSize parameter. If the two don't intersect,
* constrainedSize wins. This allows a component to always override its childrens' sizes when computing its layout.
* constrainedSize wins. This allows a component to always override its children's sizes when computing its layout.
* (The analogy for UIView: you might return a certain size from -sizeThatFits:, but a parent view can always override
* that size and set your frame to any size.)
* 3. It caches it result for reuse
Expand Down
4 changes: 2 additions & 2 deletions Source/Layout/ASStackLayoutElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ NS_ASSUME_NONNULL_BEGIN
@property (nonatomic) CGFloat spacingAfter;

/**
* @abstract If the sum of childrens' stack dimensions is less than the minimum size, how much should this component grow?
* @abstract If the sum of children's stack dimensions is less than the minimum size, how much should this component grow?
* This value represents the "flex grow factor" and determines how much this component should grow in relation to any
* other flexible children.
*/
@property (nonatomic) CGFloat flexGrow;

/**
* @abstract If the sum of childrens' stack dimensions is greater than the maximum size, how much should this component shrink?
* @abstract If the sum of children's stack dimensions is greater than the maximum size, how much should this component shrink?
* This value represents the "flex shrink factor" and determines how much this component should shink in relation to
* other flexible children.
*/
Expand Down
6 changes: 3 additions & 3 deletions Source/Layout/ASStackLayoutSpec.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ NS_ASSUME_NONNULL_BEGIN

- Suppose stacking direction is Vertical, min-width=100, max-width=300, min-height=200, max-height=500.
- All children are laid out with min-width=100, max-width=300, min-height=0, max-height=INFINITY.
- If the sum of the childrens' heights is less than 200, children with flexGrow are flexed larger.
- If the sum of the childrens' heights is greater than 500, children with flexShrink are flexed smaller.
- If the sum of the children's heights is less than 200, children with flexGrow are flexed larger.
- If the sum of the children's heights is greater than 500, children with flexShrink are flexed smaller.
Each child is shrunk by `((sum of heights) - 500)/(number of flexShrink-able children)`.
- If the sum of the childrens' heights is greater than 500 even after flexShrink-able children are flexed,
- If the sum of the children's heights is greater than 500 even after flexShrink-able children are flexed,
justifyContent determines how children are laid out.
*/
@interface ASStackLayoutSpec : ASLayoutSpec
Expand Down
4 changes: 2 additions & 2 deletions Source/Private/Layout/ASStackUnpositionedLayout.mm
Original file line number Diff line number Diff line change
Expand Up @@ -468,15 +468,15 @@ static void layoutFlexibleChildrenAtZeroSize(std::vector<ASStackLayoutSpecItem>
static CGFloat computeItemsStackDimensionSum(const std::vector<ASStackLayoutSpecItem> &items,
const ASStackLayoutSpecStyle &style)
{
// Sum up the childrens' spacing
// Sum up the children's spacing
const CGFloat childSpacingSum = std::accumulate(items.begin(), items.end(),
// Start from default spacing between each child:
items.empty() ? 0 : style.spacing * (items.size() - 1),
[&](CGFloat x, const ASStackLayoutSpecItem &l) {
return x + l.child.style.spacingBefore + l.child.style.spacingAfter;
});

// Sum up the childrens' dimensions (including spacing) in the stack direction.
// Sum up the children's dimensions (including spacing) in the stack direction.
const CGFloat childStackDimensionSum = std::accumulate(items.begin(), items.end(),
childSpacingSum,
[&](CGFloat x, const ASStackLayoutSpecItem &l) {
Expand Down
17 changes: 12 additions & 5 deletions Source/TextKit/ASTextKitContext.mm
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,17 @@ - (instancetype)initWithAttributedString:(NSAttributedString *)attributedString

{
if (self = [super init]) {
static AS::Mutex *mutex = NULL;
static dispatch_once_t onceToken;
static AS::Mutex *mutex;
// Concurrently initialising TextKit components crashes (rdar://18448377) so we use a global lock.
dispatch_once(&onceToken, ^{
mutex = new AS::Mutex();
if (!ASActivateExperimentalFeature(ASExperimentalRemoveTextKitInitialisingLock)) {
mutex = new AS::Mutex();
}
});

// Concurrently initialising TextKit components crashes (rdar://18448377) so we use a global lock.
AS::MutexLocker l(*mutex);
if (mutex != NULL) {
mutex->lock();
}
zhongwuzw marked this conversation as resolved.
Show resolved Hide resolved

__instanceLock__ = std::make_shared<AS::Mutex>();

Expand All @@ -65,6 +68,10 @@ - (instancetype)initWithAttributedString:(NSAttributedString *)attributedString
_textContainer.maximumNumberOfLines = maximumNumberOfLines;
_textContainer.exclusionPaths = exclusionPaths;
[_layoutManager addTextContainer:_textContainer];

if (mutex != NULL) {
mutex->unlock();
}
}
return self;
}
Expand Down
4 changes: 3 additions & 1 deletion Tests/ASConfigurationTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
ASExperimentalFixRangeController,
ASExperimentalOOMBackgroundDeallocDisable,
ASExperimentalTransactionOperationRetainCycle,
ASExperimentalRemoveTextKitInitialisingLock,
};

@interface ASConfigurationTests : ASTestCase <ASConfigurationDelegate>
Expand All @@ -59,7 +60,8 @@ + (NSArray *)names {
@"exp_text_drawing",
@"exp_fix_range_controller",
@"exp_oom_bg_dealloc_disable",
@"exp_transaction_operation_retain_cycle"
@"exp_transaction_operation_retain_cycle",
@"exp_remove_textkit_initialising_lock"
];
}

Expand Down
4 changes: 2 additions & 2 deletions docs/_docs/automatic-layout-containers.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ The following properties may be set on any node or `layoutSpec`s, but will only
</tr>
<tr>
<td><b><code>BOOL .flexGrow</code></b></td>
<td>If the sum of childrens' stack dimensions is less than the minimum size, should this object grow? Used when attached to a stack layout.</td>
<td>If the sum of children's stack dimensions is less than the minimum size, should this object grow? Used when attached to a stack layout.</td>
</tr>
<tr>
<td><b><code>BOOL .flexShrink</code></b></td>
<td>If the sum of childrens' stack dimensions is greater than the maximum size, should this object shrink? Used when attached to a stack layout.</td>
<td>If the sum of children's stack dimensions is greater than the maximum size, should this object shrink? Used when attached to a stack layout.</td>
</tr>
<tr>
<td><b><code>ASRelativeDimension .flexBasis</code></b></td>
Expand Down
4 changes: 2 additions & 2 deletions docs/_docs/layout2-layout-element-properties.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ nextPage: layout2-api-sizing.html
</tr>
<tr>
<td><b><code>CGFloat .style.flexGrow</code></b></td>
<td>If the sum of childrens' stack dimensions is less than the minimum size, should this object grow?</td>
<td>If the sum of children's stack dimensions is less than the minimum size, should this object grow?</td>
</tr>
<tr>
<td><b><code>CGFloat .style.flexShrink</code></b></td>
<td>If the sum of childrens' stack dimensions is greater than the maximum size, should this object shrink?</td>
<td>If the sum of children's stack dimensions is greater than the maximum size, should this object shrink?</td>
</tr>
<tr>
<td><b><code><a href="layout2-api-sizing.html#values-cgfloat-asdimension">ASDimension</a> .style.flexBasis</code></b></td>
Expand Down
6 changes: 3 additions & 3 deletions docs/appledoc/Classes/ASStackLayoutSpec.html
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,10 @@ <h2 class="subtitle subtitle-overview">Overview</h2>
<ul>
<li>Suppose stacking direction is Vertical, min-width=100, max-width=300, min-height=200, max-height=500.</li>
<li>All children are laid out with min-width=100, max-width=300, min-height=0, max-height=INFINITY.</li>
<li>If the sum of the childrens' heights is less than 200, children with flexGrow are flexed larger.</li>
<li>If the sum of the childrens' heights is greater than 500, children with flexShrink are flexed smaller.
<li>If the sum of the children's heights is less than 200, children with flexGrow are flexed larger.</li>
<li>If the sum of the children's heights is greater than 500, children with flexShrink are flexed smaller.
Each child is shrunk by <code>((sum of heights) - 500)/(number of flexShrink-able children)</code>.</li>
<li>If the sum of the childrens' heights is greater than 500 even after flexShrink-able children are flexed,
<li>If the sum of the children's heights is greater than 500 even after flexShrink-able children are flexed,
justifyContent determines how children are laid out.</li>
</ul>

Expand Down
2 changes: 1 addition & 1 deletion docs/appledoc/Constants/ASAbsoluteLayoutSpecSizing.html
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ <h4 class="method-subtitle">Constants</h4>
<dd>


<p>Computes a size for the spec that is the union of all childrens' frames.</p>
<p>Computes a size for the spec that is the union of all children's frames.</p>



Expand Down
2 changes: 1 addition & 1 deletion docs/appledoc/Protocols/ASLayoutElement.html
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ <h4 class="method-subtitle">Discussion</h4>
The base implementation of -layoutThatFits:parentSize: does the following for you:
1. First, it uses the parentSize parameter to resolve the nodes&rsquo;s size (the one assigned to the size property).
2. Then, it intersects the resolved size with the constrainedSize parameter. If the two don&rsquo;t intersect,
constrainedSize wins. This allows a component to always override its childrens' sizes when computing its layout.
constrainedSize wins. This allows a component to always override its children's sizes when computing its layout.
(The analogy for UIView: you might return a certain size from -sizeThatFits:, but a parent view can always override
that size and set your frame to any size.)
3. It caches it result for reuse</p>
Expand Down
4 changes: 2 additions & 2 deletions docs/appledoc/Protocols/ASStackLayoutElement.html
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ <h3 class="method-title"><code><a href="#//api/name/flexGrow">&nbsp;&nbsp;flexGr


<div class="method-subsection brief-description">
<p>If the sum of childrens' stack dimensions is less than the minimum size, how much should this component grow?
<p>If the sum of children's stack dimensions is less than the minimum size, how much should this component grow?
This value represents the &ldquo;flex grow factor&rdquo; and determines how much this component should grow in relation to any
other flexible children.</p>
</div>
Expand Down Expand Up @@ -264,7 +264,7 @@ <h3 class="method-title"><code><a href="#//api/name/flexShrink">&nbsp;&nbsp;flex


<div class="method-subsection brief-description">
<p>If the sum of childrens' stack dimensions is greater than the maximum size, how much should this component shrink?
<p>If the sum of children's stack dimensions is greater than the maximum size, how much should this component shrink?
This value represents the &ldquo;flex shrink factor&rdquo; and determines how much this component should shink in relation to
other flexible children.</p>
</div>
Expand Down