bug: app renders unusably slowly

[shrouxm]

Description

This is seen most easily by trying to switch between different screens on the bottom tabs, but also applies when leaving form screens (e.g. Create Site), navigating to nested screens (e.g. navigating through nested data input screens), or causing screen updates (e.g. forms with updating helper text). The behavior is intermittent but has been getting overall worse over time.

[ltseng]

Still replicable on version 40 on iOS and Android

[shrouxm]

@ltseng could you provide a bit more info? i haven't really been seeing this issue and first reported it when the app was very new, so i'm not sure what form this bug takes anymore. specific questions:

• are there particular screen transitions that are slow, or could it be any screen transition?
• how slow is the screen transition? (less than one second but noticeable lag? more than one second? inconsistent?)
• how often does it happen? (every time? some of the time? rarely?)

[ltseng]

I specifically see this on iPhone when switching between the Home and Projects tab. I'm not sure if it's because the touch target is somehow smaller on the iPhone, but when there is a delay, it's consistently less than a second, and sometimes the screen fails to switch at all.

https://github.com/techmatters/terraso-mobile-client/assets/2730009/8e079230-16eb-48c2-aa5e-9bd5df0f575b

[shrouxm]

ok thank you for the extra info! from watching the video, i'm guessing it's because of missing the touch target

we could try to resolve by making the touch targets as big as possible (i.e. extend all the way to the top and bottom of the bar, and halfway to the next button in the bar horizontally). that could potentially introduce a different frustrating interaction where users accidentally change screens when trying to tap elements near the bottom bar, so there might be a trade-off there

it seems like it happens pretty infrequently at this point and the work to do the fix is straightforward but the results might be finnicky so i will update the issue to reflect that

[ltseng]

Yeah, I wouldn't consider the bug that I encountered to be a high-priority fix at all.

[ltseng]

I think @CourtneyLee333 has encountered an issue with seeing screens sometimes taking up to 5 or 10 seconds to respond though, so I'll try to get more details or a screenrecording from her whenever she is able to consistently replicate it.

[shrouxm]

that sounds like what i used to experience that originally prompted me to file this issue, but i stopped experiencing it at some point, so would be good to know if it's still happening on other devices!

[CourtneyLee333]

It happened on my ipad today, but my app was out of date. I'll keep an eye out for it.

[ltseng]

Still replicable on build 46 on iPhone and iPad, not happening on Android

[ltseng]

I'm seeing intermittent problems with the map and selecting locations as well on iPhone build 48

https://github.com/techmatters/terraso-mobile-client/assets/2730009/2dd83d53-e1a5-4efa-a7bb-7825b8dd2ad1

[ltseng]

Seems to be worsening. This is in build 59 on iPad.

https://github.com/techmatters/terraso-mobile-client/assets/2730009/1e34e071-287a-4480-84d4-a377b8fa90a4

[ltseng]

This is steadily getting worse for me in testing on both the iPad and iPhone. I'm bumping this up to a priority 2, but we definitely need to resolve this before our first actual release to users @paulschreiber

[shrouxm]

i experience this on my testing device as well (iphone 8). it is noticeably worse when battery saver mode is on, which leads me to suspect it's a performance problem, as silly as that seems given how simple the app so far is and how powerful ios devices are. in the past when screens have been slow and i've profiled the issue has been NativeBase, so if that is true of this issue as well i'm not sure what the right way to proceed would be but it makes me think we do need to do something to mitigate the NativeBase situation before release

[ltseng]

Thanks for the analysis! I checked my iPhone and am pretty sure Battery Saver is not enabled, but the performance is terrible regardless. I agree that this is a high-priority issue to solve before release though.

[CourtneyLee333]

Would that mitigation of NativeBase situation mean switching to a different component library?

[shrouxm]

@CourtneyLee333 unclear before looking into it

[shrouxm]

going to write what i'm doing as i do it:

took a profiling sample according to these instructions. the instructions also indicate how you can load the sample in chrome devtools to look yourself

i took the first profile this way because it was easiest, but the build is in dev mode so that introduces some inaccuracy. when i restrict the timeline to areas of high render activity, it seems pretty clear that nativebase (in particular, the usePropsResolution hook which is called on each render by each of their components) is the bottleneck

i'm tempted to just move on to thinking about solutions, but i'm going to take a profile using a different method with the app in a production build just to be thorough

[shrouxm]

update:

i am blocked on doing a production build profile so i started researching solutions

first thing i checked was whether the gluestack/nativebase folks had created an easy migration path from nativebase to gluestack, since they've deprecated nativebase in favor of gluestack

i found this migration page in the docs which links to this guide which is labeled "WIP"

there is a manual migration method and an auto migration method. i followed the (very sparse) instructions for the auto migration and it is completely broken. the manual migration is what it says on the tin: manually migrating each usage of a nativebase component to the equivalent usage of a gluestack component

at this point i would rather do a manual migration to bare react native than to a new component library from this team, but either way it is a lot of work. i will do some more thinking on if there's a better path forward

[shrouxm]

update:

looking around the code a bit more, i am curious to check if it's a compound problem of nativebase components rendering slowly, and we're re-rendering the components too often. i will try to do some profiling/experiments in this vein

[shrouxm]

update:

another potential thing to look into: React Navigation doesn't use React's normal lifecycle methods to manage screens that stop being visible, so my understanding from their docs is that all of the screens in the stack will be processing/rendering even when they are not the top of the stack. it might be helpful to take better advantage of these lifecycle events

[shrouxm]

i did a write up of different theories on what might be the issue for the callstack folks who are looking into this, copying here to keep the info in one place:

the 4 potential bottlenecks i've identified are:

• in the profile i took, every single NativeBase component spends the majority of render time resolving props. this is clearly causing some slowdown since e.g. RN without NativeBase wouldn't need to do any work at runtime resolving props, but i don't understand profiling RN apps well enough to know if it's actually a bottleneck
• there are certain components that based on my react knowledge look like they must be causing whole app re-renders very often. specifically the implementation of GeospatialContext.tsx and HomeScreen.tsx . but i don't understand why this would cause something like a multi-second lag on hitting a button to switch screens since that should be triggering a big re-render anyway
• as i'm sure you're aware igor, we aren't using React Navigation very well :upside_down_face:. i think this could also be causing performance issues. specifically, we have a relatively demanding dynamic home screen, which i think continues rendering as normal in the background regardless of what screen the user is on. also while i was poking around i was curious what screens were actually in the stack so i put a console.log of the navigation prop to run on every screen's render, and it seemed to be showing two HOME entries at the bottom of the stack at all times
• the simplest way to implement the satellite image previews in the site list on the home screen was to load a tiny mapbox map instance centered on the site, but i wonder if this is taking up a lot of resources

[CourtneyLee333]

Quick comment on your last item in the list above - I would say we can get rid of the tiny thumbnail if needed. I don't know how much good it's doing for users anyway. The objective was to eventually have a "featured image" the user could upload for the site, with the tiny map image being the default featured image.

[shrouxm]

@CourtneyLee333 ok, if that does end up making a difference, should the site card design just change so that the chips move all the way to the left into the space where the preview was?

[CourtneyLee333]

@shrouxm Yes, that will be good.

[ltseng]

Question: how much more time is CallStack going to spend on this issue, since there is a draft PR open?

Steve says that they think they have made a substantial impact with the new PR, and that they recommend it be merged. Waiting for Igor from CallStack to respond with a yes/no answer on whether they are done. There were also no profiling results posted in the PR.

[igorbej]

@ltseng @CourtneyLee333 @shrouxm Hi everyone 👋, I'm still looking into a few things related to performance, I plan to post an update in the PR with my findings and some profiling results this week (perhaps tomorrow), hope that's okay with y'all, cheers 😄

[CourtneyLee333]

Hi 👋, and thank you!

[shrouxm]

update: after faffing about with RN's recommended profiling options for a while, i tried using good old react devtools instead and that has been working like a charm and helped me quickly figure out what's going on in more detail:

my current understanding of the situation is that while we are doing some silly things which are causing unnecessary rendering activity, most of the problem is NativeBase. i can confirm this because profiling loading a new screen, even a very simple screen with only a few e.g. radio buttons, can take a noticeable amount of time due entirely to NativeBase just taking that long to render a few radio buttons

so! in terms of a path forward, i think the way to try to address this would be:

• apply some quick bandaid solutions to get the amount of components we're rendering down (igor's PR already makes progress in all of these areas, thank you!):
  • bugs in navigation code causing screens to be running in the background forever (igor found one and fixed it already, i've found another that needs a more complicated fix)
  • fix problem components that re-render unnecessarily because we're passing a new instance of the same prop over and over
  • be more lazy about loading components that aren't visible. igor has done this for the lists in the app, we can also do it for the tabs in the dashboards
  • memoize problem components that would benefit from it
• start replacing NativeBase components one by one by rewriting them in bare React Native. how easy this will be will depend on the components, but we can:
  • start with the layout components (Box, {V,H,}Stack, Row, Column) because they're: used everywhere, least complicated, least reliant on theming, and have the least accessibility considerations
  • do this by creating our own components with the same name, and then running a script to swap all the import statements of those components from NativeBase to our own component. typescript errors will then tell us which props we need to add to our component

we can check in at each step how much better it's looking. i'm not sure how to do that part less haphazardly since i still haven't figured out a way to run proper benchmarks for comparison (right now i'm just clicking through slow interactions in the app and recording the profile in devtools, then inspecting slow frames), but hopefully igor's profiling process will be illuminating in that regard! and otherwise the manual process i'm doing now is still totally fine at the level of performance we're working

i would guess i can get through all of the above steps in one of my work weeks if it's all i'm focusing on. i think it'd be a good idea to do this next week, since the perf issues are also making it harder to correctly implement other parts of the app (paul and i ran into this problem trying to fix the slope meter screen on iOS today)

[igorbej]

@CourtneyLee333 @ltseng @shrouxm Hey folks, I updated the description of https://github.com/techmatters/terraso-mobile-client/pull/787, I plan to add a couple more things next week depending on my availability, but I hope what I've written so far will be useful to you, cheers!

[ltseng]

Awesome, thank you! Wonderful news and writeup. I appreciate all the time you've spent on this, and the actionable insights you have provided.