Performance improvements #220

fortmarek · 2023-02-01T16:57:42Z

Description

restyle is in its essence a runtime library to achieve better DX. Ideally, restyle should have no overhead and no performance implications. Since, as things stand, we need to run code in the runtime to convert theme values to RN style, there will be some overhead.

However, we should ensure that the overhead is as small as possible. We've already made some performance improvements in the past but we still found there's a noticeable performance drop when using restyle.

Using the fixture inside the flash-list repo in this branch, we found out restyle still incurs performance drop. Measuring on iPad Pro 2019, we found the FPS drops by ~7 % and list load time takes 12 % longer. Considering these numbers, we decided to investigate further why restyle still has such an impact on performance.

This PR includes multiple small performance improvements but the major ones have been:

Migrating from reduce to for-loop in filterRestyleProps. Creating many objects on every useRestyle render is expensive. for-loop with updating objects in-place instead of copying and creating new ones is much more efficient.
breakpoints are now optional. Querying window dimensions and computing responsive values has additional performance cost. If developers are not using breakpoints, we should not compute these values. In the future, we should consider creating responsive variants of restyle components, so the responsive computations can be done really only for those components that need them.
Style memoization. We currently have useMemo inside useRestyle to ensure a component does not need to recompute its style on every render. We decided to take this step further and memoize computed style inside the theme object. This means a style does not have to be recomputed if any other component has already computed the same theme value.

Measurements

All the following measurements have been done in this flash-list branch on iPad Pro 2019:

FPS

Without restyle:
118.2, 116, 116.6, 117.1, 116.7, 115.7, 116.2, 115.7, 116.1, 115.3, 115.9
Avg: 116.32

With restyle:
109.7, 108.3, 106.8, 108.5, 110.4, 110.2, 108.0, 108.8, 107.0, 110.1
Avg: 108.78
Perf drop: 7 %

With this PR's optimizations:
115.7, 115.2, 116.1, 113.8, 115.1, 115.4, 115.0, 114.7, 111.1, 113.8, 113.1
Avg: 114.45
Perf drop: 1.5 %
Relative perf improvement: 79 %

List load time

Without restyle:
61, 60, 62, 59, 61, 59, 61, 64, 57, 61
Avg: 60.5

With restyle:
71, 67, 71, 69, 70, 64, 79, 61, 71, 65
Avg: 68.8
Performance drop: 12 %

With optimizations:
64, 65, 66, 60, 64, 64, 67, 63, 66, 64
Average: 64.3
Performance drop: 6 %
Relative performance improvement: 50 %

Tracer

We have created this helper class called tracer to get granular numbers on how much time JS spends on evaluating useRestyle

Time spent on useRestyle in ms for ~30000 calls:
Without optimizations:
563, 533, 515, 618, 583
Average: 562.4 ms

With optimizations:
270, 253, 242, 217, 262
Average: 248.8 ms
Performance improvement: 56 %

From this, more than 50 % is spent on filterRestyleProps method. I don't see how we can effectively get rid of that for loop since we need it to computed the serializedRestyleProps that determine whether a style needs to be recomputed or not.

Summary

The changes in this PR lead to large performance improvements where the FPS when scrolling in flash-list has improved by almost 80 %. In the FlashList fixture, using restyle still incurs ~1.5 % FPS drop. We expect this number to be smaller in more complicated lists as restyle would constitute smaller part of all the work done on every render.

The total time spent on useRestyle has dropped by more than 50 %, the list load time has improved by 50 %.

There is still some performance drop attached to useRestyle even with these new changes but the impact should be now minimal.

Reviewers’ hat-rack 🎩

Check out fixture, everything should still work as expected.
If you are interested, additional measurements would be much appreciated. Let me know and I can give you some additional guidance how to do so!

Checklist

I have added a changelog entry following the Keep a Changelog guidelines.

fortmarek · 2023-02-14T11:34:53Z

src/types.ts

@@ -22,7 +38,7 @@ export interface KnownBaseTheme {
  spacing: {
    [key: string]: number | string;
  };
-  breakpoints: {
+  breakpoints?: {


breakpoints are no longer required to be defined in the theme. If this value is not defined, we will skip computing responsive values.

fortmarek · 2023-02-14T11:38:58Z

src/utilities/index.ts

+export {getValueForScreenSize} from './getValueForScreenSize';
+export {getResponsiveValue} from './getResponsiveValue';
+export {isResponsiveObjectValue} from './isResponsiveObjectValue';
+export {getThemeValue} from './getThemeValue';


I split the responsiveHelpers file into multiple ones as it was getting too long

fortmarek · 2023-02-14T11:40:03Z

src/hooks/useRestyle.ts

-  const props = omitPropertiesMap.variant
-    ? {variant: 'defaults', ...componentProps}
-    : componentProps;
-  return getKeys(props).reduce(


This reduce was extremely ineffective. filterRestyleProps is called on every render of any restyle component – that is very often.

src/utilities/isResponsiveObjectValue.ts

mattfrances

Tophat and code looks good!

src/createRestyleFunction.ts

src/hooks/useRestyle.ts

naqvitalha · 2023-02-17T14:53:48Z

documentation/docs/fundamentals/breakpoints.md

@@ -19,4 +19,4 @@ const theme = createTheme({
 });
 ```

-See the [Responsive Values](/fundamentals/responsive-values) section to see how these can be used.
+See the [Responsive Values](/fundamentals/responsive-values) section to see how these can be used. Defining `breakpoints` is optional and we recommend defining it only if you plan to use them due to the small performance hit using responsive values incur.


Is it possible to mention how much slower it'll get? It'll make it easier to decide whether or not to use this feature

naqvitalha · 2023-02-17T15:04:28Z

src/utilities/getValueForScreenSize.ts

+  ResponsiveBaseTheme,
+} from '../types';
+
+export const getValueForScreenSize = <Theme extends ResponsiveBaseTheme, TVal>({


It would be great if we can mention the purpose of some of the methods. It's a little difficult to understand the PR otherwise.

These are not new methods and I have only moved them to separate files. But yeah, I can add some descriptions.

fortmarek · 2023-02-22T11:35:44Z

@naqvitalha, please, have another look and afterwards we can merge this.

naqvitalha · 2023-02-22T20:12:25Z

src/composeRestyleFunctions.ts

+    const options = {theme, dimensions};
+    // We know props won't have prototype properties
+    // eslint-disable-next-line guard-for-in
+    for (const key in props) {


Can this break if objects inherit some default properties?
For e.g, if object prototype includes some default values?

Technically, it might be possible, although I don't expect to happen. The extra if should be fine, added it

naqvitalha

LGTM! Very excited about this!

craftzdog · 2023-06-08T10:31:59Z

Great work @fortmarek!
Can useResponsiveProp also be improved as it uses useWindowDimensions?
It affects a lot when the device orientation is changed on tablets.

craftzdog · 2023-06-08T12:15:49Z

I should just avoid using useResponsiveProp 😅

fortmarek force-pushed the performance/improvements branch 3 times, most recently from 47cc04b to cf6c06a Compare February 14, 2023 10:28

fortmarek changed the title ~~Performance/improvements~~ Performance improvements Feb 14, 2023

fortmarek requested review from mattfrances and davebcn87 February 14, 2023 11:31

fortmarek marked this pull request as ready for review February 14, 2023 11:31

fortmarek commented Feb 14, 2023

View reviewed changes

fortmarek force-pushed the performance/improvements branch 2 times, most recently from 3bd20b6 to dd95404 Compare February 14, 2023 11:37

fortmarek commented Feb 14, 2023

View reviewed changes

ecreeth reviewed Feb 14, 2023

View reviewed changes

src/utilities/isResponsiveObjectValue.ts Outdated Show resolved Hide resolved

fortmarek force-pushed the performance/improvements branch from dd95404 to a5d2e98 Compare February 14, 2023 12:45

mattfrances approved these changes Feb 14, 2023

View reviewed changes

davebcn87 reviewed Feb 15, 2023

View reviewed changes

src/createRestyleFunction.ts Show resolved Hide resolved

src/createRestyleFunction.ts Outdated Show resolved Hide resolved

src/createRestyleFunction.ts Show resolved Hide resolved

src/hooks/useRestyle.ts Show resolved Hide resolved

davebcn87 approved these changes Feb 15, 2023

View reviewed changes

fortmarek force-pushed the performance/improvements branch from 0daffe3 to e769b94 Compare February 15, 2023 15:23

naqvitalha reviewed Feb 17, 2023

View reviewed changes

fortmarek force-pushed the performance/improvements branch from e769b94 to f929170 Compare February 22, 2023 11:34

fortmarek requested a review from naqvitalha February 22, 2023 11:35

naqvitalha reviewed Feb 22, 2023

View reviewed changes

fortmarek force-pushed the performance/improvements branch from f929170 to 8d21898 Compare February 23, 2023 08:24

Restyle performance improvements

3e7f596

fortmarek force-pushed the performance/improvements branch from 8d21898 to 3e7f596 Compare February 23, 2023 15:45

naqvitalha approved these changes Feb 23, 2023

View reviewed changes

fortmarek merged commit a1f37af into master Feb 23, 2023

fortmarek mentioned this pull request Mar 16, 2023

Custom themed components are losing spacing props after upgrade from 2.3.0 to 2.4.0 #239

Closed

2 tasks

gvarandas deleted the performance/improvements branch March 19, 2024 17:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Performance improvements #220

Performance improvements #220

fortmarek commented Feb 1, 2023 •

edited

Loading

fortmarek Feb 14, 2023

fortmarek Feb 14, 2023

fortmarek Feb 14, 2023

mattfrances left a comment

naqvitalha Feb 17, 2023

fortmarek Feb 22, 2023

naqvitalha Feb 17, 2023

fortmarek Feb 21, 2023

fortmarek Feb 22, 2023

fortmarek commented Feb 22, 2023

naqvitalha Feb 22, 2023

fortmarek Feb 23, 2023

naqvitalha left a comment

craftzdog commented Jun 8, 2023

craftzdog commented Jun 8, 2023

Performance improvements #220

Performance improvements #220

Conversation

fortmarek commented Feb 1, 2023 • edited Loading

Description

Measurements

FPS

List load time

Tracer

Summary

Reviewers’ hat-rack 🎩

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattfrances left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fortmarek commented Feb 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naqvitalha left a comment

Choose a reason for hiding this comment

craftzdog commented Jun 8, 2023

craftzdog commented Jun 8, 2023

fortmarek commented Feb 1, 2023 •

edited

Loading