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

Android performance regression: view flattening is broken (0.71) #37797

Open
FlorBosch opened this issue Jun 9, 2023 · 2 comments
Open

Android performance regression: view flattening is broken (0.71) #37797

FlorBosch opened this issue Jun 9, 2023 · 2 comments
Labels
Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Needs: Triage 🔍 Platform: Android Android applications.

Comments

@FlorBosch
Copy link

FlorBosch commented Jun 9, 2023

Description

After updating from React Native 0.67 to 0.71 (still using the old architecture), we noticed our view hierarchy has grown significantly.
While debugging the issue, we noticed there now seems to be a 1:1 relations ship between React components and actual views on the screen.

The issue started happening after the introduction of accessibilityValue (e8739e9) and accessibilityState (98d84e5).
Both objects (even if empty) are passed into all components, and, as a result, seem to be skipped while flattening the hierarchy.

On the positive side, this issue seems to be (accidentally?) fixed in 0.72 already: 3681df2. Instead of always passing the accessibility value object, it's now only passed in if one of the values is actually present.

For the time being, we managed to restore view flattening by applying the following patch:

diff --git a/node_modules/react-native/Libraries/Components/View/View.js b/node_modules/react-native/Libraries/Components/View/View.js
index 8ef1f81..c0deb96 100644
--- a/node_modules/react-native/Libraries/Components/View/View.js
+++ b/node_modules/react-native/Libraries/Components/View/View.js


@@ -66,20 +66,39 @@ const View: React.AbstractComponent<
     const _accessibilityLabelledBy =
       ariaLabelledBy?.split(/\s*,\s*/g) ?? accessibilityLabelledBy;
 
-    const _accessibilityState = {
-      busy: ariaBusy ?? accessibilityState?.busy,
-      checked: ariaChecked ?? accessibilityState?.checked,
-      disabled: ariaDisabled ?? accessibilityState?.disabled,
-      expanded: ariaExpanded ?? accessibilityState?.expanded,
-      selected: ariaSelected ?? accessibilityState?.selected,
-    };
+    let _accessibilityState;
+    if (
+      accessibilityState != null ||
+      ariaBusy != null ||
+      ariaChecked != null ||
+      ariaDisabled != null ||
+      ariaExpanded != null ||
+      ariaSelected != null
+    ) {
+      _accessibilityState = {
+        busy: ariaBusy ?? accessibilityState?.busy,
+        checked: ariaChecked ?? accessibilityState?.checked,
+        disabled: ariaDisabled ?? accessibilityState?.disabled,
+        expanded: ariaExpanded ?? accessibilityState?.expanded,
+        selected: ariaSelected ?? accessibilityState?.selected,
+      };
+    }
 
-    const _accessibilityValue = {
-      max: ariaValueMax ?? accessibilityValue?.max,
-      min: ariaValueMin ?? accessibilityValue?.min,
-      now: ariaValueNow ?? accessibilityValue?.now,
-      text: ariaValueText ?? accessibilityValue?.text,
-    };
+    let _accessibilityValue;
+    if (
+      accessibilityValue != null ||
+      ariaValueMax != null ||
+      ariaValueMin != null ||
+      ariaValueNow != null ||
+      ariaValueText != null
+    ) {
+      _accessibilityValue = {
+        max: ariaValueMax ?? accessibilityValue?.max,
+        min: ariaValueMin ?? accessibilityValue?.min,
+        now: ariaValueNow ?? accessibilityValue?.now,
+        text: ariaValueText ?? accessibilityValue?.text,
+      };
+    }

React Native Version

0.71.10

Output of npx react-native info

System:
    OS: macOS 13.0
    CPU: (10) arm64 Apple M1 Pro
    Memory: 139.69 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.14.0 - ~/.nvm/versions/node/v16.14.0/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.3.1 - ~/.nvm/versions/node/v16.14.0/bin/npm
    Watchman: 2023.05.15.00 - /opt/homebrew/bin/watchman
  Managers:
    CocoaPods: 1.12.0 - /Users/lucastwisk/.rbenv/shims/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 22.1, iOS 16.1, macOS 13.0, tvOS 16.1, watchOS 9.1
    Android SDK: Not Found
  IDEs:
    Android Studio: 2022.2 AI-222.4459.24.2221.9971841
    Xcode: 14.1/14B47b - /usr/bin/xcodebuild
  Languages:
    Java: 11.0.14 - /Users/lucastwisk/.jenv/shims/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 18.2.0 => 18.2.0
    react-native: 0.71.8 => 0.71.8
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Steps to reproduce

Here's a Snack containing a reproduction project: https://snack.expo.dev/@lctwisk/flattening-repro-project

In this project, we've added an example component, taken from the view flattening docs.

We've used the Android layout inspector to retrieve the view hierarchy.
First without the fix:
view_hierarchy_without_fix.txt
And after, with the fix applied:
view_hierarchy_with_fix.txt

Even for a tiny component, there's a clear difference in the size of the view hierarchy.

Snack, code example, screenshot, or link to a repository

https://snack.expo.dev/@lctwisk/flattening-repro-project

@github-actions github-actions bot added Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: Android Android applications. labels Jun 9, 2023
@cortinico cortinico added the Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. label Jun 9, 2023
@NickGerleman
Copy link
Contributor

Added a request to back port the fix to 0.71 reactwg/react-native-releases#70 (comment).

@MBischofer
Copy link

For those of you that came across this thread like I did, this issue also applies if React Native was being too aggressive with the view flattening on Android .

Updating to RN 0.71.12 fix the issue for me 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Needs: Triage 🔍 Platform: Android Android applications.
Projects
None yet
Development

No branches or pull requests

4 participants