Try to remove global lock when initialising TextKit components (#1455)

* Try to remove global lock when initialising TextKit components * Adding experiment flag to dis/enable lock of textkit component * Adding tests * code clean fix typo Make remove lock optional Keep locks code clean
TextureGroup · Apr 22, 2019 · 5aef12d · 5aef12d
1 parent 3120c4e
commit 5aef12d
Show file tree

Hide file tree

Showing 16 changed files with 41 additions and 28 deletions.
diff --git a/Schemas/configuration.json b/Schemas/configuration.json
@@ -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"
                 ]
     		}
 		}

diff --git a/Source/ASExperimentalFeatures.h b/Source/ASExperimentalFeatures.h
@@ -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
 };
 

diff --git a/Source/ASExperimentalFeatures.mm b/Source/ASExperimentalFeatures.mm
@@ -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;
   }

diff --git a/Source/Layout/ASAbsoluteLayoutSpec.h b/Source/Layout/ASAbsoluteLayoutSpec.h
@@ -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. */
   ASAbsoluteLayoutSpecSizingSizeToFit,
 };
 

diff --git a/Source/Layout/ASLayoutElement.h b/Source/Layout/ASLayoutElement.h
@@ -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

diff --git a/Source/Layout/ASStackLayoutElement.h b/Source/Layout/ASStackLayoutElement.h
@@ -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.
  */

diff --git a/Source/Layout/ASStackLayoutSpec.h b/Source/Layout/ASStackLayoutSpec.h
@@ -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

diff --git a/Source/Private/Layout/ASStackUnpositionedLayout.mm b/Source/Private/Layout/ASStackUnpositionedLayout.mm
@@ -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) {

diff --git a/Source/TextKit/ASTextKitContext.mm b/Source/TextKit/ASTextKitContext.mm
@@ -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();
+    }
 
     __instanceLock__ = std::make_shared<AS::Mutex>();
 
@@ -65,6 +68,10 @@ - (instancetype)initWithAttributedString:(NSAttributedString *)attributedString
     _textContainer.maximumNumberOfLines = maximumNumberOfLines;
     _textContainer.exclusionPaths = exclusionPaths;
     [_layoutManager addTextContainer:_textContainer];
+
+    if (mutex != NULL) {
+      mutex->unlock();
+    }
   }
   return self;
 }

diff --git a/Tests/ASConfigurationTests.mm b/Tests/ASConfigurationTests.mm
@@ -33,6 +33,7 @@
   ASExperimentalFixRangeController,
   ASExperimentalOOMBackgroundDeallocDisable,
   ASExperimentalTransactionOperationRetainCycle,
+  ASExperimentalRemoveTextKitInitialisingLock,
 };
 
 @interface ASConfigurationTests : ASTestCase <ASConfigurationDelegate>
@@ -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"
   ];
 }
 

diff --git a/docs/_docs/automatic-layout-containers.md b/docs/_docs/automatic-layout-containers.md
@@ -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>

diff --git a/docs/_docs/layout2-layout-element-properties.md b/docs/_docs/layout2-layout-element-properties.md
@@ -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>

diff --git a/docs/appledoc/Classes/ASStackLayoutSpec.html b/docs/appledoc/Classes/ASStackLayoutSpec.html
@@ -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>
 

diff --git a/docs/appledoc/Constants/ASAbsoluteLayoutSpecSizing.html b/docs/appledoc/Constants/ASAbsoluteLayoutSpecSizing.html
@@ -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>
 
 
 

diff --git a/docs/appledoc/Protocols/ASLayoutElement.html b/docs/appledoc/Protocols/ASLayoutElement.html
@@ -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>

diff --git a/docs/appledoc/Protocols/ASStackLayoutElement.html b/docs/appledoc/Protocols/ASStackLayoutElement.html
@@ -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>
@@ -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>