Application Performance and UX


#8

I think we will start with the cold startup and mostly focus on Android, looking at the numbers.

Also this slowness of syncing looks very suspicious, but it might be mitigated by switching protocol parsing to status-go (but that needs to be confirmed).

And I guess we will start with replicating the conditions, making a test account with around ~120 chats and similar specs like Huawei M5. I think we have a bot that can make a decent history for these chatrooms.


#9

21

Here is a quick investigation

As you can see on the picture, we have one js thread and when a user presses a button, we:

  1. register re-frame event in a queue
  2. run all events in a queue
  3. calculate ALL active subscription functions (currently ~100!!)
  4. calculate all active render functions
  5. parse hiccup convert to js

Low hanging fruits

  • inspect all subs on performance
  • make more efficient subscriptions tree
  • find out why we calculate not visible render functions and subscriptions (i believe it’s because of react-navigation)
  • inspect events, optimize or split

Hight hanging

  • move events and subs in separate js thread, because of re-frame arch it should be doable, so in main thread will be only user action events
  • find out how realm impacts performance, because we need to convert message data many times, and save it encrypted in realm, i believe it would be much more efficient if we could store messages with status-go

Subsctiptions
Currently, we react on any change in app-db, and there maybe subscriptions with calculations, so we need to improve our subscriptions graph,
first node must be map only with keys which impact subscriptions, next, one subscription for each key in this map, and next subscriptions related to these keys

Events
re-frame has a solution for events. it suggests to split calculations in small functions, and we used this before, but for some reason, we decided to use our own solution with fx/merge where we merge all small event functions in a big one, and this may lead to performance issues, for example, if you’ve received a lot of messages you need to filter them, sort etc, save in realm, all in one function, maybe we could find a compromise here and split it for example for such events like messages

Screens rendering
Currently, we calculate all screens even if they are not displayed, maybe we could find a way when we could keep them in react but don’t calculate subscriptions and render functions for them

PS: 5 steps above happen for each event, for example when you type a text in the input field, for each char we calculate all subscriptions and views renderers
image


#10

I am not sure about this, subscriptions that take other subscriptions as input should only render if those subscriptions have changed afaik

that would be a nice effort, similar to events, I would be in favor of putting all subscriptions to first level keys in app-db in one file and use them as input for other more complex subscriptions.
such an effort would also include diagnosing poor design of app-db with top level keys that should rather be nested to reduce amount of recomputations

I disagree with splitting events, more on this in response to your other paragraph.

while I agree with this, the simplest and more efficient solution might actually end up being the current effort to move chat protocols to status-go. if status-go stores messages and returns them per chat and in order with a comprehensive API we could avoid the most expensive computations on status-react side.

already commented above, I think we are thinking roughly about the same thing

strongly disagree with that statement, fx/merge is about

  • composability
  • transactional events which cancels out if an error occurs instead of partially changing app state as it would happen with a small chain of events, they also avoid unnecessary recomputation of subscriptions.
  • the fx/merge actually often do little work and are mostly about returning a map of effects, which are going to do the actual work. for instance saving in realm is done outside of the event.

Keep in mind that there is overhead to dispatching events, events dispatch within another event only run next tick, but those dispatched at the same time all render during the same tick. After events are computed there is a check to app-db. fx/merged is saving a lot of time by merging changes to realm as well.

Now sorting and filtering the messages especially in big chats is a big deal and not really related to fx/merge, it will take 99% of the computation time regardless of how much fx/defn are merged within the event. So the actual effort should rather be put into finding a solution to that problem, either by offloading this work to another thread (but it has its own complications), using status-go for that, or putting more effort into alternative datastructures such as the ordered persistent hashmap which could be leveraged for more caching and less sorting.

(parenthesis re-frisk really doesn’t like custom datastructures and my repl is often wrecked by millions of lines of exceptions such as

                       java.util.concurrent.ThreadPoolExecutor$Worker.run  ThreadPoolExecutor.java:  624
                        java.util.concurrent.ThreadPoolExecutor.runWorker  ThreadPoolExecutor.java: 1149
                                                                      ...                               
                 clojure.core.async.impl.channels.ManyToManyChannel/fn/fn             channels.clj:   95
                                         clojure.core.async/do-alts/fn/fn                async.clj:  252
                                          clojure.core.async/ioc-alts!/fn                async.clj:  383
             clojure.core.async.impl.ioc-macros/run-state-machine-wrapped           ioc_macros.clj:  977
                     clojure.core.async.impl.ioc-macros/run-state-machine           ioc_macros.clj:  973
              taoensso.sente/-start-chsk-router!/fn/state-machine--auto--               sente.cljc: 1529
           taoensso.sente/-start-chsk-router!/fn/state-machine--auto--/fn               sente.cljc: 1529
                                    taoensso.sente/-start-chsk-router!/fn               sente.cljc: 1527
taoensso.sente/-start-chsk-router!/fn/state-machine--auto--/fn/inst-17988               sente.cljc: 1541
                                  re-frisk-sidecar.core/event-msg-handler                 core.clj:   70
                                                                      ...                               
                                       re-frisk-sidecar.core/eval18744/fn                 core.clj:  111
                                                       clojure.core/swap!                 core.clj: 2345
                                                                      ...                               
                                                     re-frisk.delta/apply               delta.cljc:   94
                                                 re-frisk.delta/apply-map               delta.cljc:   84
                                         re-frisk.delta/apply-map-changes               delta.cljc:   80
                                                      clojure.core/reduce                 core.clj: 6748
                                              clojure.core.protocols/fn/G            protocols.clj:   13
                                                clojure.core.protocols/fn            protocols.clj:   75
                                       clojure.core.protocols/iter-reduce            protocols.clj:   49
                                      re-frisk.delta/apply-map-changes/fn               delta.cljc:   80
                                                                      ...                               
                                                   clojure.core/update-in                 core.clj: 6092
                                                   clojure.core/update-in                 core.clj: 6106
                                                clojure.core/update-in/up                 core.clj: 6105
                                                       clojure.core/apply                 core.clj:  659
                                                                      ...                               
                                   re-frisk.delta/apply-map-changes/fn/fn               delta.cljc:   80
                                                     re-frisk.delta/apply               delta.cljc:   94
                                                 re-frisk.delta/apply-map               delta.cljc:   84
                                         re-frisk.delta/apply-map-changes               delta.cljc:   80
                                                      clojure.core/reduce                 core.clj: 6748
                                              clojure.core.protocols/fn/G            protocols.clj:   13
                                                clojure.core.protocols/fn            protocols.clj:   75
                                       clojure.core.protocols/iter-reduce            protocols.clj:   49
                                      re-frisk.delta/apply-map-changes/fn               delta.cljc:   80
                                                                      ...                               
                                                   clojure.core/update-in                 core.clj: 6092
                                                   clojure.core/update-in                 core.clj: 6106
                                                clojure.core/update-in/up                 core.clj: 6105
                                                       clojure.core/apply                 core.clj:  659
                                                                      ...                               
                                   re-frisk.delta/apply-map-changes/fn/fn               delta.cljc:   80
                                                     re-frisk.delta/apply               delta.cljc:   94
                                                 re-frisk.delta/apply-map               delta.cljc:   84
                                         re-frisk.delta/apply-map-changes               delta.cljc:   80
                                                      clojure.core/reduce                 core.clj: 6748
                                              clojure.core.protocols/fn/G            protocols.clj:   13
                                                clojure.core.protocols/fn            protocols.clj:   75
                                       clojure.core.protocols/iter-reduce            protocols.clj:   49
                                      re-frisk.delta/apply-map-changes/fn               delta.cljc:   80
                                                                      ...                               
                                                   clojure.core/update-in                 core.clj: 6092
                                                   clojure.core/update-in                 core.clj: 6106
                                                clojure.core/update-in/up                 core.clj: 6105
                                                       clojure.core/apply                 core.clj:  659
                                                                      ...                               
                                   re-frisk.delta/apply-map-changes/fn/fn               delta.cljc:   80
                                                     re-frisk.delta/apply               delta.cljc:   94
                                                 re-frisk.delta/apply-map               delta.cljc:   85
                                                                      ...                               
                                                       clojure.core/merge                 core.clj: 3033
                                                       clojure.core/merge                 core.clj: 3040
                                                     clojure.core/reduce1                 core.clj:  926
                                                     clojure.core/reduce1                 core.clj:  936
                                                    clojure.core/merge/fn                 core.clj: 3041
                                                        clojure.core/conj                 core.clj:   85
                                                                      ...                               
java.lang.IllegalArgumentException: Don't know how to create ISeq from: com.cognitect.transit.impl.TaggedValueImpl

I think relying more on local state especially for input fields would already save us a lot of subscriptions and events, we can pass a validation function for those fields that require some validation, it doesn’t have to be done through an event

Overall I would be very much in favor of having only one subscription per screen, which is composed of subscriptions with a focus on minimizing recomputations


#11

thanks Eric for your answer

yes, i wrote it not exactly how i meant, but anyway i tested it with 10x, and it’s from ~60 to ~100 subscriptions by one event are run when many screens are active, because of react-navigation when you leave the screen, subscriptions remain active for it

that’s true, but maybe we could find a way how to run them in a separate tick, just as a low hanging temporary solution, for example split them and dispatch-later

i’ll try to find a time once again to look at that

that would be great!


#12

We went through the excruciatingly difficult decision to ditch React Native and move to Swift and Kotlin code bases.

I cannot tell you how much I despise React Native. You cannot even construct a performant table view with 100 cells.

I wish you all the best as you try to speed up Status.


#13

Are they actually computed or just active? Also just like events some subs are fairly quick to compute. For instance if you just patch together a bunch of other subs it’s just going to be a few assoc


#15

I’m wondering if this is more an issue in how react-native is being used?

As someone who has mostly used react-dom, I know this can be an issue where rendering cells for all available data will lead to poor performance over just rendering the cells that come into view. There are solutions to deal with this issue like http://www.reactvirtualized.com/

Have you tried optimizing react’s rendering before migrating away?


#16

We should really get some data on the performance impact of subscriptions vs input params in child components

Here is a comparison I would like to try to verify some assumptions about subscriptions costs:

  • one huge chat subscription which includes the list of all messages down to author related informations and have all the data flowing down to child components (almost what we have now, our current implementation is more hybrid but would do the trick for now)
  • one minimalist chat subscription with a vector of message ids, each message component subscribing to a parametrized subscription that gets the data for the message, author component itself uses its own parametrized subscription.

Difficulty: find the proper way to measure perfs, rendering vs subscription computations

Assumptions:

  • In the first solution we might save a lot of subscriptions but initial subscription might be expensive. Changes in inputs might trigger lots of recomputations. Parents components would rerender when the child changes but this might actually be negligeable.
  • In the second solution there will be a lot of subscriptions but they might not be so expensive and rarely recalculated, also the subscriptions would only be computed when the messages are actually rendered so it might save lots of computations in big chats especially when scrolling is up, where the first solution recomputes everything.
  • having lots of subscriptions takes space but can save lots of time especially if they are rarely recomputed

The chat screen is definitely the place to experiment as it is were the most expensive operations occurs. While I would expect the one big subscription model to be best for other screens, I think in that case the small parametrized subscriptions might be better.


#17

This is what happens when user presses wallet tab , times maybe incorrect because it’s develop build and we have overhead for tracing tools, but this trace is for an empty account, for different conditions times might be different, for example if account has a lot of chat, messages , tokens etc, subscriptions calculation may take much more time

:arrow_forward: :event :navigate-to :wallet-stack 65.0 ms
:arrow_forward: :event/handler :wallet-stack 1.0 ms
:arrow_forward: :event/do-fx 44.0 ms
:arrow_forward: :sub/run :…ad-messages-number 0.0 ms
:arrow_forward: :sub/run :…/pending-requests 0.0 ms
:arrow_forward: :sub/run :get :accounts/login 0.0 ms
:arrow_forward: :sub/run :…/settings 1.0 ms
:arrow_forward: :sub/run :get :…/advanced? 0.0 ms
:arrow_forward: :sub/run :get :chats/loading? 0.0 ms
:arrow_forward: :sub/run :…settings/currency 1.0 ms
:arrow_forward: :sub/run :search/filter 0.0 ms
:arrow_forward: :sub/run :wallet 0.0 ms
:arrow_forward: :sub/run :…subs/contacts 0.0 ms
:arrow_forward: :sub/run :…chat.subs/chats 1.0 ms
:arrow_forward: :sub/run :account/account 0.0 ms
:arrow_forward: :sub/run :chain-sync-state 0.0 ms
:arrow_forward: :sub/run :latest-block-number 0.0 ms
:arrow_forward: :sub/run :…-network-uses-rpc? 0.0 ms
:arrow_forward: :sub/run :wallet/all-tokens 0.0 ms
:arrow_forward: :sub/run :…twork-initialized? 0.0 ms
:arrow_forward: :sub/run :prices 0.0 ms
:arrow_forward: :sub/run :peers-count 0.0 ms
:arrow_forward: :sub/run :dimensions/window 1.0 ms
:arrow_forward: :sub/run :mailserver/state 0.0 ms
:arrow_forward: :sub/run :…/request-error? 0.0 ms
:arrow_forward: :sub/run :get :view-id 0.0 ms
:arrow_forward: :sub/run :get :network/type 0.0 ms
:arrow_forward: :sub/run :get :…/editing? 0.0 ms
:arrow_forward: :sub/run :bottom-sheet 0.0 ms
:arrow_forward: :sub/run :get :account/account 0.0 ms
:arrow_forward: :sub/run :get :…/profile 0.0 ms
:arrow_forward: :sub/run :network-status 0.0 ms
:arrow_forward: :sub/run :get :…/profile 0.0 ms
:arrow_forward: :sub/run :sync-state 0.0 ms
:arrow_forward: :event :…ui/pull-to-refresh 72.0 ms
:arrow_forward: :event/handler 71.0 ms
:arrow_forward: :event/do-fx 0.0 ms
:arrow_forward: :event :update-wallet 20.0 ms
:arrow_forward: :event/handler 6.0 ms
:arrow_forward: :event/do-fx 7.0 ms
:arrow_forward: :event :…/on-will-focus :wallet 7.0 ms
:arrow_forward: :event/handler :wallet 1.0 ms
:arrow_forward: :event/do-fx 0.0 ms
:arrow_forward: :sub/run :…/pending-requests 0.0 ms
:arrow_forward: :sub/run :get :accounts/login 1.0 ms
:arrow_forward: :sub/run :…/settings 0.0 ms
:arrow_forward: :sub/run :…ad-messages-number 0.0 ms
:arrow_forward: :sub/run :get :…/advanced? 0.0 ms
:arrow_forward: :sub/run :get :chats/loading? 0.0 ms
:arrow_forward: :sub/run :…settings/currency 0.0 ms
:arrow_forward: :sub/run :search/filter 1.0 ms
:arrow_forward: :sub/run :wallet 0.0 ms
:arrow_forward: :sub/run :…subs/contacts 0.0 ms
:arrow_forward: :sub/run :…chat.subs/chats 0.0 ms
:arrow_forward: :sub/run :account/account 0.0 ms
:arrow_forward: :sub/run :chain-sync-state 0.0 ms
:arrow_forward: :sub/run :latest-block-number 0.0 ms
:arrow_forward: :sub/run :…-network-uses-rpc? 0.0 ms
:arrow_forward: :sub/run :wallet/all-tokens 0.0 ms
:arrow_forward: :sub/run :…twork-initialized? 0.0 ms
:arrow_forward: :sub/run :prices 0.0 ms
:arrow_forward: :sub/run :peers-count 0.0 ms
:arrow_forward: :sub/run :dimensions/window 1.0 ms
:arrow_forward: :sub/run :mailserver/state 0.0 ms
:arrow_forward: :sub/run :…/request-error? 0.0 ms
:arrow_forward: :sub/run :get :view-id 0.0 ms
:arrow_forward: :sub/run :get :network/type 0.0 ms
:arrow_forward: :sub/run :get :…/editing? 1.0 ms
:arrow_forward: :sub/run :bottom-sheet 0.0 ms
:arrow_forward: :sub/run :get :account/account 0.0 ms
:arrow_forward: :sub/run :get :…/profile 0.0 ms
:arrow_forward: :sub/run :network-status 0.0 ms
:arrow_forward: :sub/run :get :…/profile 1.0 ms
:arrow_forward: :sub/run :sync-state 0.0 ms
:arrow_forward: :event :…ate-prices-success {[…], […]} 3.0 ms
:arrow_forward: :event/handler {:SNT {:…171.85}}} 0.0 ms
:arrow_forward: :event/do-fx 1.0 ms
:arrow_forward: :sub/run :…/pending-requests 1.0 ms
:arrow_forward: :sub/run :get :accounts/login 0.0 ms
:arrow_forward: :sub/run :…/settings 0.0 ms
:arrow_forward: :sub/run :…ad-messages-number 0.0 ms
:arrow_forward: :sub/run :get :…/advanced? 0.0 ms
:arrow_forward: :sub/run :get :chats/loading? 0.0 ms
:arrow_forward: :sub/run :…settings/currency 0.0 ms
:arrow_forward: :sub/run :search/filter 0.0 ms
:arrow_forward: :sub/run :wallet 0.0 ms
:arrow_forward: :sub/run :…subs/contacts 0.0 ms
:arrow_forward: :sub/run :…chat.subs/chats 0.0 ms
:arrow_forward: :sub/run :account/account 0.0 ms
:arrow_forward: :sub/run :chain-sync-state 0.0 ms
:arrow_forward: :sub/run :latest-block-number 0.0 ms
:arrow_forward: :sub/run :…-network-uses-rpc? 0.0 ms
:arrow_forward: :sub/run :wallet/all-tokens 0.0 ms
:arrow_forward: :sub/run :…twork-initialized? 0.0 ms
:arrow_forward: :sub/run :prices 0.0 ms
:arrow_forward: :sub/run :peers-count 1.0 ms
:arrow_forward: :sub/run :dimensions/window 0.0 ms
:arrow_forward: :sub/run :mailserver/state 0.0 ms
:arrow_forward: :sub/run :…/request-error? 0.0 ms
:arrow_forward: :sub/run :get :view-id 0.0 ms
:arrow_forward: :sub/run :get :network/type 0.0 ms
:arrow_forward: :sub/run :get :…/editing? 0.0 ms
:arrow_forward: :sub/run :bottom-sheet 0.0 ms
:arrow_forward: :sub/run :get :account/account 0.0 ms
:arrow_forward: :sub/run :get :…/profile 0.0 ms
:arrow_forward: :sub/run :network-status 0.0 ms
:arrow_forward: :sub/run :get :…/profile 0.0 ms
:arrow_forward: :sub/run :sync-state 0.0 ms
:sub/run :portfolio-value 1.0 ms

#18

This is really helpful! Given the pull-to-refresh I would assume this relates to requesting fiat value. @andmironov can you look into a loading state for the wallet fiat amounts or wallet as a whole? That way we can seperate delay in rendering from delay in navigation.


#19

I’m pretty sure FlatList in RN renders only the items it needs to render, and I feel like the chat view is the least of our problems right now. At least I didn’t notice that when I was profiling.

The problem I saw is the clogged JS thread, mostly when a lot of messages arrive. In a native case you just offload this asynchronously to a background thread and deal with the results, but there isn’t an option like that for pure JS implementations.

Even if you make every JS call super efficient, there still will be a threshold at which the JS thread will be clogged. But it needs to be free, because it also dispatches all the events from touches, etc.

So, one of the ideas of fixing that part is to move all the processing of messages to status-go and to only return necessary pre-processed messages window (maybe 100 at a time) that we show. This way we will keep all the message processing, persisting, etc to the background.

There’s already an initiative on that by @adamb, and I’m planning to take it over when he is on vacation.

Btw, @flexsurfer, @yenda I want to discuss that with you :smiley:


#20

Just had call with @anna @igor @andrey to discuss how Core UI should take performance into account as we build new screens and features.

Two main things came out of that:

  1. Per Igor’s post, moving some things to status-go will potentially affect new feature implementation. The Core UI team can build accordingly with @andrey’s leadership.
  2. Determining performance metrics will help the test team enforce relevant acceptance criteria.
    A bottoms-up approach to this is to define a set of critical flows, break them out into steps (e.g. request made, awaiting response, response received), measure how long they take under various conditions, and repeat after improvements. We can then set benchmarks and compare performance across builds. Anna will take the lead on defining critical flows and metrics with Igor.

Unedited call notes


#21

You could use web workers in JS for background threads. https://github.com/devfd/react-native-workers is an example library I just found but there maybe something better. Before going down the path of multi process I would confirm the main thread is indeed the issue and the way its calls are being made are correct (IO bound calls should not be blocking) as multi proc comes with it’s own complexity. WatermelonDB (https://github.com/Nozbe/WatermelonDB) is an example of using workers in JS specifically for localDB reads and writes to take pressure off the main thread.


#22

I think we tried this library a while ago and it had some problems, though it was a year or something since then so maybe it is better now.

Also, I’m really not sure if this library actually switches threads in iOS, I only barely looked at the source code and I couldn’t find anything that creates an additional JS thread or switches to it.

I did profile the app and then the JS thread was indeed clogged and mostly because we had to process each message coming from a mailserver in JS. Theorizing about performance bottlenecks is a pretty unfruitful endeavour.

I think we have an option move down a level some of our code and just go multithreaded when we need that, since we have quite a lot of the logic already implemented there. If we just were a “pure” RN app, it would have make sense to look for a RN library for that. But we already have status-go and some API that we expose, so I think we should use that.


#23

I’m curious to know what goes into processing a message and how long on average does that single operation take?


#24

@igor I think there is still one problem with messages, because even though a lot of work was done in order to support batching them, it appears that currently there is still one event dispatched down the line for each message (:message/add event in ::chat-received-message/add-fx side effect). So if someone could look into that that might already be a big part of the clogging when fetching from mailserver

edit: I checked and they are received all at once so I was wrong


#25

Prepared PR with subscriptions optimizations, and also some optimization to navigate between main tabs, main wallet screen optimized as well https://github.com/status-im/status-react/pull/8050


#26

Thank you @andrey that is awesome work that we should have done long time ago, kudos!

I have two threads opened to discuss implementation efforts to improve performances:

  • first one in regards to communication protocol in which I describe the steps to take before moving the whole thing to status-go (doing so without doing these steps first sounds like a very bad idea to me) , @igor is planning to look at first step next week. Performances: before moving communication protocol to status-go
  • second one is improving wallet perfs ans should take about a week, my plan is to look at react-side next week, go side is basically the same as for communication protocol. [Proposal] Fix the transactions list

#27

One more picture from me, sorry
22
here you can see why wallet screen is slow, because we (1) request all tokens balances from a chain and (2) request prices from http api, each time we open wallet screen

(1) can be optimized by moving to status-go
(2) maybe we could cache it , and request prices only in some interval


#28

@andrey this is already being discussed here [Proposal] Fix the transactions list