Fix duplicate component tree creation in CKRenderWithSizeSpecComponent

Summary: - Yoga might run a double measurement on a component. - `CKRenderWithSizeSpecComponent` calls `buildComponentTree:` on its children in the `measureChild` method. - Without this diff, in case of a double measurement we could potentially create the component tree multiple times which is expensive - Now we cache the components that have been measured and don't create then again in case of a double measurement Reviewed By: kevin0571 Differential Revision: D9882959 fbshipit-source-id: f40093b00a0a7a084ef6f8d53719640d7da34938
facebook · Sep 18, 2018 · 907a5d0 · 907a5d0
1 parent 7a560cc
commit 907a5d0
Showing 1 changed file with 42 additions and 35 deletions.
diff --git a/ComponentKit/Core/Render/CKRenderWithSizeSpecComponent.mm b/ComponentKit/Core/Render/CKRenderWithSizeSpecComponent.mm
@@ -30,6 +30,7 @@
 @implementation CKRenderWithSizeSpecComponent {
   __weak CKRenderTreeNodeWithChildren *_node;
   std::unique_ptr<CKRenderWithSizeSpecComponentParameters> _parameters;
+  NSHashTable<CKComponent *> *_measuredComponents;
 #if CK_ASSERTIONS_ENABLED
   NSMutableSet *_renderedChildrenSet;
 #endif
@@ -51,23 +52,57 @@ + (instancetype)newRenderComponentWithView:(const CKComponentViewConfiguration &
                          isLayoutComponent:(BOOL)isLayoutComponent
 {
   auto const c = [super newRenderComponentWithView:view size:size isLayoutComponent:isLayoutComponent];
-#if CK_ASSERTIONS_ENABLED
   if (c) {
+    c->_measuredComponents = [NSHashTable weakObjectsHashTable];
+#if CK_ASSERTIONS_ENABLED
     c->_renderedChildrenSet = [NSMutableSet new];
-  }
 #endif
+  }
   return c;
 }
 
+- (void)buildComponentTree:(id<CKTreeNodeWithChildrenProtocol>)parent
+            previousParent:(id<CKTreeNodeWithChildrenProtocol>)previousParent
+                    params:(const CKBuildComponentTreeParams &)params
+                    config:(const CKBuildComponentConfig &)config
+            hasDirtyParent:(BOOL)hasDirtyParent
+{
+  if (!_node) {
+    auto const node = [[CKRenderTreeNodeWithChildren alloc]
+                       initWithComponent:self
+                       parent:parent
+                       previousParent:previousParent
+                       scopeRoot:params.scopeRoot
+                       stateUpdates:params.stateUpdates];
+    _node = node;
+
+    // Update the `hasDirtyParent` param for Faster state/props updates.
+    if (!hasDirtyParent && CKRender::hasDirtyParent(node, previousParent, params, config)) {
+      hasDirtyParent = YES;
+    }
+
+    auto const previousParentForChild = (id<CKTreeNodeWithChildrenProtocol>)[previousParent childForComponentKey:[node componentKey]];
+    _parameters = std::make_unique<CKRenderWithSizeSpecComponentParameters>(previousParentForChild,
+                                                                            params,
+                                                                            config,
+                                                                            hasDirtyParent);
+  }
+}
+
+
 - (CKComponentLayout)measureChild:(CKComponent *)child
                   constrainedSize:(CKSizeRange)constrainedSize
              relativeToParentSize:(CGSize)parentSize {
   CKAssert(_parameters.get() != nullptr, @"measureChild called outside layout calculations");
-  [child buildComponentTree:_node
-             previousParent:_parameters->previousParentForChild
-                     params:_parameters->params
-                     config:_parameters->config
-             hasDirtyParent:_parameters->hasDirtyParent];
+  if (![_measuredComponents containsObject:child]) {
+    [child buildComponentTree:_node
+               previousParent:_parameters->previousParentForChild
+                       params:_parameters->params
+                       config:_parameters->config
+               hasDirtyParent:_parameters->hasDirtyParent];
+    [_measuredComponents addObject:child];
+  }
+
 #if CK_ASSERTIONS_ENABLED
   [_renderedChildrenSet addObject:child];
 #endif
@@ -80,7 +115,6 @@ - (CKComponentLayout)computeLayoutThatFits:(CKSizeRange)constrainedSize
                           restrictedToSize:(const CKComponentSize &)size
                       relativeToParentSize:(CGSize)parentSize
 {
-  [_node reset];
   auto const layout = [self render:_node.state constrainedSize:constrainedSize restrictedToSize:size relativeToParentSize:parentSize];
 #if CK_ASSERTIONS_ENABLED
   checkIfAllChildrenComponentHaveBeenAddedToComponentTree(layout,_renderedChildrenSet);
@@ -105,33 +139,6 @@ - (CKComponentLayout)render:(id)state
   return {};
 }
 
-- (void)buildComponentTree:(id<CKTreeNodeWithChildrenProtocol>)parent
-            previousParent:(id<CKTreeNodeWithChildrenProtocol>)previousParent
-                    params:(const CKBuildComponentTreeParams &)params
-                    config:(const CKBuildComponentConfig &)config
-            hasDirtyParent:(BOOL)hasDirtyParent
-{
-  if (!_node) {
-    auto const node = [[CKRenderTreeNodeWithChildren alloc]
-                       initWithComponent:self
-                       parent:parent
-                       previousParent:previousParent
-                       scopeRoot:params.scopeRoot
-                       stateUpdates:params.stateUpdates];
-    _node = node;
-
-    // Update the `hasDirtyParent` param for Faster state/props updates.
-    if (!hasDirtyParent && CKRender::hasDirtyParent(node, previousParent, params, config)) {
-      hasDirtyParent = YES;
-    }
-
-    _parameters = std::make_unique<CKRenderWithSizeSpecComponentParameters>((id<CKTreeNodeWithChildrenProtocol>)[previousParent childForComponentKey:[_node componentKey]],
-                                                                            params,
-                                                                            config,
-                                                                            hasDirtyParent);
-  }
-}
-
 - (void)dealloc
 {
   _parameters = nullptr;