Skip to content

Commit

Permalink
perf: Fix app freeze after login (#20729)
Browse files Browse the repository at this point in the history
We do a few things to reduce the initial load and make the app more responsive
after login. The scenario we are covering is a user who joined communities with
a large number of members and/or which contain token-gated channels with many
members.

- Related to #20283
- Related to #20285

- Optimize how we convert a community from JS to CLJS. Community members and
  chat members are no longer transformed to CLJS, they are kept as JS. Read more
  details below.
- Delay processing lower-priority events by creating a third login phase. The
  goal is to not put on the same queue we process communities less important
  events, like fetching the count of unread notifications. Around 15 events
  could be delayed without causing trouble (and this further prevent a big chain
  of more events to be dispatched right after login).
- Tried to use re-frame's flush-dom metadata, but removed due to uncertainty,
  check out the discussion:
  #20729 (comment)
  Use re-frame’s support for the flush-dom metadata whenever a signal arrives.
  According to the official documentation, this should tell re-frame to only
  process the event after the UI has been updated. It’s hard to say if this
  makes any difference, but the theory is sound.
- Reduce the amount of data returned to the subscription that renders a list of
  communities. We were returning too much, like all members, chats, token
  permissions, etc.

Other things I fixed or improved along the way:

- Because members are now stored as JS, I took the opportunity to fix how
  members are sorted when they are listed.
- Removed a few unused subs.
- Configured oops to not throw during development (in production the behavior is
  to never throw). This means oops is now safe to be used instead of interop
  that can mysteriously fail in advanced compilation.
- Show compressed key instead of public key in member list for the account
  currently logged in.

Technical details

The number one reason affecting the freeze after login was coming from
converting thousands of members inside communities and also because we were
doing it in an inefficient way using clojure.walk/stringify-keys. We shouldn't
also transform that much data on the client as the parent issue created by
flexsurfer correctly recommends. Ever since PR
#20414 was merged, status-go
doesn't return members in open channels, which greatly helps, for example, to
load the Status community. The problem still exists for communities with
token-gated channels with many members.

The current code in develop does something quite inefficient: it fetches the
communities, then transforms them recursively with js->clj and keywordizes keys,
then transforms again all the potentially thousands of member IDs back to
strings. This PR changes this. We now shallowly convert a community and ignore
members because they can grow too fast. From artificial benchmarks simulating
many members in token-gated channels, or communities with thousands of members,
the improvement is noticeable.

You will only really notice improvements if you have spectated or joined a
community with 1000+ members and/or a community with many token-gated channels,
each containing perhaps hundreds of members.

What's the ideal solution?

We should consider removing community members and channel members from the
community entity returned by status-go entirely. The members should be a
separate resource and paginated so that the client doesn't need to worry
about the number of members, for the most part.
  • Loading branch information
ilmotta authored Jul 26, 2024
1 parent 0fed811 commit c1d2d44
Show file tree
Hide file tree
Showing 21 changed files with 527 additions and 345 deletions.
137 changes: 77 additions & 60 deletions src/legacy/status_im/data_store/communities.cljs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
(ns legacy.status-im.data-store.communities
(:require
[clojure.set :as set]
[clojure.walk :as walk]
[status-im.constants :as constants]))
[status-im.constants :as constants]
[utils.transforms :as transforms]))

(defn <-revealed-accounts-rpc
[accounts]
Expand All @@ -22,19 +22,26 @@
(reduce #(assoc %1 (key-fn %2) (<-request-to-join-community-rpc %2)) {} requests))

(defn <-chats-rpc
[chats]
(reduce-kv (fn [acc k v]
(assoc acc
(name k)
(-> v
(assoc :token-gated? (:tokenGated v)
:can-post? (:canPost v)
:can-view? (:canView v)
:hide-if-permissions-not-met? (:hideIfPermissionsNotMet v))
(dissoc :canPost :tokenGated :canView :hideIfPermissionsNotMet)
(update :members walk/stringify-keys))))
{}
chats))
"This transformation from RPC is optimized differently because there can be
thousands of members in all chats and we don't want to transform them from JS
to CLJS because they will only be used to list community members or community
chat members."
[chats-js]
(let [chat-key-fn (fn [k]
(case k
"tokenGated" :token-gated?
"canPost" :can-post?
"can-view" :can-view?
"hideIfPermissionsNotMet" :hide-if-permissions-not-met?
(keyword k)))
chat-val-fn (fn [k v]
(if (= "members" k)
v
(transforms/js->clj v)))]
(transforms/<-js-map
chats-js
{:val-fn (fn [_ v]
(transforms/<-js-map v {:key-fn chat-key-fn :val-fn chat-val-fn}))})))

(defn <-categories-rpc
[categ]
Expand All @@ -48,49 +55,59 @@
[token-permission]
(= (:type token-permission) constants/community-token-permission-become-member))

(defn- rename-community-key
[k]
(case k
"canRequestAccess" :can-request-access?
"canManageUsers" :can-manage-users?
"canDeleteMessageForEveryone" :can-delete-message-for-everyone?
;; This flag is misleading based on its name alone
;; because it should not be used to decide if the user
;; is *allowed* to join. Allowance is based on token
;; permissions. Still, the flag can be used to know
;; whether or not the user will have to wait until an
;; admin approves a join request.
"canJoin" :can-join?
"requestedToJoinAt" :requested-to-join-at
"isMember" :is-member?
"outroMessage" :outro-message
"adminSettings" :admin-settings
"tokenPermissions" :token-permissions
"communityTokensMetadata" :tokens-metadata
"introMessage" :intro-message
"muteTill" :muted-till
"lastOpenedAt" :last-opened-at
"joinedAt" :joined-at
(keyword k)))

(defn <-rpc
[c]
(-> c
(set/rename-keys
{:canRequestAccess :can-request-access?
:canManageUsers :can-manage-users?
:canDeleteMessageForEveryone :can-delete-message-for-everyone?
;; This flag is misleading based on its name alone
;; because it should not be used to decide if the user
;; is *allowed* to join. Allowance is based on token
;; permissions. Still, the flag can be used to know
;; whether or not the user will have to wait until an
;; admin approves a join request.
:canJoin :can-join?
:requestedToJoinAt :requested-to-join-at
:isMember :is-member?
:outroMessage :outro-message
:adminSettings :admin-settings
:tokenPermissions :token-permissions
:communityTokensMetadata :tokens-metadata
:introMessage :intro-message
:muteTill :muted-till
:lastOpenedAt :last-opened-at
:joinedAt :joined-at})
(update :admin-settings
set/rename-keys
{:pinMessageAllMembersEnabled :pin-message-all-members-enabled?})
(update :members walk/stringify-keys)
(update :chats <-chats-rpc)
(update :token-permissions seq)
(update :categories <-categories-rpc)
(assoc :role-permissions?
(->> c
:tokenPermissions
vals
(some role-permission?)))
(assoc :membership-permissions?
(->> c
:tokenPermissions
vals
(some membership-permission?)))
(assoc :token-images
(reduce (fn [acc {sym :symbol image :image}]
(assoc acc sym image))
{}
(:communityTokensMetadata c)))))
[c-js]
(let [community (transforms/<-js-map
c-js
{:key-fn rename-community-key
:val-fn (fn [k v]
(case k
"members" v
"chats" (<-chats-rpc v)
(transforms/js->clj v)))})]
(-> community
(update :admin-settings
set/rename-keys
{:pinMessageAllMembersEnabled :pin-message-all-members-enabled?})
(update :token-permissions seq)
(update :categories <-categories-rpc)
(assoc :role-permissions?
(->> community
:tokenPermissions
vals
(some role-permission?)))
(assoc :membership-permissions?
(->> community
:tokenPermissions
vals
(some membership-permission?)))
(assoc :token-images
(reduce (fn [acc {sym :symbol image :image}]
(assoc acc sym image))
{}
(:communityTokensMetadata community))))))
2 changes: 1 addition & 1 deletion src/legacy/status_im/ui/screens/communities/members.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@
(when (and can-manage-users? (= constants/community-on-request-access (:access permissions)))
[requests-to-join community-id])
[rn/flat-list
{:data (keys sorted-members)
{:data sorted-members
:render-data {:community-id community-id
:my-public-key my-public-key
:can-kick-users? (and can-manage-users?
Expand Down
4 changes: 3 additions & 1 deletion src/native_module/core.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,9 @@
(defn set-blank-preview-flag
[flag]
(log/debug "[native-module] set-blank-preview-flag")
(.setBlankPreviewFlag ^js (encryption) flag))
;; Sometimes the app crashes during logout because `flag` is nil.
(when flag
(.setBlankPreviewFlag ^js (encryption) flag)))

(defn get-device-model-info
[]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
(ns status-im.contexts.chat.messenger.messages.link-preview.events
(:require [camel-snake-kebab.core :as csk]
[status-im.common.json-rpc.events :as json-rpc]
[taoensso.timbre :as log]
[utils.collection]
[utils.re-frame :as rf]))

Expand Down Expand Up @@ -35,17 +33,6 @@
{:fx [[:dispatch
[:profile.settings/profile-update :link-preview-request-enabled (boolean enabled?)]]]}))

(rf/reg-event-fx :chat.ui/link-preview-whitelist-received
(fn [{:keys [db]} [whitelist]]
{:db (assoc db :link-previews-whitelist whitelist)}))

(rf/reg-fx :chat.ui/request-link-preview-whitelist
(fn []
(json-rpc/call {:method "wakuext_getLinkPreviewWhitelist"
:params []
:on-success [:chat.ui/link-preview-whitelist-received]
:on-error #(log/error "Failed to get link preview whitelist")})))

(rf/reg-event-fx :chat.ui/enable-link-previews
(fn [{{:profile/keys [profile]} :db} [site enabled?]]
(let [enabled-sites (if enabled?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@
(models.contact/process-js-contacts cofx response-js)

(seq communities)
(let [communities-clj (types/js->clj communities)]
(do
(js-delete response-js "communities")
(rf/merge cofx
(process-next response-js sync-handler)
(communities/handle-communities communities-clj)))
(communities/handle-communities communities)))

(seq bookmarks)
(let [bookmarks-clj (types/js->clj bookmarks)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,24 @@
:index index})

(defn- members
[items theme]
[rn/section-list
{:key-fn :public-key
:content-container-style {:padding-bottom 20}
:get-item-layout get-item-layout
:content-inset-adjustment-behavior :never
:sections items
:sticky-section-headers-enabled false
:render-section-header-fn contact-list/contacts-section-header
:render-section-footer-fn footer
:render-data {:theme theme}
:render-fn contact-item
:scroll-event-throttle 32}])
[community-id chat-id theme]
(let [online-members (rf/sub [:communities/chat-members-sorted community-id chat-id :online])
offline-members (rf/sub [:communities/chat-members-sorted community-id chat-id :offline])]
[rn/section-list
{:key-fn :public-key
:content-container-style {:padding-bottom 20}
:get-item-layout get-item-layout
:content-inset-adjustment-behavior :never
:sections [{:title (i18n/label :t/online)
:data online-members}
{:title (i18n/label :t/offline)
:data offline-members}]
:sticky-section-headers-enabled false
:render-section-header-fn contact-list/contacts-section-header
:render-section-footer-fn footer
:render-data {:theme theme}
:render-fn contact-item
:scroll-event-throttle 32}]))

(defn view
[]
Expand All @@ -78,8 +83,6 @@
{:keys [description chat-name emoji muted chat-type color]
:as chat} (rf/sub [:chats/chat-by-id chat-id])
pins-count (rf/sub [:chats/pin-messages-count chat-id])
items (rf/sub [:communities/sorted-community-members-section-list
community-id chat-id])
theme (quo.theme/use-theme)]
(rn/use-mount (fn []
(rf/dispatch [:pin-message/load-pin-messages chat-id])))
Expand Down Expand Up @@ -133,4 +136,4 @@
(if muted
(home.actions/unmute-chat-action chat-id)
(home.actions/mute-chat-action chat-id chat-type muted)))}]}]]]
[members items theme]]))
[members community-id chat-id theme]]))
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
:as item}]
(let [user-selected? (rf/sub [:is-contact-selected? public-key])
{:keys [id]} (rf/sub [:get-screen-params])
community-members-keys (set (keys (rf/sub [:communities/community-members id])))
community-members-keys (set (rf/sub [:communities/community-members id]))
community-member? (boolean (community-members-keys public-key))
on-toggle (fn []
(when-not community-member?
Expand Down
53 changes: 25 additions & 28 deletions src/status_im/contexts/communities/events.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,6 @@

(rf/reg-event-fx :communities/handle-community handle-community)

(schema/=> handle-community
[:=>
[:catn
[:cofx :schema.re-frame/cofx]
[:args
[:schema [:catn [:community-js map?]]]]]
[:maybe
[:map
[:db [:map [:communities map?]]]
[:fx vector?]]]])

(rf/defn handle-removed-chats
[{:keys [db]} chat-ids]
{:db (reduce (fn [db chat-id]
Expand Down Expand Up @@ -84,10 +73,9 @@

(rf/defn handle-communities
{:events [:community/fetch-success]}
[{:keys [db]} communities]
{:fx
(->> communities
(map #(vector :dispatch [:communities/handle-community %])))})
[{:keys [db]} communities-js]
{:fx (map (fn [c] [:dispatch [:communities/handle-community c]])
communities-js)})

(rf/reg-event-fx :communities/request-to-join-result
(fn [{:keys [db]} [community-id request-id response-js]]
Expand Down Expand Up @@ -136,21 +124,30 @@
{}
categories))}))

(rf/reg-event-fx :community/fetch-low-priority
(fn []
{:fx [[:json-rpc/call
[{:method "wakuext_checkAndDeletePendingRequestToJoinCommunity"
:params []
:js-response true
:on-success [:sanitize-messages-and-process-response]
:on-error #(log/info "failed to fetch communities" %)}
{:method "wakuext_collapsedCommunityCategories"
:params []
:on-success [:communities/fetched-collapsed-categories-success]
:on-error #(log/error "failed to fetch collapsed community categories" %)}]]]}))

(rf/reg-event-fx :community/fetch
(fn [_]
{:json-rpc/call [{:method "wakuext_serializedCommunities"
:params []
:on-success #(rf/dispatch [:community/fetch-success %])
:on-error #(log/error "failed to fetch communities" %)}
{:method "wakuext_checkAndDeletePendingRequestToJoinCommunity"
:params []
:js-response true
:on-success #(rf/dispatch [:sanitize-messages-and-process-response %])
:on-error #(log/info "failed to fetch communities" %)}
{:method "wakuext_collapsedCommunityCategories"
:params []
:on-success #(rf/dispatch [:communities/fetched-collapsed-categories-success %])
:on-error #(log/error "failed to fetch collapsed community categories" %)}]}))
{:fx [[:json-rpc/call
[{:method "wakuext_serializedCommunities"
:params []
:on-success [:community/fetch-success]
:js-response true
:on-error #(log/error "failed to fetch communities" %)}]]
;; Dispatch a little after 1000ms because other higher-priority events
;; after login are being processed at the 1000ms mark.
[:dispatch-later [{:ms 1200 :dispatch [:community/fetch-low-priority]}]]]}))

(defn update-previous-permission-addresses
[{:keys [db]} [community-id]]
Expand Down
11 changes: 7 additions & 4 deletions src/status_im/contexts/communities/events_test.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@
(-> effects :json-rpc/call first (select-keys [:method :params]))))))))

(deftest handle-community-test
(let [community {:id community-id :clock 2}]
(let [community #js {:id community-id :clock 2}]
(testing "given a unjoined community"
(let [effects (events/handle-community {} [community])]
(is (match? community-id
Expand All @@ -163,7 +163,7 @@
(filter some? (:fx effects))))))

(testing "given a joined community"
(let [community (assoc community :joined true)
(let [community #js {:id community-id :clock 2 :joined true}
effects (events/handle-community {} [community])]
(is (match?
[[:dispatch
Expand All @@ -172,16 +172,19 @@
(filter some? (:fx effects))))))

(testing "given a community with token-permissions-check"
(let [community (assoc community :token-permissions-check :fake-token-permissions-check)
(let [community #js
{:id community-id :clock 2 :token-permissions-check :fake-token-permissions-check}
effects (events/handle-community {} [community])]
(is (match?
[[:dispatch
[:communities/check-permissions-to-join-community-with-all-addresses community-id]]]
(filter some? (:fx effects))))))

(testing "given a community with lower clock"
(let [effects (events/handle-community {:db {:communities {community-id {:clock 3}}}} [community])]
(is (nil? effects))))

(testing "given a community without clock"
(let [community (dissoc community :clock)
(let [community #js {:id community-id}
effects (events/handle-community {} [community])]
(is (nil? effects))))))
Loading

0 comments on commit c1d2d44

Please sign in to comment.