From 00751f64c7fab7c7b79425ddf1bc41e741e62077 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Thu, 2 Jun 2022 23:36:54 -0700 Subject: [PATCH] Fix out-of-order prop parsing deoptimization in TextView/Paragraph Summary: See also D36889794. This is a very similar idea, except the core problem is that BaseTextProps is accessing the same props as ViewProps; and for ParagraphProps parsing, it first defers to ViewProps' parser and then BaseTextProps. RawPropsParser is optimized to access the same props in the same order, *exactly once*, so if we access a prop out-of-order, or for a second time, that access and the next access are deoptimized. Paragraph/Text, in particular, were quite bad because we did this several times, and each out-of-order access requires scanning /all/ props. This fixes the issue, at least partially, by (1) pulling all the duplicate accesses to the beginning of BaseTextProps, and (2) accessing them all in the same order as ViewProps, relatively (some props are skipped, but that matters less). Practically what this means is that now, all of Props' accesses have a cost of O(1) for lookup, or a total of O(n) for all of them; each access is at the n+1 position in the internal RawPropsParser array, so each access is cheap. BaseTextProps' duplicate accesses, even though there are only 4 of them: (1) the first one scans the entire array until we reach the prop in question; (2) the next accesses require scans, but not whole-array scans, since they're in order. (3) The BaseTextProps accesses /after/ the duplicate accesses are all O(1). tl;dr is that before we had something like O(n*6) cost for BaseTextProps parsing and now it is O(n*2). Similar to my summary in the last diff: we may want to revisit the RawPropsParser API... but I want to tread gently there, and this gets us a large improvement without major, risky changes. Empirically, based on a couple of systraces, average time for a single UIManager::createNode called from JS thread, before this stack: 17us. After: 667ns (3% as long). On average, for every 60 createNode calls, we will save 1ms on the UI thread. The savings will be greater for certain screens that use many Views or Text nodes, and lesser for screens that use fewer of these components. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D36890072 fbshipit-source-id: 5d24b986c391d7bb158ed2f43d130a71960837d1 --- .../components/text/BaseTextProps.cpp | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/ReactCommon/react/renderer/components/text/BaseTextProps.cpp b/ReactCommon/react/renderer/components/text/BaseTextProps.cpp index af7d0734ff5..a5482e80aa8 100644 --- a/ReactCommon/react/renderer/components/text/BaseTextProps.cpp +++ b/ReactCommon/react/renderer/components/text/BaseTextProps.cpp @@ -22,25 +22,13 @@ static TextAttributes convertRawProp( TextAttributes const &defaultTextAttributes) { auto textAttributes = TextAttributes{}; - // Color + // Color (not accessed by ViewProps) textAttributes.foregroundColor = convertRawProp( context, rawProps, "color", sourceTextAttributes.foregroundColor, defaultTextAttributes.foregroundColor); - textAttributes.backgroundColor = convertRawProp( - context, - rawProps, - "backgroundColor", - sourceTextAttributes.backgroundColor, - defaultTextAttributes.backgroundColor); - textAttributes.opacity = convertRawProp( - context, - rawProps, - "opacity", - sourceTextAttributes.opacity, - defaultTextAttributes.opacity); // Font textAttributes.fontFamily = convertRawProp( @@ -166,6 +154,16 @@ static TextAttributes convertRawProp( sourceTextAttributes.isHighlighted, defaultTextAttributes.isHighlighted); + // In general, we want this class to access props in the same order + // that ViewProps accesses them in, so that RawPropParser can optimize + // accesses. This is both theoretical, and ParagraphProps takes advantage + // of this. + // In particular: accessibilityRole, opacity, and backgroundColor also + // are parsed first by ViewProps (and indirectly AccessibilityProps). + // However, since RawPropsParser will always store these props /before/ + // the unique BaseTextProps props, it is most efficient to parse these, in + // order, /after/ all of the other BaseTextProps, so that the RawPropsParser + // index rolls over only once instead of twice. textAttributes.accessibilityRole = convertRawProp( context, rawProps, @@ -173,6 +171,20 @@ static TextAttributes convertRawProp( sourceTextAttributes.accessibilityRole, defaultTextAttributes.accessibilityRole); + // Color (accessed in this order by ViewProps) + textAttributes.opacity = convertRawProp( + context, + rawProps, + "opacity", + sourceTextAttributes.opacity, + defaultTextAttributes.opacity); + textAttributes.backgroundColor = convertRawProp( + context, + rawProps, + "backgroundColor", + sourceTextAttributes.backgroundColor, + defaultTextAttributes.backgroundColor); + return textAttributes; }